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

James Almer jamrial at gmail.com
Tue Nov 1 18:06:14 EET 2016

On 11/1/2016 12:54 PM, Andreas Cadhalpun wrote:
> 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.

Is it worth doing that? This patch is pretty simple and solves the issue for
both encoder and demuxer.
With it the muxer will use either the original extradata or any new one sent
within a packet. In both cases, the muxer uses its own buffer to keep it.

I don't think changing the demuxer to append the same extradata it already
sent during init() again with a packet makes sense. It's extra complexity
for no gain.

More information about the ffmpeg-devel mailing list