diff mbox series

[FFmpeg-devel,1/2] avcodec/pngenc.c: avoid writing cICP when inappropriate

Message ID 20230201171227.57864-2-leo.izen@gmail.com
State New
Headers show
Series PNG cICP improvements | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Leo Izen Feb. 1, 2023, 5:12 p.m. UTC
We parse the fallback cHRM on decode and correctly determine that we
have BT.709 primaries, but unknown TRC. This causes us to write cICP
where we shouldn't. Primaries without transfer can be handled entirely
by cHRM, so we should only write cICP if we actually know the transfer
function.

Additionally, we should avoid writing cICP if there's an ICC profile
because the spec says decoders must prioritize cICP over the ICC
profile.

Signed-off-by: Leo Izen <leo.izen@gmail.com>
---
 libavcodec/pngenc.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
index 2393161c3b..81b95c143d 100644
--- a/libavcodec/pngenc.c
+++ b/libavcodec/pngenc.c
@@ -412,14 +412,25 @@  static int encode_headers(AVCodecContext *avctx, const AVFrame *pict)
         }
     }
 
+    side_data = av_frame_get_side_data(pict, AV_FRAME_DATA_ICC_PROFILE);
+    if ((ret = png_write_iccp(s, side_data)))
+        return ret;
+
     /* write colorspace information */
     if (pict->color_primaries == AVCOL_PRI_BT709 &&
         pict->color_trc == AVCOL_TRC_IEC61966_2_1) {
         s->buf[0] = 1; /* rendering intent, relative colorimetric by default */
         png_write_chunk(&s->bytestream, MKTAG('s', 'R', 'G', 'B'), s->buf, 1);
-    } else if (pict->color_primaries != AVCOL_PRI_UNSPECIFIED ||
-        pict->color_trc != AVCOL_TRC_UNSPECIFIED) {
-        /* these values match H.273 so no translation is needed */
+    } else if (pict->color_trc != AVCOL_TRC_UNSPECIFIED && !side_data) {
+        /*
+         * Avoid writing cICP if the transfer is unknown. Known primaries
+         * with unknown transfer can be handled by cHRM.
+         *
+         * We also avoid writing cICP if an ICC Profile is present, because
+         * the standard requires that cICP overrides iCCP.
+         *
+         * These values match H.273 so no translation is needed.
+         */
         s->buf[0] = pict->color_primaries;
         s->buf[1] = pict->color_trc;
         s->buf[2] = 0; /* colorspace = RGB */
@@ -432,10 +443,6 @@  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);
 
-    side_data = av_frame_get_side_data(pict, AV_FRAME_DATA_ICC_PROFILE);
-    if ((ret = png_write_iccp(s, side_data)))
-        return ret;
-
     /* put the palette if needed, must be after colorspace information */
     if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
         int has_alpha, alpha, i;