[FFmpeg-devel] [PATCH] ffmpeg: copy the extradata from encoder to muxer

[Andreas Cadhalpun]

On 27.10.2016 23:39, James Almer wrote:
> On 10/27/2016 5:38 PM, Andreas Cadhalpun wrote:
>>  libavcodec/pngenc.c   |  4 ++++
>>  libavformat/apngenc.c | 27 ++++++++++++++++++++++++---
>>  2 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
>> index 00c830e..1a308f2 100644
>> --- a/libavcodec/pngenc.c
>> +++ b/libavcodec/pngenc.c
>> @@ -917,6 +917,10 @@ static int encode_apng(AVCodecContext *avctx, AVPacket *pkt,
>>      if (s->last_frame) {
>>          uint8_t* last_fctl_chunk_start = pkt->data;
>>          uint8_t buf[26];
>> +        uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA, avctx->extradata_size);
> 
> You could add a variable called extradata_updated or so to PNGEncContext and set it to
> 1 here so the encoder doesn't add side data to every packet when it's only needed for
> the first.

OK.

>> +        if (!side_data)
>> +            return AVERROR(ENOMEM);
>> +        memcpy(side_data, avctx->extradata, avctx->extradata_size);
>>  
>>          AV_WB32(buf + 0, s->last_frame_fctl.sequence_number);
>>          AV_WB32(buf + 4, s->last_frame_fctl.width);
>> diff --git a/libavformat/apngenc.c b/libavformat/apngenc.c
>> index e25df2e..f702667 100644
>> --- a/libavformat/apngenc.c
>> +++ b/libavformat/apngenc.c
>> @@ -101,15 +101,29 @@ static int apng_write_header(AVFormatContext *format_context)
>>      return 0;
>>  }
>>  
>> -static void flush_packet(AVFormatContext *format_context, AVPacket *packet)
>> +static int flush_packet(AVFormatContext *format_context, AVPacket *packet)
>>  {
>>      APNGMuxContext *apng = format_context->priv_data;
>>      AVIOContext *io_context = format_context->pb;
>>      AVStream *codec_stream = format_context->streams[0];
>>      AVCodecParameters *codec_par = codec_stream->codecpar;
>> +    uint8_t *side_data = NULL;
>> +    int side_data_size = 0;
>>  
>>      av_assert0(apng->prev_packet);
>>  
>> +    if (packet)
>> +        side_data = av_packet_get_side_data(packet, AV_PKT_DATA_NEW_EXTRADATA, &side_data_size);
> 
> If the muxer gets only one frame, the code below (standard png mode fallback) will not
> work. You can test this with "./ffmpeg -f lavfi -i testsrc=s=32x32 -vframes 1 apng.apng".
> 
> Don't check for packet and use apng->prev_packet unconditionally instead. That should
> do it.

Indeed, fixed.

> Ideally, you'd not use avctx->extradata* in the apng encoder or codecpar->extradata*
> in the muxer. The former should only be written by the init() function, and the latter
> should afaik not be modified by the muxer at all.
> If you can store the pointers and sizes for the extradata in the encoder/muxer contexts
> and use them instead of avctx and codecpar extradata then that'd be great.

Fine for me, updated patch attached.
To avoid confusion and for lack of a better name I called the variables in the encoder/muxer
contexts extra_side_data*.

Best regards,
Andreas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-apng-use-side-data-to-pass-extradata-to-muxer.patch
Type: text/x-diff
Size: 8035 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161028/7f290299/attachment.patch>