| Message ID | 20230202150938.22297-2-leo.izen@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PNG color tagging improvements | expand |
| 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 |
On 2/2/23 10:09, Leo Izen wrote: > 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. > Andreas has an issue with one of the other patches, but as this first one is just a fix I'll apply it soon if there's no objections. - Leo Izen (thebombzen)
On 2/7/23 14:17, Leo Izen wrote: > On 2/2/23 10:09, Leo Izen wrote: >> 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. >> > > Andreas has an issue with one of the other patches, but as this first > one is just a fix I'll apply it soon if there's no objections. > > - Leo Izen (thebombzen) Pushed just this one patch. The other two are still open for review. - Leo Izen (thebombzen)
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;
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(-)