[FFmpeg-devel] [PATCH] avcodec/png{dec, enc}: update mDCV and cLLI chunk capitalization

[Alexander Strasser]

Hi Leo!

On 2024-12-01 09:20 -0500, Leo Izen wrote:
> The PNGv3 Specification Draft [1] has changed the capitalization
> of mDCV and cLLI chunks (formerly mDCv and cLLi). This patch updates
> FFmpeg to work with the new chunk names while retaining decode-side
> compatibility with files created using the old names.
>
> [1]: https://w3c.github.io/png/
>
> Signed-off-by: Leo Izen <leo.izen at gmail.com>
> ---
>  libavcodec/pngdec.c     | 12 +++++++-----
>  libavcodec/pngenc.c     |  4 ++--
>  tests/ref/fate/png-mdcv |  2 +-
>  3 files changed, 10 insertions(+), 8 deletions(-)

LGTM.

Your changes are sensible and complete as far as I could check.

Only problem I could think of is being to fast, as it isn't
in the latest published version of the W3C yet.

If I understood chunk naming conventions correctly they marked
those 2 chunks unsafe to copy, which will probably stay that
way. So I think the patch can be pushed.


Best regards,
  Alexander

> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> index c5b32c166d..f8cb61775e 100644
> --- a/libavcodec/pngdec.c
> +++ b/libavcodec/pngdec.c
> @@ -757,7 +757,7 @@ static int populate_avctx_color_fields(AVCodecContext *avctx, AVFrame *frame)
>          if (clli) {
>              /*
>               * 0.0001 divisor value
> -             * see: https://www.w3.org/TR/png-3/#cLLi-chunk
> +             * see: https://www.w3.org/TR/png-3/#cLLI-chunk
>               */
>              clli->MaxCLL = s->clli_max / 10000;
>              clli->MaxFALL = s->clli_avg / 10000;
> @@ -1566,18 +1566,20 @@ static int decode_frame_common(AVCodecContext *avctx, PNGDecContext *s,
>
>              break;
>          }
> -        case MKTAG('c', 'L', 'L', 'i'):
> +        case MKTAG('c', 'L', 'L', 'i'): /* legacy spelling, for backwards compat */
> +        case MKTAG('c', 'L', 'L', 'I'):
>              if (bytestream2_get_bytes_left(&gb_chunk) != 8) {
> -                av_log(avctx, AV_LOG_WARNING, "Invalid cLLi chunk size: %d\n", bytestream2_get_bytes_left(&gb_chunk));
> +                av_log(avctx, AV_LOG_WARNING, "Invalid cLLI chunk size: %d\n", bytestream2_get_bytes_left(&gb_chunk));
>                  break;
>              }
>              s->have_clli = 1;
>              s->clli_max = bytestream2_get_be32u(&gb_chunk);
>              s->clli_avg = bytestream2_get_be32u(&gb_chunk);
>              break;
> -        case MKTAG('m', 'D', 'C', 'v'):
> +        case MKTAG('m', 'D', 'C', 'v'): /* legacy spelling, for backward compat */
> +        case MKTAG('m', 'D', 'C', 'V'):
>              if (bytestream2_get_bytes_left(&gb_chunk) != 24) {
> -                av_log(avctx, AV_LOG_WARNING, "Invalid mDCv chunk size: %d\n", bytestream2_get_bytes_left(&gb_chunk));
> +                av_log(avctx, AV_LOG_WARNING, "Invalid mDCV chunk size: %d\n", bytestream2_get_bytes_left(&gb_chunk));
>                  break;
>              }
>              s->have_mdcv = 1;
> diff --git a/libavcodec/pngenc.c b/libavcodec/pngenc.c
> index cb79c04e11..37e8d5bfcf 100644
> --- a/libavcodec/pngenc.c
> +++ b/libavcodec/pngenc.c
> @@ -445,7 +445,7 @@ static int encode_headers(AVCodecContext *avctx, const AVFrame *pict)
>          AVContentLightMetadata *clli = (AVContentLightMetadata *) side_data->data;
>          AV_WB32(s->buf, clli->MaxCLL * 10000);
>          AV_WB32(s->buf + 4, clli->MaxFALL * 10000);
> -        png_write_chunk(&s->bytestream, MKTAG('c', 'L', 'L', 'i'), s->buf, 8);
> +        png_write_chunk(&s->bytestream, MKTAG('c', 'L', 'L', 'I'), s->buf, 8);
>      }
>
>      side_data = av_frame_get_side_data(pict, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
> @@ -460,7 +460,7 @@ static int encode_headers(AVCodecContext *avctx, const AVFrame *pict)
>              AV_WB16(s->buf + 14, PNG_Q2D(mdcv->white_point[1], 50000));
>              AV_WB32(s->buf + 16, PNG_Q2D(mdcv->max_luminance, 10000));
>              AV_WB32(s->buf + 20, PNG_Q2D(mdcv->min_luminance, 10000));
> -            png_write_chunk(&s->bytestream, MKTAG('m', 'D', 'C', 'v'), s->buf, 24);
> +            png_write_chunk(&s->bytestream, MKTAG('m', 'D', 'C', 'V'), s->buf, 24);
>          }
>      }
>
> diff --git a/tests/ref/fate/png-mdcv b/tests/ref/fate/png-mdcv
> index c524a94ded..4328c56349 100644
> --- a/tests/ref/fate/png-mdcv
> +++ b/tests/ref/fate/png-mdcv
> @@ -1,4 +1,4 @@
> -fc68fe6c8c72343b96d2695f6913995b *tests/data/fate/png-mdcv.image2pipe
> +b1837f5557ad969a3f9763840480d5c0 *tests/data/fate/png-mdcv.image2pipe
>  439248 tests/data/fate/png-mdcv.image2pipe
>  #tb 0: 1/25
>  #media_type 0: video
> --