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

[James Almer]

On 10/27/2016 7:43 PM, Andreas Cadhalpun wrote:
> 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*.

extra_data* (with an underscore so it's different) is IMO better. It contains new codec
extradata, as the AVPacketSideDataType enum states, and is simply transmitted from a
library to the other as packet side data.

> 
> Best regards,
> Andreas
> 
> 
> 0001-apng-use-side-data-to-pass-extradata-to-muxer.patch
> 
> 
> From 20ad7d6905ce2123fd8100b6fe6e092dbbdf3c06 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Thu, 27 Oct 2016 22:34:48 +0200
> Subject: [PATCH] apng: use side data to pass extradata to muxer
> 
> This fixes creating apng files, which is broken since commit
> 5ef19590802f000299e418143fc2301e3f43affe.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavcodec/pngenc.c   | 18 +++++++++++++++---
>  libavformat/apngenc.c | 45 +++++++++++++++++++++++++++++++++++----------
>  2 files changed, 50 insertions(+), 13 deletions(-)

Seems to work, so LGTM with or without the above suggestion.

Thanks.