On Sat, Mar 28, 2015 at 09:12:10AM +0000, Donny Yang wrote: > Sorry for the delay. I was a bit busy this week. > > On 25 March 2015 at 09:59, Michael Niedermayer <michae...@gmx.at> wrote: > > > is there any advantage for multiple small IDATs ? > > if not i suggest to make the IDAT change to png as well in a seperate > > patch so that a single frame APNG and PNG produce identical data > > and especially so that the introduction of APNG does not change PNG > > files > > > Having multiple small IDAT sections is simply to make the encoder easier to > write. > For now, I've left it how it is, but now the APNG encoder and muxer > produces identical output to PNG before. > > I've also changed my edits to libavcodec a fair bit as well after thinking > more carefully about the next steps in implementing a full APNG encoder. > The APNG encoder currently just outputs normal PNG headers and IDAT chunks, > while the muxer adds the necessary headers for APNG and converts the IDAT > chunks to fdAT where needed. >
> On 28 March 2015 at 18:24, Paul B Mahol <one...@gmail.com> wrote:
>
> > Dana 28. 3. 2015. 04:56 osoba "Donny Yang" <w...@kota.moe> napisala je:
> > > On 28 March 2015 at 04:36, Paul B Mahol <one...@gmail.com> wrote:
> > >>
> > >> The style of code inside patch do not match other files in codebase,
> > >> look at style of other files inside codebase.
> > >
> > > I assume you're referring to the opening brace style on functions?
> > >
> > Yes.
> >
> Okay, I've fixed them.
>
> Also, I just realised I'm not supposed to be sending more than one patch
> per email.
> I've put them again for this email, but what should I do instead next time?
git send-email should send one patch per mail
>
> configure | 1
> libavcodec/Makefile | 1
> libavcodec/allcodecs.c | 2
> libavcodec/pngenc.c | 326
> +++++++++++++++++++++++++++++--------------------
> libavcodec/version.h | 2
> 5 files changed, 203 insertions(+), 129 deletions(-)
> 2724818cf358ca284e59add36b1a28e410bc28da
> 0001-apng-Make-PNG-encoder-only-write-headers-once-in-APN.patch
> From cdc40aa212d3c8a34b2895213f6bc63efe881df5 Mon Sep 17 00:00:00 2001
> From: Donny Yang <w...@kota.moe>
> Date: Sat, 28 Mar 2015 19:09:11 +1100
> Subject: [PATCH 1/2] apng: Make PNG encoder only write headers once in APNG
> mode
>
> Signed-off-by: Donny Yang <w...@kota.moe>
> ---
> configure | 1 +
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 2 +-
> libavcodec/pngenc.c | 326
> ++++++++++++++++++++++++++++++-------------------
> libavcodec/version.h | 2 +-
> 5 files changed, 203 insertions(+), 129 deletions(-)
>
> diff --git a/configure b/configure
> index 017a9d2..8a30549 100755
> --- a/configure
> +++ b/configure
> @@ -2096,6 +2096,7 @@ amv_decoder_select="sp5x_decoder exif"
> amv_encoder_select="aandcttables mpegvideoenc"
> ape_decoder_select="bswapdsp llauddsp"
> apng_decoder_select="zlib"
> +apng_encoder_select="huffyuvencdsp zlib"
> asv1_decoder_select="blockdsp bswapdsp idctdsp"
> asv1_encoder_select="bswapdsp fdctdsp pixblockdsp"
> asv2_decoder_select="blockdsp bswapdsp idctdsp"
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index b2d9c71..9a145d3 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -144,6 +144,7 @@ OBJS-$(CONFIG_ANM_DECODER) += anm.o
> OBJS-$(CONFIG_ANSI_DECODER) += ansi.o cga_data.o
> OBJS-$(CONFIG_APE_DECODER) += apedec.o
> OBJS-$(CONFIG_APNG_DECODER) += png.o pngdec.o pngdsp.o
> +OBJS-$(CONFIG_APNG_ENCODER) += png.o pngenc.o
> OBJS-$(CONFIG_SSA_DECODER) += assdec.o ass.o ass_split.o
> OBJS-$(CONFIG_SSA_ENCODER) += assenc.o ass.o
> OBJS-$(CONFIG_ASS_DECODER) += assdec.o ass.o ass_split.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 10aad4c..2e5d558 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -107,7 +107,7 @@ void avcodec_register_all(void)
> REGISTER_ENCDEC (AMV, amv);
> REGISTER_DECODER(ANM, anm);
> REGISTER_DECODER(ANSI, ansi);
> - REGISTER_DECODER(APNG, apng);
> + REGISTER_ENCDEC (APNG, apng);
> REGISTER_ENCDEC (ASV1, asv1);
> REGISTER_ENCDEC (ASV2, asv2);
> REGISTER_DECODER(AURA, aura);
> diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
> index 9bdefc4..6959435 100644
> --- a/libavcodec/pngenc.c
> +++ b/libavcodec/pngenc.c
> @@ -48,6 +48,11 @@ typedef struct PNGEncContext {
> uint8_t buf[IOBUF_SIZE];
> int dpi; ///< Physical pixel density, in dots per
> inch, if set
> int dpm; ///< Physical pixel density, in dots per
> meter, if set
> +
> + int is_progressive;
> + int bit_depth;
> + int color_type;
> + int bits_per_pixel;
> } PNGEncContext;
>
> static void png_get_interlaced_row(uint8_t *dst, int row_size,
> @@ -286,107 +291,9 @@ static int png_get_gama(enum
> AVColorTransferCharacteristic trc, uint8_t *buf)
> return 1;
> }
>
> -static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> - const AVFrame *pict, int *got_packet)
> +static int encode_headers(AVCodecContext *avctx, const AVFrame *pict)
> {
> - PNGEncContext *s = avctx->priv_data;
> - const AVFrame *const p = pict;
> - int bit_depth, color_type, y, len, row_size, ret, is_progressive;
> - int bits_per_pixel, pass_row_size, enc_row_size;
> - int64_t max_packet_size;
> - int compression_level;
> - uint8_t *ptr, *top, *crow_buf, *crow;
> - uint8_t *crow_base = NULL;
> - uint8_t *progressive_buf = NULL;
> - uint8_t *top_buf = NULL;
> -
> - is_progressive = !!(avctx->flags & CODEC_FLAG_INTERLACED_DCT);
> - switch (avctx->pix_fmt) {
> - case AV_PIX_FMT_RGBA64BE:
> - bit_depth = 16;
> - color_type = PNG_COLOR_TYPE_RGB_ALPHA;
> - break;
> - case AV_PIX_FMT_RGB48BE:
> - bit_depth = 16;
> - color_type = PNG_COLOR_TYPE_RGB;
> - break;
> - case AV_PIX_FMT_RGBA:
> - bit_depth = 8;
> - color_type = PNG_COLOR_TYPE_RGB_ALPHA;
> - break;
> - case AV_PIX_FMT_RGB24:
> - bit_depth = 8;
> - color_type = PNG_COLOR_TYPE_RGB;
> - break;
> - case AV_PIX_FMT_GRAY16BE:
> - bit_depth = 16;
> - color_type = PNG_COLOR_TYPE_GRAY;
> - break;
> - case AV_PIX_FMT_GRAY8:
> - bit_depth = 8;
> - color_type = PNG_COLOR_TYPE_GRAY;
> - break;
> - case AV_PIX_FMT_GRAY8A:
> - bit_depth = 8;
> - color_type = PNG_COLOR_TYPE_GRAY_ALPHA;
> - break;
> - case AV_PIX_FMT_YA16BE:
> - bit_depth = 16;
> - color_type = PNG_COLOR_TYPE_GRAY_ALPHA;
> - break;
> - case AV_PIX_FMT_MONOBLACK:
> - bit_depth = 1;
> - color_type = PNG_COLOR_TYPE_GRAY;
> - break;
> - case AV_PIX_FMT_PAL8:
> - bit_depth = 8;
> - color_type = PNG_COLOR_TYPE_PALETTE;
> - break;
> - default:
> - return -1;
> - }
> - bits_per_pixel = ff_png_get_nb_channels(color_type) * bit_depth;
> - row_size = (avctx->width * bits_per_pixel + 7) >> 3;
> -
> - s->zstream.zalloc = ff_png_zalloc;
> - s->zstream.zfree = ff_png_zfree;
> - s->zstream.opaque = NULL;
> - compression_level = avctx->compression_level == FF_COMPRESSION_DEFAULT
> - ? Z_DEFAULT_COMPRESSION
> - : av_clip(avctx->compression_level, 0, 9);
> - ret = deflateInit2(&s->zstream, compression_level,
> - Z_DEFLATED, 15, 8, Z_DEFAULT_STRATEGY);
> - if (ret != Z_OK)
> - return -1;
> -
> - enc_row_size = deflateBound(&s->zstream, row_size);
> - max_packet_size = avctx->height * (int64_t)(enc_row_size +
> - ((enc_row_size + IOBUF_SIZE - 1) /
> IOBUF_SIZE) * 12)
> - + FF_MIN_BUFFER_SIZE;
> - if (max_packet_size > INT_MAX)
> - return AVERROR(ENOMEM);
> - if ((ret = ff_alloc_packet2(avctx, pkt, max_packet_size)) < 0)
> - return ret;
> -
> - s->bytestream_start =
> - s->bytestream = pkt->data;
> - s->bytestream_end = pkt->data + pkt->size;
> -
> - crow_base = av_malloc((row_size + 32) << (s->filter_type ==
> PNG_FILTER_VALUE_MIXED));
> - if (!crow_base)
> - goto fail;
> - // pixel data should be aligned, but there's a control byte before it
> - crow_buf = crow_base + 15;
> - if (is_progressive) {
> - progressive_buf = av_malloc(row_size + 1);
> - if (!progressive_buf)
> - goto fail;
> - }
> - if (is_progressive) {
> - top_buf = av_malloc(row_size + 1);
> - if (!top_buf)
> - goto fail;
> - }
> + PNGEncContext *s = avctx->priv_data;
moving code around should be in a seperate patch from changing code
in ways other than needed for the move
>
> /* write png header */
> AV_WB64(s->bytestream, PNGSIG);
> @@ -394,14 +301,14 @@ static int encode_frame(AVCodecContext *avctx, AVPacket
> *pkt,
>
> AV_WB32(s->buf, avctx->width);
> AV_WB32(s->buf + 4, avctx->height);
> - s->buf[8] = bit_depth;
> - s->buf[9] = color_type;
> + s->buf[8] = s->bit_depth;
> + s->buf[9] = s->color_type;
> s->buf[10] = 0; /* compression type */
> s->buf[11] = 0; /* filter type */
> - s->buf[12] = is_progressive; /* interlace type */
> -
> + s->buf[12] = s->is_progressive; /* interlace type */
> png_write_chunk(&s->bytestream, MKTAG('I', 'H', 'D', 'R'), s->buf, 13);
moving variables to the context should also be in a seperate patch
[...]
> the_end:
> - av_free(crow_base);
> - av_free(progressive_buf);
> - av_free(top_buf);
> - deflateEnd(&s->zstream);
> + av_freep(&crow_base);
> + av_freep(&progressive_buf);
> + av_freep(&top_buf);
> + deflateReset(&s->zstream);
> return ret;
changing av_free to av_freep() should be a seperate patch as well
> -fail:
> - ret = -1;
> - goto the_end;
> +}
> +
> +static int encode(AVCodecContext *avctx, AVPacket *pkt,
> + const AVFrame *pict, int *got_packet)
> +{
> + PNGEncContext *s = avctx->priv_data;
> + int ret;
> + int enc_row_size;
> + size_t max_packet_size;
> +
> + enc_row_size = deflateBound(&s->zstream, (avctx->width *
> s->bits_per_pixel + 7) >> 3);
> + max_packet_size =
> + 8 + // PNGSIG
> + 13 + 12 + // IHDR
> + 9 + 12 + // pHYs
> + 1 + 12 + // sRGB
> + 32 + 12 + // cHRM
> + 4 + 12 + // gAMA
> + 256 * 3 + 12 + // PLTE
> + 256 + 12 + // tRNS
> + avctx->height * (
> + enc_row_size +
> + 12 * ((enc_row_size + IOBUF_SIZE - 1) / IOBUF_SIZE) // 12 *
> ceil(enc_row_size / IOBUF_SIZE)
> + );
> +
> + ret = ff_alloc_packet2(avctx, pkt, max_packet_size < FF_MIN_BUFFER_SIZE
> ? FF_MIN_BUFFER_SIZE : max_packet_size);
> + if (ret)
> + return ret;
> + s->bytestream_start =
> + s->bytestream = pkt->data;
> + s->bytestream_end = pkt->data + pkt->size;
> +
> + if (avctx->codec_id == AV_CODEC_ID_PNG || pict->pts == 0) {
pts == 0 is not a reliable way to detect the first picture
this patchset also breaks PAL8
./ffmpeg -i lena.pnm -pix_fmt pal8 test.png
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
