| Message ID | 20170603033333.4436-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Jun 03, 2017 at 12:33:33AM -0300, James Almer wrote: > Atempt to read and propagate only full ADTS frames and not other data, > like id3v1 or APETags at the end of the file. > > Fixes ticket #6439. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/aacdec.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c > index 5ab5197e33..aee1efe03c 100644 > --- a/libavformat/aacdec.c > +++ b/libavformat/aacdec.c > @@ -23,10 +23,11 @@ > #include "libavutil/intreadwrite.h" > #include "avformat.h" > #include "internal.h" > -#include "rawdec.h" > #include "id3v1.h" > #include "apetag.h" > > +#define ADTS_HEADER_SIZE 7 > + > static int adts_aac_probe(AVProbeData *p) > { > int max_frames = 0, first_frames = 0; > @@ -79,6 +80,7 @@ static int adts_aac_probe(AVProbeData *p) > static int adts_aac_read_header(AVFormatContext *s) > { > AVStream *st; > + uint16_t state; > > st = avformat_new_stream(s, NULL); > if (!st) > @@ -96,18 +98,54 @@ static int adts_aac_read_header(AVFormatContext *s) > avio_seek(s->pb, cur, SEEK_SET); > } > > + // skip data until the first ADTS frame is found > + state = avio_r8(s->pb); > + while (!avio_feof(s->pb)) { > + state = (state << 8) | avio_r8(s->pb); > + if ((state >> 4) != 0xFFF) > + continue; > + avio_seek(s->pb, -2, SEEK_CUR); > + break; > + } this would loop forever with /dev/zero as input no more comments from me, seems working with what i tested is it easy to add a fate test ? if so please add one thx [...]
On 6/3/2017 5:16 PM, Michael Niedermayer wrote: > On Sat, Jun 03, 2017 at 12:33:33AM -0300, James Almer wrote: >> Atempt to read and propagate only full ADTS frames and not other data, >> like id3v1 or APETags at the end of the file. >> >> Fixes ticket #6439. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/aacdec.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c >> index 5ab5197e33..aee1efe03c 100644 >> --- a/libavformat/aacdec.c >> +++ b/libavformat/aacdec.c >> @@ -23,10 +23,11 @@ >> #include "libavutil/intreadwrite.h" >> #include "avformat.h" >> #include "internal.h" >> -#include "rawdec.h" >> #include "id3v1.h" >> #include "apetag.h" >> >> +#define ADTS_HEADER_SIZE 7 >> + >> static int adts_aac_probe(AVProbeData *p) >> { >> int max_frames = 0, first_frames = 0; >> @@ -79,6 +80,7 @@ static int adts_aac_probe(AVProbeData *p) >> static int adts_aac_read_header(AVFormatContext *s) >> { >> AVStream *st; >> + uint16_t state; >> >> st = avformat_new_stream(s, NULL); >> if (!st) >> @@ -96,18 +98,54 @@ static int adts_aac_read_header(AVFormatContext *s) >> avio_seek(s->pb, cur, SEEK_SET); >> } >> >> + // skip data until the first ADTS frame is found >> + state = avio_r8(s->pb); >> + while (!avio_feof(s->pb)) { >> + state = (state << 8) | avio_r8(s->pb); >> + if ((state >> 4) != 0xFFF) >> + continue; >> + avio_seek(s->pb, -2, SEEK_CUR); >> + break; >> + } > > this would loop forever with /dev/zero as input How can i prevent this? Maybe checking a max of AVFormatContext.probesize or AVFormatContext.format_probesize bytes before bailing out? Which one if so? > > no more comments from me, seems working with what i tested > > is it easy to add a fate test ? if so please add one Sure, i'll add one after i push this. > > thx > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Sat, Jun 03, 2017 at 06:14:12PM -0300, James Almer wrote: > On 6/3/2017 5:16 PM, Michael Niedermayer wrote: > > On Sat, Jun 03, 2017 at 12:33:33AM -0300, James Almer wrote: > >> Atempt to read and propagate only full ADTS frames and not other data, > >> like id3v1 or APETags at the end of the file. > >> > >> Fixes ticket #6439. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavformat/aacdec.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > >> 1 file changed, 40 insertions(+), 2 deletions(-) > >> > >> diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c > >> index 5ab5197e33..aee1efe03c 100644 > >> --- a/libavformat/aacdec.c > >> +++ b/libavformat/aacdec.c > >> @@ -23,10 +23,11 @@ > >> #include "libavutil/intreadwrite.h" > >> #include "avformat.h" > >> #include "internal.h" > >> -#include "rawdec.h" > >> #include "id3v1.h" > >> #include "apetag.h" > >> > >> +#define ADTS_HEADER_SIZE 7 > >> + > >> static int adts_aac_probe(AVProbeData *p) > >> { > >> int max_frames = 0, first_frames = 0; > >> @@ -79,6 +80,7 @@ static int adts_aac_probe(AVProbeData *p) > >> static int adts_aac_read_header(AVFormatContext *s) > >> { > >> AVStream *st; > >> + uint16_t state; > >> > >> st = avformat_new_stream(s, NULL); > >> if (!st) > >> @@ -96,18 +98,54 @@ static int adts_aac_read_header(AVFormatContext *s) > >> avio_seek(s->pb, cur, SEEK_SET); > >> } > >> > >> + // skip data until the first ADTS frame is found > >> + state = avio_r8(s->pb); > >> + while (!avio_feof(s->pb)) { > >> + state = (state << 8) | avio_r8(s->pb); > >> + if ((state >> 4) != 0xFFF) > >> + continue; > >> + avio_seek(s->pb, -2, SEEK_CUR); > >> + break; > >> + } > > > > this would loop forever with /dev/zero as input > > How can i prevent this? Maybe checking a max of > AVFormatContext.probesize or AVFormatContext.format_probesize bytes > before bailing out? Which one if so? id tend toward probesize, but anything will do. thx [...]
On 6/4/2017 8:44 AM, Michael Niedermayer wrote: > On Sat, Jun 03, 2017 at 06:14:12PM -0300, James Almer wrote: >> On 6/3/2017 5:16 PM, Michael Niedermayer wrote: >>> On Sat, Jun 03, 2017 at 12:33:33AM -0300, James Almer wrote: >>>> Atempt to read and propagate only full ADTS frames and not other data, >>>> like id3v1 or APETags at the end of the file. >>>> >>>> Fixes ticket #6439. It's ticket 6437, sorry. Amended locally. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavformat/aacdec.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >>>> 1 file changed, 40 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c >>>> index 5ab5197e33..aee1efe03c 100644 >>>> --- a/libavformat/aacdec.c >>>> +++ b/libavformat/aacdec.c >>>> @@ -23,10 +23,11 @@ >>>> #include "libavutil/intreadwrite.h" >>>> #include "avformat.h" >>>> #include "internal.h" >>>> -#include "rawdec.h" >>>> #include "id3v1.h" >>>> #include "apetag.h" >>>> >>>> +#define ADTS_HEADER_SIZE 7 >>>> + >>>> static int adts_aac_probe(AVProbeData *p) >>>> { >>>> int max_frames = 0, first_frames = 0; >>>> @@ -79,6 +80,7 @@ static int adts_aac_probe(AVProbeData *p) >>>> static int adts_aac_read_header(AVFormatContext *s) >>>> { >>>> AVStream *st; >>>> + uint16_t state; >>>> >>>> st = avformat_new_stream(s, NULL); >>>> if (!st) >>>> @@ -96,18 +98,54 @@ static int adts_aac_read_header(AVFormatContext *s) >>>> avio_seek(s->pb, cur, SEEK_SET); >>>> } >>>> >>>> + // skip data until the first ADTS frame is found >>>> + state = avio_r8(s->pb); >>>> + while (!avio_feof(s->pb)) { >>>> + state = (state << 8) | avio_r8(s->pb); >>>> + if ((state >> 4) != 0xFFF) >>>> + continue; >>>> + avio_seek(s->pb, -2, SEEK_CUR); >>>> + break; >>>> + } >>> >>> this would loop forever with /dev/zero as input >> >> How can i prevent this? Maybe checking a max of >> AVFormatContext.probesize or AVFormatContext.format_probesize bytes >> before bailing out? Which one if so? > > id tend toward probesize, but anything will do. > > thx Done and pushed, thanks.
diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c index 5ab5197e33..aee1efe03c 100644 --- a/libavformat/aacdec.c +++ b/libavformat/aacdec.c @@ -23,10 +23,11 @@ #include "libavutil/intreadwrite.h" #include "avformat.h" #include "internal.h" -#include "rawdec.h" #include "id3v1.h" #include "apetag.h" +#define ADTS_HEADER_SIZE 7 + static int adts_aac_probe(AVProbeData *p) { int max_frames = 0, first_frames = 0; @@ -79,6 +80,7 @@ static int adts_aac_probe(AVProbeData *p) static int adts_aac_read_header(AVFormatContext *s) { AVStream *st; + uint16_t state; st = avformat_new_stream(s, NULL); if (!st) @@ -96,18 +98,54 @@ static int adts_aac_read_header(AVFormatContext *s) avio_seek(s->pb, cur, SEEK_SET); } + // skip data until the first ADTS frame is found + state = avio_r8(s->pb); + while (!avio_feof(s->pb)) { + state = (state << 8) | avio_r8(s->pb); + if ((state >> 4) != 0xFFF) + continue; + avio_seek(s->pb, -2, SEEK_CUR); + break; + } + // LCM of all possible ADTS sample rates avpriv_set_pts_info(st, 64, 1, 28224000); return 0; } +static int adts_aac_read_packet(AVFormatContext *s, AVPacket *pkt) +{ + int ret, fsize; + + ret = av_get_packet(s->pb, pkt, ADTS_HEADER_SIZE); + if (ret < 0) + return ret; + if (ret < ADTS_HEADER_SIZE) { + av_packet_unref(pkt); + return AVERROR(EIO); + } + + if ((AV_RB16(pkt->data) >> 4) != 0xfff) { + av_packet_unref(pkt); + return AVERROR_INVALIDDATA; + } + + fsize = (AV_RB32(pkt->data + 3) >> 13) & 0x1FFF; + if (fsize < ADTS_HEADER_SIZE) { + av_packet_unref(pkt); + return AVERROR_INVALIDDATA; + } + + return av_append_packet(s->pb, pkt, fsize - ADTS_HEADER_SIZE); +} + AVInputFormat ff_aac_demuxer = { .name = "aac", .long_name = NULL_IF_CONFIG_SMALL("raw ADTS AAC (Advanced Audio Coding)"), .read_probe = adts_aac_probe, .read_header = adts_aac_read_header, - .read_packet = ff_raw_read_partial_packet, + .read_packet = adts_aac_read_packet, .flags = AVFMT_GENERIC_INDEX, .extensions = "aac", .mime_type = "audio/aac,audio/aacp,audio/x-aac",
Atempt to read and propagate only full ADTS frames and not other data, like id3v1 or APETags at the end of the file. Fixes ticket #6439. Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/aacdec.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-)