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

[James Almer]

On 11/17/2016 8:48 PM, James Almer wrote:
> 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.
>>
>> Best regards,
>> Andreas
> 
> I'll apply this patch later tonight or tomorrow morning and revert yours to
> restore the original proper behavior of making extradata available at init
> when possible, and signaling new one as needed on packets.
> 
> Will also backport it to 3.2 seeing you already backported yours. apng in
> 3.2 must not behave in a different way than 3.1.
> 
> Sorry for the delay.

Applied.