[FFmpeg-devel] [PATCH] libavformat/aac: Parse all ID3 tags present between ADTS frames

Michael Niedermayer michael at niedermayer.cc
Wed Apr 4 03:28:10 EEST 2018


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 <mattiasa at 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 at ffmpeg.org
> >>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel at 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 <mattiasa at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180404/285d7cc0/attachment.sig>


More information about the ffmpeg-devel mailing list