On Tue, Apr 03, 2018 at 07:58:53AM +0200, Mattias Amnefelt wrote: > Yes, my feeling was also that it's better to handle this when possible. > > You are of course correct that the two tags needs to be inbetween frames. > Sorry about that, I stripped the sample down too much. I updated with a > sample which has two frames. This new sample fails the test without the > patch. > > /mattiasa > > On 2018-04-02 23:25, Richard Shaffer wrote: > >It seems like some software can insert an additional ID3v2 tag instead > >of appending a frame to an existing one. One could argue that that is > >somewhat broken, but I agree it's better to handle it instead of > >returning an error. The changes in aacdec.c look ok to me. > > > >The fate sample you provided has two tags at the beginning of the > >file. These will actually get parsed by avformat_open_input calling > >ff_id3v2_read_dict and not by the adts demuxer. The ff_id3v2_read_dict > >code path will already parse multiple tags, which is why the test > >passes. In order to exercise your code change, the ID3v2 tags would > >have to be between ADTS frames. > > > >-Richard > > > >On Mon, Apr 2, 2018 at 4:35 AM, Mattias Amnefelt <matti...@avm.se> wrote: > >>This is a follow-up to https://patchwork.ffmpeg.org/patch/7477/ and changes > >>so all ID3 tags between ADTS frames gets parsed, not just the first one. > >> > >>Sample media for the included fate test is available at > >>http://mattias.amnefe.lt/tmp/id3v2_two_tags.aac > >> > >> > >> > >>_______________________________________________ > >>ffmpeg-devel mailing list > >>ffmpeg-devel@ffmpeg.org > >>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >_______________________________________________ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
> libavformat/aacdec.c | 14 > tests/fate/demux.mak | 3 > tests/ref/fate/adts-id3v2-two-tags-demux | 475 > +++++++++++++++++++++++++++++++ > 3 files changed, 486 insertions(+), 6 deletions(-) > 95d2c77bc7fa82503377e09da98fe17e1b3eb302 > 0001-libavformat-aac-Parse-all-ID3-tags-present-between-A.patch > From 8d587983b6cc5c535e29f0898d4cac433cd0a609 Mon Sep 17 00:00:00 2001 > From: Mattias Amnefelt <matti...@avm.se> > Date: Mon, 2 Apr 2018 11:30:40 +0200 > Subject: [PATCH] libavformat/aac: Parse all ID3 tags present between ADTS > frames > > Some ADTS streams can have multiple ID3 tags between frames. This > change parses all of them, rather than just the first one. uploaded aac fate sample the patch should be ok thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is what and why we do it that matters, not just one of them.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel