diff mbox series

[FFmpeg-devel,v8,1/2] avcodec/libjxldec: properly tag output colorspace

Message ID 20220602021412.58306-2-leo.izen@gmail.com
State New
Headers show
Series libjxl Colorspace Fixes | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Leo Izen June 2, 2022, 2:14 a.m. UTC
Whether an ICC profile is present or not, the decoder
should now properly tag the colorspace of pixel data
received by the decoder.
---
 libavcodec/libjxldec.c | 142 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 133 insertions(+), 9 deletions(-)

Comments

Anton Khirnov June 23, 2022, 2:02 p.m. UTC | #1
Quoting Leo Izen (2022-06-02 04:14:11)
> Whether an ICC profile is present or not, the decoder
> should now properly tag the colorspace of pixel data
> received by the decoder.
> ---
>  libavcodec/libjxldec.c | 142 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 133 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
> index cd4bca3343..d2e0616124 100644
> --- a/libavcodec/libjxldec.c
> +++ b/libavcodec/libjxldec.c
> @@ -27,6 +27,7 @@
>  #include "libavutil/avassert.h"
>  #include "libavutil/buffer.h"
>  #include "libavutil/common.h"
> +#include "libavutil/csp.h"
>  #include "libavutil/error.h"
>  #include "libavutil/mem.h"
>  #include "libavutil/pixdesc.h"
> @@ -92,7 +93,7 @@ static av_cold int libjxl_decode_init(AVCodecContext *avctx)
>      return libjxl_init_jxl_decoder(avctx);
>  }
>  
> -static enum AVPixelFormat libjxl_get_pix_fmt(AVCodecContext *avctx, JxlBasicInfo *basic_info, JxlPixelFormat *format)
> +static enum AVPixelFormat libjxl_get_pix_fmt(void *avctx, const JxlBasicInfo *basic_info, JxlPixelFormat *format)
>  {
>      format->endianness = JXL_NATIVE_ENDIAN;
>      format->num_channels = basic_info->num_color_channels + (basic_info->alpha_bits > 0);
> @@ -129,12 +130,71 @@ static enum AVPixelFormat libjxl_get_pix_fmt(AVCodecContext *avctx, JxlBasicInfo
>      return AV_PIX_FMT_NONE;
>  }
>  
> +static enum AVColorPrimaries libjxl_get_primaries(void *avctx, const JxlColorEncoding *jxl_color)
> +{
> +    AVColorPrimariesDesc desc;
> +    enum AVColorPrimaries prim;
> +
> +    /* libjxl populates these double values even if it uses an enum space */
> +    desc.prim.r.x = av_d2q(jxl_color->primaries_red_xy[0], 300000);
> +    desc.prim.r.y = av_d2q(jxl_color->primaries_red_xy[1], 300000);
> +    desc.prim.g.x = av_d2q(jxl_color->primaries_green_xy[0], 300000);
> +    desc.prim.g.y = av_d2q(jxl_color->primaries_green_xy[1], 300000);
> +    desc.prim.b.x = av_d2q(jxl_color->primaries_blue_xy[0], 300000);
> +    desc.prim.b.y = av_d2q(jxl_color->primaries_blue_xy[1], 300000);
> +    desc.wp.x = av_d2q(jxl_color->white_point_xy[0], 300000);
> +    desc.wp.y = av_d2q(jxl_color->white_point_xy[1], 300000);
> +
> +    prim = av_csp_primaries_id_from_desc(&desc);
> +    if (prim == AVCOL_PRI_UNSPECIFIED) {
> +        /* try D65 with the same primaries */
> +        /* BT.709 uses D65 white point */
> +        const AVWhitepointCoefficients *d65 = &av_csp_primaries_desc_from_id(AVCOL_PRI_BT709)->wp;
> +        desc.wp = *d65;

Omitting the intermediate pointer seems simpler to me:
           desc.wp = av_csp_primaries_desc_from_id(AVCOL_TRC_BT709)->wp


> +        av_log(avctx, AV_LOG_WARNING, "Changing unknown white point to D65\n");
> +        prim = av_csp_primaries_id_from_desc(&desc);
> +    }
> +
> +    return prim;
> +}
> +
> +static enum AVColorTransferCharacteristic libjxl_get_trc(void *avctx, const JxlColorEncoding *jxl_color)
> +{
> +    switch (jxl_color->transfer_function) {
> +    case JXL_TRANSFER_FUNCTION_709:
> +        return AVCOL_TRC_BT709;
> +    case JXL_TRANSFER_FUNCTION_LINEAR:
> +        return AVCOL_TRC_LINEAR;
> +    case JXL_TRANSFER_FUNCTION_SRGB:
> +        return AVCOL_TRC_IEC61966_2_1;
> +    case JXL_TRANSFER_FUNCTION_PQ:
> +        return AVCOL_TRC_SMPTE2084;
> +    case JXL_TRANSFER_FUNCTION_DCI:
> +        return AVCOL_TRC_SMPTE428;
> +    case JXL_TRANSFER_FUNCTION_HLG:
> +        return AVCOL_TRC_ARIB_STD_B67;

cosmetics nit: it seems more readable to put the case and the return on
the same line in this case

> +    case JXL_TRANSFER_FUNCTION_GAMMA:
> +        if (jxl_color->gamma > 2.199 && jxl_color->gamma < 2.201)
> +            return AVCOL_TRC_GAMMA22;
> +        else if (jxl_color->gamma > 2.799 && jxl_color->gamma < 2.801)
> +            return AVCOL_TRC_GAMMA28;
> +        else
> +            av_log(avctx, AV_LOG_WARNING, "Unsupported gamma transfer: %f\n", jxl_color->gamma);
> +        break;
> +    default:
> +        av_log(avctx, AV_LOG_WARNING, "Unknown transfer function: %d\n", jxl_color->transfer_function);
> +    }
> +
> +    return AVCOL_TRC_UNSPECIFIED;
> +}
> +
>  static int libjxl_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_frame, AVPacket *avpkt)
>  {
>      LibJxlDecodeContext *ctx = avctx->priv_data;
>      uint8_t *buf = avpkt->data;
>      size_t remaining = avpkt->size, iccp_len;
>      JxlDecoderStatus jret;
> +    JxlColorEncoding jxl_color;
>      int ret;
>      *got_frame = 0;
>  
> @@ -189,16 +249,80 @@ static int libjxl_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_f
>              continue;
>          case JXL_DEC_COLOR_ENCODING:
>              av_log(avctx, AV_LOG_DEBUG, "COLOR_ENCODING event emitted\n");
> -            jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &iccp_len);
> -            if (jret == JXL_DEC_SUCCESS && iccp_len > 0) {
> -                av_buffer_unref(&ctx->iccp);
> -                ctx->iccp = av_buffer_alloc(iccp_len);
> -                if (!ctx->iccp)
> -                    return AVERROR(ENOMEM);
> -                jret = JxlDecoderGetColorAsICCProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, ctx->iccp->data, iccp_len);
> +            jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, NULL, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &jxl_color);
> +            if (jret == JXL_DEC_SUCCESS) {
> +                /* enum values describe the colors of this image */
> +                jret = JxlDecoderSetPreferredColorProfile(ctx->decoder, &jxl_color);
> +                if (jret == JXL_DEC_SUCCESS)
> +                    jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &jxl_color);
> +            } else {
> +                /* an ICC Profile is present in the stream */
> +                if (ctx->basic_info.uses_original_profile) {
> +                    av_log(avctx, AV_LOG_VERBOSE, "Using embedded ICC Profile\n");
> +                    /* an ICC profile is present, and we can meaningfully get it,
> +                     * because the pixel data is not XYB-encoded */
> +                    jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &iccp_len);
> +                    if (jret == JXL_DEC_SUCCESS && iccp_len > 0) {
> +                        av_buffer_unref(&ctx->iccp);
> +                        ctx->iccp = av_buffer_alloc(iccp_len);
> +                        if (!ctx->iccp)
> +                            return AVERROR(ENOMEM);
> +                        jret = JxlDecoderGetColorAsICCProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, ctx->iccp->data, iccp_len);
> +                        if (jret != JXL_DEC_SUCCESS) {
> +                            av_log(avctx, AV_LOG_WARNING, "Unable to obtain ICCP from header\n");
> +                            av_buffer_unref(&ctx->iccp);
> +                        }
> +                    }
> +                }
> +                /* when libjxl adds JxlDecoderSetICCProfile() properly handle the XYB case as well */
> +            }
> +
> +            avctx->color_range = frame->color_range = AVCOL_RANGE_JPEG;
> +            if (ctx->jxl_pixfmt.num_channels >= 3)
> +                avctx->colorspace = AVCOL_SPC_RGB;
> +            avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
> +            avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
> +
> +            if (!ctx->iccp) {
> +                /* checking enum values */
> +                if (jret == JXL_DEC_SUCCESS) {

This whole case: block is starting to look overly complex and would IMO
benefit from being moved to a separate function (or multiple. And the
control flow is starting to look complicated, e.g. it is not immediately
obvious which call does jret come from here.

Other than these "style" issues, I see no issues with the patch. Then
again I'm far from being an exper on colorspaces.
Niklas Haas June 24, 2022, 10:14 a.m. UTC | #2
On Thu, 23 Jun 2022 16:02:08 +0200 Anton Khirnov <anton@khirnov.net> wrote:
> Quoting Leo Izen (2022-06-02 04:14:11)
> > +    case JXL_TRANSFER_FUNCTION_GAMMA:
> > +        if (jxl_color->gamma > 2.199 && jxl_color->gamma < 2.201)
> > +            return AVCOL_TRC_GAMMA22;
> > +        else if (jxl_color->gamma > 2.799 && jxl_color->gamma < 2.801)
> > +            return AVCOL_TRC_GAMMA28;
> > +        else
> > +            av_log(avctx, AV_LOG_WARNING, "Unsupported gamma transfer: %f\n", jxl_color->gamma);
> > +        break;
> > +    default:
> > +        av_log(avctx, AV_LOG_WARNING, "Unknown transfer function: %d\n", jxl_color->transfer_function);
> > +    }
> > +
> > +    return AVCOL_TRC_UNSPECIFIED;
> > +}
> > +
> >  static int libjxl_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_frame, AVPacket *avpkt)
> >  {
> >      LibJxlDecodeContext *ctx = avctx->priv_data;
> >      uint8_t *buf = avpkt->data;
> >      size_t remaining = avpkt->size, iccp_len;
> >      JxlDecoderStatus jret;
> > +    JxlColorEncoding jxl_color;
> >      int ret;
> >      *got_frame = 0;
> >  
> > @@ -189,16 +249,80 @@ static int libjxl_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_f
> >              continue;
> >          case JXL_DEC_COLOR_ENCODING:
> >              av_log(avctx, AV_LOG_DEBUG, "COLOR_ENCODING event emitted\n");
> > -            jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &iccp_len);
> > -            if (jret == JXL_DEC_SUCCESS && iccp_len > 0) {
> > -                av_buffer_unref(&ctx->iccp);
> > -                ctx->iccp = av_buffer_alloc(iccp_len);
> > -                if (!ctx->iccp)
> > -                    return AVERROR(ENOMEM);
> > -                jret = JxlDecoderGetColorAsICCProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, ctx->iccp->data, iccp_len);
> > +            jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, NULL, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &jxl_color);
> > +            if (jret == JXL_DEC_SUCCESS) {
> > +                /* enum values describe the colors of this image */
> > +                jret = JxlDecoderSetPreferredColorProfile(ctx->decoder, &jxl_color);
> > +                if (jret == JXL_DEC_SUCCESS)
> > +                    jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &jxl_color);
> > +            } else {
> > +                /* an ICC Profile is present in the stream */
> > +                if (ctx->basic_info.uses_original_profile) {
> > +                    av_log(avctx, AV_LOG_VERBOSE, "Using embedded ICC Profile\n");
> > +                    /* an ICC profile is present, and we can meaningfully get it,
> > +                     * because the pixel data is not XYB-encoded */
> > +                    jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &iccp_len);
> > +                    if (jret == JXL_DEC_SUCCESS && iccp_len > 0) {
> > +                        av_buffer_unref(&ctx->iccp);
> > +                        ctx->iccp = av_buffer_alloc(iccp_len);
> > +                        if (!ctx->iccp)
> > +                            return AVERROR(ENOMEM);
> > +                        jret = JxlDecoderGetColorAsICCProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, ctx->iccp->data, iccp_len);
> > +                        if (jret != JXL_DEC_SUCCESS) {
> > +                            av_log(avctx, AV_LOG_WARNING, "Unable to obtain ICCP from header\n");
> > +                            av_buffer_unref(&ctx->iccp);
> > +                        }
> > +                    }
> > +                }
> > +                /* when libjxl adds JxlDecoderSetICCProfile() properly handle the XYB case as well */
> > +            }
> > +
> > +            avctx->color_range = frame->color_range = AVCOL_RANGE_JPEG;
> > +            if (ctx->jxl_pixfmt.num_channels >= 3)
> > +                avctx->colorspace = AVCOL_SPC_RGB;
> > +            avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
> > +            avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
> > +
> > +            if (!ctx->iccp) {
> > +                /* checking enum values */
> > +                if (jret == JXL_DEC_SUCCESS) {
> 
> This whole case: block is starting to look overly complex and would IMO
> benefit from being moved to a separate function (or multiple. And the
> control flow is starting to look complicated, e.g. it is not immediately
> obvious which call does jret come from here.
> 
> Other than these "style" issues, I see no issues with the patch. Then
> again I'm far from being an exper on colorspaces.

The logic looks correct to me but I needed an explanation of the control
flow on IRC to verify that fact. I think you should, at the very least,
document here explicitly the four cases you explained to me.

But ideally, I'd restructure the logic to make it fully explicit which
case is wanting to do what, rather than trying to be clever and ensuring
the control flow implicitly does the right thing.
diff mbox series

Patch

diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
index cd4bca3343..d2e0616124 100644
--- a/libavcodec/libjxldec.c
+++ b/libavcodec/libjxldec.c
@@ -27,6 +27,7 @@ 
 #include "libavutil/avassert.h"
 #include "libavutil/buffer.h"
 #include "libavutil/common.h"
+#include "libavutil/csp.h"
 #include "libavutil/error.h"
 #include "libavutil/mem.h"
 #include "libavutil/pixdesc.h"
@@ -92,7 +93,7 @@  static av_cold int libjxl_decode_init(AVCodecContext *avctx)
     return libjxl_init_jxl_decoder(avctx);
 }
 
-static enum AVPixelFormat libjxl_get_pix_fmt(AVCodecContext *avctx, JxlBasicInfo *basic_info, JxlPixelFormat *format)
+static enum AVPixelFormat libjxl_get_pix_fmt(void *avctx, const JxlBasicInfo *basic_info, JxlPixelFormat *format)
 {
     format->endianness = JXL_NATIVE_ENDIAN;
     format->num_channels = basic_info->num_color_channels + (basic_info->alpha_bits > 0);
@@ -129,12 +130,71 @@  static enum AVPixelFormat libjxl_get_pix_fmt(AVCodecContext *avctx, JxlBasicInfo
     return AV_PIX_FMT_NONE;
 }
 
+static enum AVColorPrimaries libjxl_get_primaries(void *avctx, const JxlColorEncoding *jxl_color)
+{
+    AVColorPrimariesDesc desc;
+    enum AVColorPrimaries prim;
+
+    /* libjxl populates these double values even if it uses an enum space */
+    desc.prim.r.x = av_d2q(jxl_color->primaries_red_xy[0], 300000);
+    desc.prim.r.y = av_d2q(jxl_color->primaries_red_xy[1], 300000);
+    desc.prim.g.x = av_d2q(jxl_color->primaries_green_xy[0], 300000);
+    desc.prim.g.y = av_d2q(jxl_color->primaries_green_xy[1], 300000);
+    desc.prim.b.x = av_d2q(jxl_color->primaries_blue_xy[0], 300000);
+    desc.prim.b.y = av_d2q(jxl_color->primaries_blue_xy[1], 300000);
+    desc.wp.x = av_d2q(jxl_color->white_point_xy[0], 300000);
+    desc.wp.y = av_d2q(jxl_color->white_point_xy[1], 300000);
+
+    prim = av_csp_primaries_id_from_desc(&desc);
+    if (prim == AVCOL_PRI_UNSPECIFIED) {
+        /* try D65 with the same primaries */
+        /* BT.709 uses D65 white point */
+        const AVWhitepointCoefficients *d65 = &av_csp_primaries_desc_from_id(AVCOL_PRI_BT709)->wp;
+        desc.wp = *d65;
+        av_log(avctx, AV_LOG_WARNING, "Changing unknown white point to D65\n");
+        prim = av_csp_primaries_id_from_desc(&desc);
+    }
+
+    return prim;
+}
+
+static enum AVColorTransferCharacteristic libjxl_get_trc(void *avctx, const JxlColorEncoding *jxl_color)
+{
+    switch (jxl_color->transfer_function) {
+    case JXL_TRANSFER_FUNCTION_709:
+        return AVCOL_TRC_BT709;
+    case JXL_TRANSFER_FUNCTION_LINEAR:
+        return AVCOL_TRC_LINEAR;
+    case JXL_TRANSFER_FUNCTION_SRGB:
+        return AVCOL_TRC_IEC61966_2_1;
+    case JXL_TRANSFER_FUNCTION_PQ:
+        return AVCOL_TRC_SMPTE2084;
+    case JXL_TRANSFER_FUNCTION_DCI:
+        return AVCOL_TRC_SMPTE428;
+    case JXL_TRANSFER_FUNCTION_HLG:
+        return AVCOL_TRC_ARIB_STD_B67;
+    case JXL_TRANSFER_FUNCTION_GAMMA:
+        if (jxl_color->gamma > 2.199 && jxl_color->gamma < 2.201)
+            return AVCOL_TRC_GAMMA22;
+        else if (jxl_color->gamma > 2.799 && jxl_color->gamma < 2.801)
+            return AVCOL_TRC_GAMMA28;
+        else
+            av_log(avctx, AV_LOG_WARNING, "Unsupported gamma transfer: %f\n", jxl_color->gamma);
+        break;
+    default:
+        av_log(avctx, AV_LOG_WARNING, "Unknown transfer function: %d\n", jxl_color->transfer_function);
+    }
+
+    return AVCOL_TRC_UNSPECIFIED;
+}
+
 static int libjxl_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_frame, AVPacket *avpkt)
 {
     LibJxlDecodeContext *ctx = avctx->priv_data;
     uint8_t *buf = avpkt->data;
     size_t remaining = avpkt->size, iccp_len;
     JxlDecoderStatus jret;
+    JxlColorEncoding jxl_color;
     int ret;
     *got_frame = 0;
 
@@ -189,16 +249,80 @@  static int libjxl_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_f
             continue;
         case JXL_DEC_COLOR_ENCODING:
             av_log(avctx, AV_LOG_DEBUG, "COLOR_ENCODING event emitted\n");
-            jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &iccp_len);
-            if (jret == JXL_DEC_SUCCESS && iccp_len > 0) {
-                av_buffer_unref(&ctx->iccp);
-                ctx->iccp = av_buffer_alloc(iccp_len);
-                if (!ctx->iccp)
-                    return AVERROR(ENOMEM);
-                jret = JxlDecoderGetColorAsICCProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, ctx->iccp->data, iccp_len);
+            jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, NULL, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &jxl_color);
+            if (jret == JXL_DEC_SUCCESS) {
+                /* enum values describe the colors of this image */
+                jret = JxlDecoderSetPreferredColorProfile(ctx->decoder, &jxl_color);
+                if (jret == JXL_DEC_SUCCESS)
+                    jret = JxlDecoderGetColorAsEncodedProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &jxl_color);
+            } else {
+                /* an ICC Profile is present in the stream */
+                if (ctx->basic_info.uses_original_profile) {
+                    av_log(avctx, AV_LOG_VERBOSE, "Using embedded ICC Profile\n");
+                    /* an ICC profile is present, and we can meaningfully get it,
+                     * because the pixel data is not XYB-encoded */
+                    jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, &iccp_len);
+                    if (jret == JXL_DEC_SUCCESS && iccp_len > 0) {
+                        av_buffer_unref(&ctx->iccp);
+                        ctx->iccp = av_buffer_alloc(iccp_len);
+                        if (!ctx->iccp)
+                            return AVERROR(ENOMEM);
+                        jret = JxlDecoderGetColorAsICCProfile(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_DATA, ctx->iccp->data, iccp_len);
+                        if (jret != JXL_DEC_SUCCESS) {
+                            av_log(avctx, AV_LOG_WARNING, "Unable to obtain ICCP from header\n");
+                            av_buffer_unref(&ctx->iccp);
+                        }
+                    }
+                }
+                /* when libjxl adds JxlDecoderSetICCProfile() properly handle the XYB case as well */
+            }
+
+            avctx->color_range = frame->color_range = AVCOL_RANGE_JPEG;
+            if (ctx->jxl_pixfmt.num_channels >= 3)
+                avctx->colorspace = AVCOL_SPC_RGB;
+            avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
+            avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
+
+            if (!ctx->iccp) {
+                /* checking enum values */
+                if (jret == JXL_DEC_SUCCESS) {
+                    if (avctx->colorspace == AVCOL_SPC_RGB)
+                        avctx->color_primaries = libjxl_get_primaries(avctx, &jxl_color);
+                    avctx->color_trc = libjxl_get_trc(avctx, &jxl_color);
+                }
+                /* fall back on wide gamut if enum values fail */
+                if (avctx->color_primaries == AVCOL_PRI_UNSPECIFIED) {
+                    if (avctx->colorspace == AVCOL_SPC_RGB) {
+                        av_log(avctx, AV_LOG_WARNING, "Falling back on wide gamut output\n");
+                        jxl_color.primaries = JXL_PRIMARIES_2100;
+                        avctx->color_primaries = AVCOL_PRI_BT2020;
+                    }
+                    /* libjxl requires this set even for grayscale */
+                    jxl_color.white_point = JXL_WHITE_POINT_D65;
+                }
+                if (avctx->color_trc == AVCOL_TRC_UNSPECIFIED) {
+                    if (ctx->jxl_pixfmt.data_type == JXL_TYPE_FLOAT
+                        || ctx->jxl_pixfmt.data_type == JXL_TYPE_FLOAT16) {
+                        av_log(avctx, AV_LOG_WARNING, "Falling back on Linear Light transfer\n");
+                        jxl_color.transfer_function = JXL_TRANSFER_FUNCTION_LINEAR;
+                        avctx->color_trc = AVCOL_TRC_LINEAR;
+                    } else {
+                        av_log(avctx, AV_LOG_WARNING, "Falling back on iec61966-2-1/sRGB transfer\n");
+                        jxl_color.transfer_function = JXL_TRANSFER_FUNCTION_SRGB;
+                        avctx->color_trc = AVCOL_TRC_IEC61966_2_1;
+                    }
+                }
+                /* all colors will be in-gamut so we want accurate colors */
+                jxl_color.rendering_intent = JXL_RENDERING_INTENT_RELATIVE;
+                jxl_color.color_space = avctx->colorspace == AVCOL_SPC_RGB ? JXL_COLOR_SPACE_RGB : JXL_COLOR_SPACE_GRAY;
+                jret = JxlDecoderSetPreferredColorProfile(ctx->decoder, &jxl_color);
                 if (jret != JXL_DEC_SUCCESS)
-                    av_buffer_unref(&ctx->iccp);
+                    av_log(avctx, AV_LOG_WARNING, "Unable to set fallback color encoding\n");
             }
+
+            frame->color_trc = avctx->color_trc;
+            frame->color_primaries = avctx->color_primaries;
+            frame->colorspace = avctx->colorspace;
             continue;
         case JXL_DEC_NEED_IMAGE_OUT_BUFFER:
             av_log(avctx, AV_LOG_DEBUG, "NEED_IMAGE_OUT_BUFFER event emitted\n");