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.cadhal...@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.cadhal...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
