diff mbox series

[FFmpeg-devel] avcodec/libjxldec: fix gamma22 and gamma28 recognization

Message ID 20230118142405.23568-1-leo.izen@gmail.com
State Accepted
Commit 9d5e66942c5bae578926e29efebec348199798df
Headers show
Series [FFmpeg-devel] avcodec/libjxldec: fix gamma22 and gamma28 recognization | expand

Commit Message

Leo Izen Jan. 18, 2023, 2:24 p.m. UTC 
Gamma 2.2 and Gamma 2.8 are tagged in the file as 0.45455 and 0.35714,
respectively (i.e. 1/2.2 and 1/2.8). Trying to identify them as 2.2 and
2.8 instead of these values will cause the transfer function to not
properly be recognized. This patch fixes this.
---
 libavcodec/libjxldec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jan Ekström Jan. 20, 2023, 10:15 p.m. UTC | #1 
On Wed, Jan 18, 2023 at 4:24 PM Leo Izen <leo.izen@gmail.com> wrote:
>
> Gamma 2.2 and Gamma 2.8 are tagged in the file as 0.45455 and 0.35714,
> respectively (i.e. 1/2.2 and 1/2.8). Trying to identify them as 2.2 and
> 2.8 instead of these values will cause the transfer function to not
> properly be recognized. This patch fixes this.

Agreed upon reading the spec, which notes that the value is saved as
an unsigned 24bit integer consisting of the numerator part of a
rational number, with a 10^7 denumerator and a valid range of (0, 1].

Only nit: recognization -> recognition

> ---
>  libavcodec/libjxldec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
> index 55c7366f61..0009af9615 100644
> --- a/libavcodec/libjxldec.c
> +++ b/libavcodec/libjxldec.c
> @@ -167,9 +167,9 @@ static enum AVColorTransferCharacteristic libjxl_get_trc(void *avctx, const JxlC
>      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)
> +        if (jxl_color->gamma > 0.45355 && jxl_color->gamma < 0.45555)
>              return AVCOL_TRC_GAMMA22;
> -        else if (jxl_color->gamma > 2.799 && jxl_color->gamma < 2.801)
> +        else if (jxl_color->gamma > 0.35614 && jxl_color->gamma < 0.35814)

These values leave ~0.001 of leeway which seems like it was before.

LGTM
Jan Ekström Jan. 22, 2023, 3:20 p.m. UTC | #2 
On Sat, Jan 21, 2023 at 12:15 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Wed, Jan 18, 2023 at 4:24 PM Leo Izen <leo.izen@gmail.com> wrote:
> >
> > Gamma 2.2 and Gamma 2.8 are tagged in the file as 0.45455 and 0.35714,
> > respectively (i.e. 1/2.2 and 1/2.8). Trying to identify them as 2.2 and
> > 2.8 instead of these values will cause the transfer function to not
> > properly be recognized. This patch fixes this.
>
> Agreed upon reading the spec, which notes that the value is saved as
> an unsigned 24bit integer consisting of the numerator part of a
> rational number, with a 10^7 denumerator and a valid range of (0, 1].
>
> Only nit: recognization -> recognition
>
> > ---
> >  libavcodec/libjxldec.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
> > index 55c7366f61..0009af9615 100644
> > --- a/libavcodec/libjxldec.c
> > +++ b/libavcodec/libjxldec.c
> > @@ -167,9 +167,9 @@ static enum AVColorTransferCharacteristic libjxl_get_trc(void *avctx, const JxlC
> >      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)
> > +        if (jxl_color->gamma > 0.45355 && jxl_color->gamma < 0.45555)
> >              return AVCOL_TRC_GAMMA22;
> > -        else if (jxl_color->gamma > 2.799 && jxl_color->gamma < 2.801)
> > +        else if (jxl_color->gamma > 0.35614 && jxl_color->gamma < 0.35814)
>
> These values leave ~0.001 of leeway which seems like it was before.
>
> LGTM

Applied to master as 9d5e66942c5bae578926e29efebec348199798df with the
typo fix, as well as back-ported to release/5.1.

Jan
diff mbox series

Patch

diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
index 55c7366f61..0009af9615 100644
--- a/libavcodec/libjxldec.c
+++ b/libavcodec/libjxldec.c
@@ -167,9 +167,9 @@  static enum AVColorTransferCharacteristic libjxl_get_trc(void *avctx, const JxlC
     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)
+        if (jxl_color->gamma > 0.45355 && jxl_color->gamma < 0.45555)
             return AVCOL_TRC_GAMMA22;
-        else if (jxl_color->gamma > 2.799 && jxl_color->gamma < 2.801)
+        else if (jxl_color->gamma > 0.35614 && jxl_color->gamma < 0.35814)
             return AVCOL_TRC_GAMMA28;
         else
             av_log(avctx, AV_LOG_WARNING, "Unsupported gamma transfer: %f\n", jxl_color->gamma);