[FFmpeg-devel] [mp3] Skip APE tags when parsing mp3 packets.

[James Almer]

On 1/29/2018 10:24 PM, Michael Niedermayer wrote:
> On Mon, Jan 29, 2018 at 03:13:54PM -0800, Dale Curtis wrote:
>> Otherwise the decoder will throw "Missing header" errors when the
>> packets are sent for decoding.
> 
>>  mpegaudio_parser.c |    7 +++++++
>>  1 file changed, 7 insertions(+)
>> 2628fa8480b15237a528e94b1689da7321ce9440  skip-ape-tags.patch
>> From 05f5e837862a958fb8791668ab3cdf8fc7766fe5 Mon Sep 17 00:00:00 2001
>> From: Dale Curtis <dalecurtis at chromium.org>
>> Date: Mon, 29 Jan 2018 15:10:26 -0800
>> Subject: [PATCH] [mp3] Skip APE tags when parsing mp3 packets.
>>
>> Otherwise the decoder will throw "Missing header" errors when the
>> packets are sent for decoding.
>> ---
>>  libavcodec/mpegaudio_parser.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
>> index 8c39825792..244281b56f 100644
>> --- a/libavcodec/mpegaudio_parser.c
>> +++ b/libavcodec/mpegaudio_parser.c
>> @@ -23,6 +23,7 @@
>>  #include "parser.h"
>>  #include "mpegaudiodecheader.h"
>>  #include "libavutil/common.h"
>> +#include "libavformat/apetag.h" // for APE tag.
>>  #include "libavformat/id3v1.h" // for ID3v1_TAG_SIZE
>>  
>>  typedef struct MpegAudioParseContext {
>> @@ -120,6 +121,12 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
>>          return next;
>>      }
>>  
>> +    if (flush && buf_size >= APE_TAG_FOOTER_BYTES && memcmp(buf, APE_TAG_PREAMBLE, 8) == 0) {
>> +        *poutbuf = NULL;
>> +        *poutbuf_size = 0;
>> +        return next;
>> +    }
> 
> This doesnt feel right
> 
> Parsers should not discard data
> 
> a bistream filter could discard data, so could a demuxer if thats how the
> format should be interpreted. Or the decoder could simply detect this case
> and not print an error/warning

This patches duplicates existing code for id3v1 added by you in 89a420b71b5.
I don't know if the rationale mentioned in the above commit is still
valid today, and if it can be applied for apetag as intended by this
patch, but there's at least a precedent for this.