diff mbox series

[FFmpeg-devel] avformat/aacdec: enable probesize-sized resyncs mid-file

Message ID 20210927213133.28258-1-jeebjp@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/aacdec: enable probesize-sized resyncs mid-file | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Jan Ekström Sept. 27, 2021, 9:31 p.m. UTC 
Before adts_aac_resync would always bail out after probesize amount
of bytes had been progressed from the start of the input.

Add an argument for the start position, and set it to zero when
reading the header (which should happen in the beginning) to mimic
previous behavior of going only up to probesize. Then, when doing
a resync mid-file when reading a packet, pass the current position
in stream to the function.

Fixes #9433
---
 libavformat/aacdec.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

James Almer Sept. 27, 2021, 10:34 p.m. UTC | #1 
On 9/27/2021 6:31 PM, Jan Ekström wrote:
> Before adts_aac_resync would always bail out after probesize amount
> of bytes had been progressed from the start of the input.
> 
> Add an argument for the start position, and set it to zero when
> reading the header (which should happen in the beginning) to mimic
> previous behavior of going only up to probesize. Then, when doing
> a resync mid-file when reading a packet, pass the current position
> in stream to the function.

There's no need to keep the probesize limit from start of stream 
hardcoded in adts_aac_read_header(). Your solution in 
http://up-cat.net/p/e046e8f7 is IMO simpler. It will ensure any call to 
adts_aac_resync() will read only up to probesize bytes from the current 
position of the stream.

> 
> Fixes #9433
> ---
>   libavformat/aacdec.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c
> index ab97be60b5..1b0e05d256 100644
> --- a/libavformat/aacdec.c
> +++ b/libavformat/aacdec.c
> @@ -80,13 +80,14 @@ static int adts_aac_probe(const AVProbeData *p)
>           return 0;
>   }
>   
> -static int adts_aac_resync(AVFormatContext *s)
> +static int adts_aac_resync(AVFormatContext *s, int64_t start_pos)
>   {
>       uint16_t state;
>   
>       // skip data until an ADTS frame is found
>       state = avio_r8(s->pb);
> -    while (!avio_feof(s->pb) && avio_tell(s->pb) < s->probesize) {
> +    while (!avio_feof(s->pb) &&
> +           (avio_tell(s->pb) - start_pos) < s->probesize) {
>           state = (state << 8) | avio_r8(s->pb);
>           if ((state >> 4) != 0xFFF)
>               continue;
> @@ -122,7 +123,7 @@ static int adts_aac_read_header(AVFormatContext *s)
>           avio_seek(s->pb, cur, SEEK_SET);
>       }
>   
> -    ret = adts_aac_resync(s);
> +    ret = adts_aac_resync(s, 0);
>       if (ret < 0)
>           return ret;
>   
> @@ -187,7 +188,7 @@ retry:
>           }
>           if (!ff_id3v2_match(pkt->data, ID3v2_DEFAULT_MAGIC)) {
>               av_packet_unref(pkt);
> -            ret = adts_aac_resync(s);
> +            ret = adts_aac_resync(s, avio_tell(s->pb));
>           } else
>               ret = handle_id3(s, pkt);
>           if (ret < 0)
>
Jan Ekström Sept. 27, 2021, 10:40 p.m. UTC | #2 
On Tue, Sep 28, 2021 at 1:34 AM James Almer <jamrial@gmail.com> wrote:
>
> On 9/27/2021 6:31 PM, Jan Ekström wrote:
> > Before adts_aac_resync would always bail out after probesize amount
> > of bytes had been progressed from the start of the input.
> >
> > Add an argument for the start position, and set it to zero when
> > reading the header (which should happen in the beginning) to mimic
> > previous behavior of going only up to probesize. Then, when doing
> > a resync mid-file when reading a packet, pass the current position
> > in stream to the function.
>
> There's no need to keep the probesize limit from start of stream
> hardcoded in adts_aac_read_header(). Your solution in
> http://up-cat.net/p/e046e8f7 is IMO simpler. It will ensure any call to
> adts_aac_resync() will read only up to probesize bytes from the current
> position of the stream.
>

Alright. I thought that was done for a reason and thus tried to
emulate it, but yes - I did like my initial version's simplicity as
well.

Will post a v2 with that.

Jan
diff mbox series

Patch

diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c
index ab97be60b5..1b0e05d256 100644
--- a/libavformat/aacdec.c
+++ b/libavformat/aacdec.c
@@ -80,13 +80,14 @@  static int adts_aac_probe(const AVProbeData *p)
         return 0;
 }
 
-static int adts_aac_resync(AVFormatContext *s)
+static int adts_aac_resync(AVFormatContext *s, int64_t start_pos)
 {
     uint16_t state;
 
     // skip data until an ADTS frame is found
     state = avio_r8(s->pb);
-    while (!avio_feof(s->pb) && avio_tell(s->pb) < s->probesize) {
+    while (!avio_feof(s->pb) &&
+           (avio_tell(s->pb) - start_pos) < s->probesize) {
         state = (state << 8) | avio_r8(s->pb);
         if ((state >> 4) != 0xFFF)
             continue;
@@ -122,7 +123,7 @@  static int adts_aac_read_header(AVFormatContext *s)
         avio_seek(s->pb, cur, SEEK_SET);
     }
 
-    ret = adts_aac_resync(s);
+    ret = adts_aac_resync(s, 0);
     if (ret < 0)
         return ret;
 
@@ -187,7 +188,7 @@  retry:
         }
         if (!ff_id3v2_match(pkt->data, ID3v2_DEFAULT_MAGIC)) {
             av_packet_unref(pkt);
-            ret = adts_aac_resync(s);
+            ret = adts_aac_resync(s, avio_tell(s->pb));
         } else
             ret = handle_id3(s, pkt);
         if (ret < 0)