[FFmpeg-devel] [PATCH] avformat/apngenc: use the stream parameters extradata if no updated one is made available

[Andreas Cadhalpun]

On 01.11.2016 05:09, James Almer wrote:
> On 10/31/2016 11:32 PM, James Almer wrote:
>> Fixes remuxing apng streams coming from the apng demuxer.
>> This is a regression since 97792e85c338d129342f5812e2a52048373e57d6.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavformat/apngenc.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/libavformat/apngenc.c b/libavformat/apngenc.c
>> index e5e8aa9..2b88dcc 100644
>> --- a/libavformat/apngenc.c
>> +++ b/libavformat/apngenc.c
>> @@ -101,6 +101,13 @@ static int apng_write_header(AVFormatContext *format_context)
>>      avio_wb64(format_context->pb, PNGSIG);
>>      // Remaining headers are written when they are copied from the encoder
>>  
>> +    apng->extra_data = av_mallocz(format_context->streams[0]->codecpar->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
>> +    if (!apng->extra_data)
>> +        return AVERROR(ENOMEM);
>> +    apng->extra_data_size =  format_context->streams[0]->codecpar->extradata_size;
>> +    memcpy(apng->extra_data, format_context->streams[0]->codecpar->extradata,
>> +           format_context->streams[0]->codecpar->extradata_size);
>> +
> 
> Alternatively we could just go back to the first version of Andreas' patch,
> where the codecpar extradata was overwritten by the updated one from the
> packet side data, and get rid of the private context buffer.
> 
> I assumed keeping the codecpar extradata intact was the correct behavior,
> based on the avcodec.h documentation and since AVCodecParameters is an
> intermediary struct meant to pass parameters between de/muxers and
> de/encoders, but it seems the mov and flv muxers just overwrite it. I'm not
> sure if it makes a difference at all.
> 
> Any opinions?

What I like about the approach of using the private extra_data context buffer is
that is is quite obvious where it is set. On the other hand the codecpar
extradata can be set anywhere, which makes it difficult to understand/debug.
The very bug this patch would fix is an excellent example of that.

So I'd rather convert the apngdec demuxer to pass the extradata as side data.
I'll send a patch doing that.

Best regards,
Andreas