[FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[James Almer]

Regression since 0e917389fe73c932049635d947bba076f1709589.
This change also reverts commit 431805c096738da7661a0baee2b12fe9724dcc95.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/exr.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 4482f104d0..107281e115 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -42,6 +42,7 @@
 #include "libavutil/avstring.h"
 #include "libavutil/mem.h"
 #include "libavutil/opt.h"
+#include "libavutil/float2half.h"
 #include "libavutil/half2float.h"
 
 #include "avcodec.h"
@@ -193,7 +194,10 @@ typedef struct EXRContext {
 
     uint8_t *offset_table;
 
+    uint16_t gamma_table[65536];
+
     Half2FloatTables h2f_tables;
+    Float2HalfTables f2h_tables;
 } EXRContext;
 
 static int zip_uncompress(const EXRContext *s, const uint8_t *src, int compressed_size,
@@ -1401,8 +1405,13 @@ static int decode_block(AVCodecContext *avctx, void *tdata,
                     }
                 } else if (s->pixel_type == EXR_HALF) {
                     // 16-bit
-                    for (int x = 0; x < xsize; x++, ptr_x += step)
-                        AV_WN16A(ptr_x, bytestream_get_le16(&src));
+                    if (trc_func && (!c || (c < 3 && s->desc->flags & AV_PIX_FMT_FLAG_PLANAR))) {
+                        for (int x = 0; x < xsize; x++, ptr_x += step)
+                            AV_WN16A(ptr_x, s->gamma_table[bytestream_get_le16(&src)]);
+                    } else {
+                        for (int x = 0; x < xsize; x++, ptr_x += step)
+                            AV_WN16A(ptr_x, bytestream_get_le16(&src));
+                    }
                 }
 
                 // Zero out the end if xmax+1 is not w
@@ -2212,8 +2221,13 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *picture,
 static av_cold int decode_init(AVCodecContext *avctx)
 {
     EXRContext *s = avctx->priv_data;
+    uint32_t i;
+    union av_intfloat32 t;
+    float one_gamma = 1.0f / s->gamma;
+    av_csp_trc_function trc_func = NULL;
 
     ff_init_half2float_tables(&s->h2f_tables);
+    ff_init_float2half_tables(&s->f2h_tables);
 
     s->avctx              = avctx;
 
@@ -2223,6 +2237,26 @@ static av_cold int decode_init(AVCodecContext *avctx)
     ff_bswapdsp_init(&s->bbdsp);
 #endif
 
+    trc_func = av_csp_trc_func_from_id(s->apply_trc_type);
+    if (trc_func) {
+        for (i = 0; i < 65536; ++i) {
+            t.i = half2float(i, &s->h2f_tables);
+            t.f = trc_func(t.f);
+            s->gamma_table[i] = float2half(av_float2int(t.f), &s->f2h_tables);
+        }
+    } else if (one_gamma != 1.0f) {
+        for (i = 0; i < 65536; ++i) {
+            t.i = half2float(i, &s->h2f_tables);
+            /* If negative value we reuse half value */
+            if (t.f <= 0.0f) {
+                s->gamma_table[i] = i;
+            } else {
+                t.f = powf(t.f, one_gamma);
+                s->gamma_table[i] = float2half(t.i, &s->f2h_tables);
+            }
+        }
+    }
+
     // allocate thread data, used for non EXR_RAW compression types
     s->thread_data = av_calloc(avctx->thread_count, sizeof(*s->thread_data));
     if (!s->thread_data)
-- 
2.48.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 949 bytes --]

Hi

On Thu, Mar 13, 2025 at 11:08:09PM -0300, James Almer wrote:
> Regression since 0e917389fe73c932049635d947bba076f1709589.
> This change also reverts commit 431805c096738da7661a0baee2b12fe9724dcc95.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/exr.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)

We dont do that for any other codec.
For every other case we export all the color and gamma information
but do not modify the sample values to some "normalized" representation

Its also lossy to do it in the decoder like this

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 810 bytes --]

On 3/15/2025 8:35 PM, Michael Niedermayer wrote:
> Hi
> 
> On Thu, Mar 13, 2025 at 11:08:09PM -0300, James Almer wrote:
>> Regression since 0e917389fe73c932049635d947bba076f1709589.
>> This change also reverts commit 431805c096738da7661a0baee2b12fe9724dcc95.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/exr.c | 38 ++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 36 insertions(+), 2 deletions(-)
> 
> We dont do that for any other codec.
> For every other case we export all the color and gamma information
> but do not modify the sample values to some "normalized" representation
> 
> Its also lossy to do it in the decoder like this
> 
> thx
We're still doing it for the float path, so in that case you may want to 
remove it from there too.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1237 bytes --]

Hi

On Sat, Mar 15, 2025 at 08:36:33PM -0300, James Almer wrote:
> On 3/15/2025 8:35 PM, Michael Niedermayer wrote:
> > Hi
> > 
> > On Thu, Mar 13, 2025 at 11:08:09PM -0300, James Almer wrote:
> > > Regression since 0e917389fe73c932049635d947bba076f1709589.
> > > This change also reverts commit 431805c096738da7661a0baee2b12fe9724dcc95.
> > > 
> > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > ---
> > >   libavcodec/exr.c | 38 ++++++++++++++++++++++++++++++++++++--
> > >   1 file changed, 36 insertions(+), 2 deletions(-)
> > 
> > We dont do that for any other codec.
> > For every other case we export all the color and gamma information
> > but do not modify the sample values to some "normalized" representation
> > 
> > Its also lossy to do it in the decoder like this
> > 
> > thx
> We're still doing it for the float path, so in that case you may want to
> remove it from there too.

I think we should remove it, but I think I have no samples for that code path.
So i would not be able to test ...

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1499 bytes --]

On 3/15/2025 8:42 PM, Michael Niedermayer wrote:
> Hi
> 
> On Sat, Mar 15, 2025 at 08:36:33PM -0300, James Almer wrote:
>> On 3/15/2025 8:35 PM, Michael Niedermayer wrote:
>>> Hi
>>>
>>> On Thu, Mar 13, 2025 at 11:08:09PM -0300, James Almer wrote:
>>>> Regression since 0e917389fe73c932049635d947bba076f1709589.
>>>> This change also reverts commit 431805c096738da7661a0baee2b12fe9724dcc95.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavcodec/exr.c | 38 ++++++++++++++++++++++++++++++++++++--
>>>>    1 file changed, 36 insertions(+), 2 deletions(-)
>>>
>>> We dont do that for any other codec.
>>> For every other case we export all the color and gamma information
>>> but do not modify the sample values to some "normalized" representation
>>>
>>> Its also lossy to do it in the decoder like this
>>>
>>> thx
>> We're still doing it for the float path, so in that case you may want to
>> remove it from there too.
> 
> I think we should remove it, but I think I have no samples for that code path.
> So i would not be able to test ...
> 
> thx

Even with no samples it should be a straightforward change.
But even then, since this feature includes two AVOptions that would be 
broken after such removal (and in fact currently are for half float 
samples), we should IMO deprecate them and remove the feature once the 
options are gone. That includes applying this patch to restore the 
functionality with half float samples.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/exrdec: restore applying color transfer characteristics for float16 samples

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1967 bytes --]

On Sat, Mar 15, 2025 at 09:09:08PM -0300, James Almer wrote:
> On 3/15/2025 8:42 PM, Michael Niedermayer wrote:
> > Hi
> > 
> > On Sat, Mar 15, 2025 at 08:36:33PM -0300, James Almer wrote:
> > > On 3/15/2025 8:35 PM, Michael Niedermayer wrote:
> > > > Hi
> > > > 
> > > > On Thu, Mar 13, 2025 at 11:08:09PM -0300, James Almer wrote:
> > > > > Regression since 0e917389fe73c932049635d947bba076f1709589.
> > > > > This change also reverts commit 431805c096738da7661a0baee2b12fe9724dcc95.
> > > > > 
> > > > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > > > ---
> > > > >    libavcodec/exr.c | 38 ++++++++++++++++++++++++++++++++++++--
> > > > >    1 file changed, 36 insertions(+), 2 deletions(-)
> > > > 
> > > > We dont do that for any other codec.
> > > > For every other case we export all the color and gamma information
> > > > but do not modify the sample values to some "normalized" representation
> > > > 
> > > > Its also lossy to do it in the decoder like this
> > > > 
> > > > thx
> > > We're still doing it for the float path, so in that case you may want to
> > > remove it from there too.
> > 
> > I think we should remove it, but I think I have no samples for that code path.
> > So i would not be able to test ...
> > 
> > thx
> 
> Even with no samples it should be a straightforward change.
> But even then, since this feature includes two AVOptions that would be
> broken after such removal (and in fact currently are for half float
> samples), we should IMO deprecate them and remove the feature once the
> options are gone. That includes applying this patch to restore the
> functionality with half float samples.

sure, feel free to restore it and deprecate them
if thats whatr people prefer

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What is kyc? Its a tool that makes you give out your real ID, while criminals
give out a forged ID card.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avcodec/exr: deprecate gamma and apply_trc options

[James Almer]

Decoders should not modify sample values, as that's the job of a library like swscale.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/exr.c           | 38 ++++++++++++++++++++++++++------------
 libavcodec/version_major.h |  1 +
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index e1e05902c6..0bd9ce9ebe 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -189,15 +189,17 @@ typedef struct EXRContext {
     const char *layer;
     int selected_part;
 
-    enum AVColorTransferCharacteristic apply_trc_type;
-    float gamma;
 
     uint8_t *offset_table;
 
+#if FF_API_EXR_GAMMA
+    enum AVColorTransferCharacteristic apply_trc_type;
+    float gamma;
     uint16_t gamma_table[65536];
+    Float2HalfTables f2h_tables;
+#endif
 
     Half2FloatTables h2f_tables;
-    Float2HalfTables f2h_tables;
 } EXRContext;
 
 static int zip_uncompress(const EXRContext *s, const uint8_t *src, int compressed_size,
@@ -1194,8 +1196,10 @@ static int decode_block(AVCodecContext *avctx, void *tdata,
     int data_xoffset, data_yoffset, data_window_offset, xsize, ysize;
     int i, x, buf_size = s->buf_size;
     int c, rgb_channel_count;
+#if FF_API_EXR_GAMMA
     float one_gamma = 1.0f / s->gamma;
     av_csp_trc_function trc_func = av_csp_trc_func_from_id(s->apply_trc_type);
+#endif
     int ret;
 
     line_offset = AV_RL64(s->gb.buffer + jobnr * 8);
@@ -1387,6 +1391,7 @@ static int decode_block(AVCodecContext *avctx, void *tdata,
                     s->compression == EXR_DWAA ||
                     s->compression == EXR_DWAB) {
                     // 32-bit
+#if FF_API_EXR_GAMMA
                     if (trc_func && (!c || (c < 3 && s->desc->flags & AV_PIX_FMT_FLAG_PLANAR))) {
                         for (int x = 0; x < xsize; x++, ptr_x += step) {
                             float f = av_int2float(bytestream_get_le32(&src));
@@ -1399,19 +1404,20 @@ static int decode_block(AVCodecContext *avctx, void *tdata,
                                 f = powf(f, one_gamma);
                             AV_WN32A(ptr_x, av_float2int(f));
                         }
-                    } else {
+                    } else
+#endif
                         for (int x = 0; x < xsize; x++, ptr_x += step)
                             AV_WN32A(ptr_x, bytestream_get_le32(&src));
-                    }
                 } else if (s->pixel_type == EXR_HALF) {
                     // 16-bit
+#if FF_API_EXR_GAMMA
                     if (one_gamma != 1.f || (trc_func && (!c || (c < 3 && s->desc->flags & AV_PIX_FMT_FLAG_PLANAR)))) {
                         for (int x = 0; x < xsize; x++, ptr_x += step)
                             AV_WN16A(ptr_x, s->gamma_table[bytestream_get_le16(&src)]);
-                    } else {
+                    } else
+#endif
                         for (int x = 0; x < xsize; x++, ptr_x += step)
                             AV_WN16A(ptr_x, bytestream_get_le16(&src));
-                    }
                 }
 
                 // Zero out the end if xmax+1 is not w
@@ -2093,10 +2099,12 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *picture,
         return AVERROR_INVALIDDATA;
     }
 
+#if FF_API_EXR_GAMMA
     if (s->apply_trc_type != AVCOL_TRC_UNSPECIFIED)
         avctx->color_trc = s->apply_trc_type;
     else if (s->gamma > 0.9999f && s->gamma < 1.0001f)
         avctx->color_trc = AVCOL_TRC_LINEAR;
+#endif
 
     switch (s->compression) {
     case EXR_RAW:
@@ -2221,13 +2229,15 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *picture,
 static av_cold int decode_init(AVCodecContext *avctx)
 {
     EXRContext *s = avctx->priv_data;
+#if FF_API_EXR_GAMMA
     uint32_t i;
     union av_intfloat32 t;
     float one_gamma = 1.0f / s->gamma;
     av_csp_trc_function trc_func = NULL;
+    ff_init_float2half_tables(&s->f2h_tables);
+#endif
 
     ff_init_half2float_tables(&s->h2f_tables);
-    ff_init_float2half_tables(&s->f2h_tables);
 
     s->avctx              = avctx;
 
@@ -2237,6 +2247,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
     ff_bswapdsp_init(&s->bbdsp);
 #endif
 
+#if FF_API_EXR_GAMMA
     trc_func = av_csp_trc_func_from_id(s->apply_trc_type);
     if (trc_func) {
         for (i = 0; i < 65536; ++i) {
@@ -2256,6 +2267,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
             }
         }
     }
+#endif
 
     // allocate thread data, used for non EXR_RAW compression types
     s->thread_data = av_calloc(avctx->thread_count, sizeof(*s->thread_data));
@@ -2298,12 +2310,13 @@ static const AVOption options[] = {
         AV_OPT_TYPE_STRING, { .str = "" }, 0, 0, VD },
     { "part",  "Set the decoding part", OFFSET(selected_part),
         AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VD },
-    { "gamma", "Set the float gamma value when decoding", OFFSET(gamma),
-        AV_OPT_TYPE_FLOAT, { .dbl = 1.0f }, 0.001, FLT_MAX, VD },
+#if FF_API_EXR_GAMMA
+    { "gamma", "Set the float gamma value when decoding (deprecated, use a scaler)", OFFSET(gamma),
+        AV_OPT_TYPE_FLOAT, { .dbl = 1.0f }, 0.001, FLT_MAX, VD | AV_OPT_FLAG_DEPRECATED },
 
     // XXX: Note the abuse of the enum using AVCOL_TRC_UNSPECIFIED to subsume the existing gamma option
-    { "apply_trc", "color transfer characteristics to apply to EXR linear input", OFFSET(apply_trc_type),
-        AV_OPT_TYPE_INT, {.i64 = AVCOL_TRC_UNSPECIFIED }, 1, AVCOL_TRC_NB-1, VD, .unit = "apply_trc_type"},
+    { "apply_trc", "color transfer characteristics to apply to EXR linear input (deprecated, use a scaler)", OFFSET(apply_trc_type),
+        AV_OPT_TYPE_INT, {.i64 = AVCOL_TRC_UNSPECIFIED }, 1, AVCOL_TRC_NB-1, VD | AV_OPT_FLAG_DEPRECATED, .unit = "apply_trc_type"},
     { "bt709",        "BT.709",           0,
         AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_BT709 },        INT_MIN, INT_MAX, VD, .unit = "apply_trc_type"},
     { "gamma",        "gamma",            0,
@@ -2336,6 +2349,7 @@ static const AVOption options[] = {
         AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_SMPTEST2084 },  INT_MIN, INT_MAX, VD, .unit = "apply_trc_type"},
     { "smpte428_1",   "SMPTE ST 428-1",   0,
         AV_OPT_TYPE_CONST, {.i64 = AVCOL_TRC_SMPTEST428_1 }, INT_MIN, INT_MAX, VD, .unit = "apply_trc_type"},
+#endif
 
     { NULL },
 };
diff --git a/libavcodec/version_major.h b/libavcodec/version_major.h
index 35df5eb779..55e79d5505 100644
--- a/libavcodec/version_major.h
+++ b/libavcodec/version_major.h
@@ -50,6 +50,7 @@
 #define FF_API_QUALITY_FACTOR      (LIBAVCODEC_VERSION_MAJOR < 62)
 #define FF_API_V408_CODECID        (LIBAVCODEC_VERSION_MAJOR < 62)
 #define FF_API_CODEC_PROPS         (LIBAVCODEC_VERSION_MAJOR < 63)
+#define FF_API_EXR_GAMMA           (LIBAVCODEC_VERSION_MAJOR < 63)
 
 // reminder to remove the OMX encoder on next major bump
 #define FF_CODEC_OMX               (LIBAVCODEC_VERSION_MAJOR < 62)
-- 
2.48.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".