Niklas Haas:
> From: Niklas Haas <g...@haasn.dev>
>
> encode_zbuf is mostly a mirror image of decode_zbuf. Other than that,
> the code is pretty straightforward. Special care needs to be taken to
> avoid writing more than 79 characters of the profile description (the
> maximum supported).
>
> Also add a FATE transcode test to ensure that the ICC profile gets
> encoded correctly.
> ---
> Oops. `-c copy` doesn't actually test the PNG writing code. Need to use
> `-c png` instead. Fixed in v2.
> ---
> libavcodec/pngenc.c | 77 +++++++++++++++++++++++++++++++++++++++++-
> tests/fate/image.mak | 3 ++
> tests/ref/fate/png-icc | 8 +++++
> 3 files changed, 87 insertions(+), 1 deletion(-)
> create mode 100644 tests/ref/fate/png-icc
>
> diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
> index 3ebcc1e571..24530bb62f 100644
> --- a/libavcodec/pngenc.c
> +++ b/libavcodec/pngenc.c
> @@ -28,6 +28,7 @@
> #include "apng.h"
>
> #include "libavutil/avassert.h"
> +#include "libavutil/bprint.h"
> #include "libavutil/crc.h"
> #include "libavutil/libm.h"
> #include "libavutil/opt.h"
> @@ -343,6 +344,65 @@ static int png_get_gama(enum
> AVColorTransferCharacteristic trc, uint8_t *buf)
> return 1;
> }
>
> +static int encode_zbuf(AVBPrint *bp, const uint8_t *data, size_t size)
> +{
> + z_stream zstream;
> + unsigned char *buf;
> + unsigned buf_size;
> + int ret;
> +
> + zstream.zalloc = ff_png_zalloc,
> + zstream.zfree = ff_png_zfree,
> + zstream.opaque = NULL;
> + if (deflateInit(&zstream, Z_DEFAULT_COMPRESSION) != Z_OK)
> + return AVERROR_EXTERNAL;
> + zstream.next_in = data;
> + zstream.avail_in = size;
> +
> + for (;;) {
> + av_bprint_get_buffer(bp, 2, &buf, &buf_size);
> + if (buf_size < 2) {
> + deflateEnd(&zstream);
> + return AVERROR(ENOMEM);
> + }
> +
> + zstream.next_out = buf;
> + zstream.avail_out = buf_size - 1;
> + ret = deflate(&zstream, Z_FINISH);
> + if (ret != Z_OK && ret != Z_STREAM_END) {
> + deflateEnd(&zstream);
> + return AVERROR_EXTERNAL;
> + }
> +
> + bp->len += zstream.next_out - buf;
> + if (ret == Z_STREAM_END) {
> + deflateEnd(&zstream);
> + return 0;
> + }
> + }
> +}
> +
> +static int png_get_iccp(AVBPrint *bp, const AVFrameSideData *sd, char
> **buf_out)
> +{
> + const AVDictionaryEntry *name;
> + int ret;
> +
> + av_bprint_init(bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> +
> + /* profile header */
> + name = av_dict_get(sd->metadata, "name", NULL, 0);
> + av_bprintf(bp, "%.79s", (name && name->value[0]) ? name->value : "icc");
> + av_bprint_chars(bp, 0, 2); /* terminating \0 and compression method */
> +
> + /* profile data */
> + if ((ret = encode_zbuf(bp, sd->data, sd->size))) {
> + av_bprint_finalize(bp, NULL);
> + return ret;
> + }
> +
> + return av_bprint_finalize(bp, buf_out);
1. This is not how should work with an AVBPrint -- you are throwing the
small-string optimization away here.
2. Using an AVBPrint with its dynamic reallocations is probably not good
here at all: It is easy to get a good upper bound via deflateBound()
which allows to omit the reallocations/the loop. (I should probably have
applied
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210317163202.672493-1-andreas.rheinha...@gmail.com/)
> +}
> +
> static int encode_headers(AVCodecContext *avctx, const AVFrame *pict)
> {
> AVFrameSideData *side_data;
> @@ -399,7 +459,22 @@ static int encode_headers(AVCodecContext *avctx, const
> AVFrame *pict)
> if (png_get_gama(pict->color_trc, s->buf))
> png_write_chunk(&s->bytestream, MKTAG('g', 'A', 'M', 'A'), s->buf,
> 4);
>
> - /* put the palette if needed */
> + side_data = av_frame_get_side_data(pict, AV_FRAME_DATA_ICC_PROFILE);
> + if (side_data && side_data->size) {
> + AVBPrint bp;
> + char *buf;
> + int ret;
> +
> + if ((ret = png_get_iccp(&bp, side_data, &buf))) {
> + av_log(avctx, AV_LOG_WARNING, "Failed writing iCCP chunk: %s\n",
> + av_err2str(ret));
3. You should error out in case of error.
> + } else {
> + png_write_chunk(&s->bytestream, MKTAG('i', 'C', 'C', 'P'), buf,
> bp.size);
4. The size of this chunk is not accounted for in the max_packet_size.
(You are lucky that the current estimate for max_packet_size is too
generous (the zstream is not reset/flushed for each row, so it should be
deflatebound(alldata) instead of height*deflatebound(data_from_one_row)).)
> + av_free(buf);
> + }
> + }
> +
> + /* put the palette if needed, must be after colorspace information */
> if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
> int has_alpha, alpha, i;
> unsigned int v;
> diff --git a/tests/fate/image.mak b/tests/fate/image.mak
> index 573d398915..da4f3709e9 100644
> --- a/tests/fate/image.mak
> +++ b/tests/fate/image.mak
> @@ -385,6 +385,9 @@ FATE_PNG_PROBE += fate-png-side-data
> fate-png-side-data: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_frames \
> -i $(TARGET_SAMPLES)/png1/lena-int_rgb24.png
>
> +FATE_PNG_PROBE += fate-png-icc
> +fate-png-icc: CMD = transcode png_pipe
> $(TARGET_SAMPLES)/png1/lena-int_rgb24.png image2 "-c png"
> +
> FATE_PNG-$(call DEMDEC, IMAGE2, PNG) += $(FATE_PNG)
> FATE_PNG_PROBE-$(call DEMDEC, IMAGE2, PNG) += $(FATE_PNG_PROBE)
> FATE_IMAGE += $(FATE_PNG-yes)
> diff --git a/tests/ref/fate/png-icc b/tests/ref/fate/png-icc
> new file mode 100644
> index 0000000000..d3cf55263e
> --- /dev/null
> +++ b/tests/ref/fate/png-icc
> @@ -0,0 +1,8 @@
> +7e412f6a9e2c7fcb674336e5c104518d *tests/data/fate/png-icc.image2
> +49398 tests/data/fate/png-icc.image2
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 128x128
> +#sar 0: 2835/2835
> +0, 0, 0, 1, 49152, 0xe0013dee
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
