[FFmpeg-devel,v12,03/14] lavc/tiff: Convert DNGs to sRGB color space

Submitted by velocityra@gmail.com on Aug. 9, 2019, 4:29 p.m.

Details

Message ID 20190809162959.17924-3-velocityra@gmail.com
State New
Headers show

Commit Message

velocityra@gmail.com Aug. 9, 2019, 4:29 p.m.
From: Nick Renieris <velocityra@gmail.com>

Signed-off-by: Nick Renieris <velocityra@gmail.com>
---
 libavcodec/tiff.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

Comments

Michael Niedermayer Aug. 10, 2019, 12:42 p.m.
On Fri, Aug 09, 2019 at 07:29:48PM +0300, Nick Renieris wrote:
> From: Nick Renieris <velocityra@gmail.com>
> 
> Signed-off-by: Nick Renieris <velocityra@gmail.com>
> ---
>  libavcodec/tiff.c | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index d5673abb19..a118c37c41 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -731,14 +731,23 @@ static int tiff_unpack_strip(TiffContext *s, AVFrame *p, uint8_t *dst, int strid
>      return 0;
>  }
>  
> +static float av_always_inline linear_to_srgb(float value) {
> +    if (value <= 0.0031308)
> +        return value * 12.92;
> +    else
> +        return pow(value * 1.055, 1.0 / 2.4) - 0.055;
> +}
> +
>  /**
> - * Map stored raw sensor values into linear reference values.
> - * See: DNG Specification - Chapter 5
> + * Map stored raw sensor values into linear reference values (see: DNG Specification - Chapter 5)
> + * Then convert to sRGB color space.
>   */
> -static uint16_t av_always_inline dng_raw_to_linear16(uint16_t value,
> -                                                    const uint16_t *lut,
> -                                                    uint16_t black_level,
> -                                                    float scale_factor) {
> +static uint16_t av_always_inline dng_process_color16(uint16_t value,
> +                                                     const uint16_t *lut,
> +                                                     uint16_t black_level,
> +                                                     float scale_factor) {
> +    float value_norm;
> +
>      // Lookup table lookup
>      if (lut)
>          value = lut[value];
> @@ -747,16 +756,19 @@ static uint16_t av_always_inline dng_raw_to_linear16(uint16_t value,
>      value = av_clip_uint16_c((unsigned)value - black_level);
>  
>      // Color scaling
> -    value = av_clip_uint16_c((unsigned)(((float)value * scale_factor) * 0xFFFF));
> +    value_norm = (float)value * scale_factor;
> +
> +    // Color space conversion (sRGB)
> +    value = av_clip_uint16_c((uint16_t)(linear_to_srgb(value_norm) * 0xFFFF));
>  
>      return value;
>  }

Why do you put all this color space convertion code into the decoders ?

Colorspace and pixel format convertion is generally done outside decoders.

Thanks


[...]
Paul B Mahol Aug. 10, 2019, 1:06 p.m.
On Sat, Aug 10, 2019 at 2:42 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Fri, Aug 09, 2019 at 07:29:48PM +0300, Nick Renieris wrote:
> > From: Nick Renieris <velocityra@gmail.com>
> >
> > Signed-off-by: Nick Renieris <velocityra@gmail.com>
> > ---
> >  libavcodec/tiff.c | 34 +++++++++++++++++++++++-----------
> >  1 file changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> > index d5673abb19..a118c37c41 100644
> > --- a/libavcodec/tiff.c
> > +++ b/libavcodec/tiff.c
> > @@ -731,14 +731,23 @@ static int tiff_unpack_strip(TiffContext *s,
> AVFrame *p, uint8_t *dst, int strid
> >      return 0;
> >  }
> >
> > +static float av_always_inline linear_to_srgb(float value) {
> > +    if (value <= 0.0031308)
> > +        return value * 12.92;
> > +    else
> > +        return pow(value * 1.055, 1.0 / 2.4) - 0.055;
> > +}
> > +
> >  /**
> > - * Map stored raw sensor values into linear reference values.
> > - * See: DNG Specification - Chapter 5
> > + * Map stored raw sensor values into linear reference values (see: DNG
> Specification - Chapter 5)
> > + * Then convert to sRGB color space.
> >   */
> > -static uint16_t av_always_inline dng_raw_to_linear16(uint16_t value,
> > -                                                    const uint16_t *lut,
> > -                                                    uint16_t
> black_level,
> > -                                                    float scale_factor)
> {
> > +static uint16_t av_always_inline dng_process_color16(uint16_t value,
> > +                                                     const uint16_t
> *lut,
> > +                                                     uint16_t
> black_level,
> > +                                                     float
> scale_factor) {
> > +    float value_norm;
> > +
> >      // Lookup table lookup
> >      if (lut)
> >          value = lut[value];
> > @@ -747,16 +756,19 @@ static uint16_t av_always_inline
> dng_raw_to_linear16(uint16_t value,
> >      value = av_clip_uint16_c((unsigned)value - black_level);
> >
> >      // Color scaling
> > -    value = av_clip_uint16_c((unsigned)(((float)value * scale_factor) *
> 0xFFFF));
> > +    value_norm = (float)value * scale_factor;
> > +
> > +    // Color space conversion (sRGB)
> > +    value = av_clip_uint16_c((uint16_t)(linear_to_srgb(value_norm) *
> 0xFFFF));
> >
> >      return value;
> >  }
>
> Why do you put all this color space convertion code into the decoders ?
>
>
It is part of DNG specification. Do you want to not follow DNG
specification?



> Colorspace and pixel format convertion is generally done outside decoders.
>
> Thanks
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> What does censorship reveal? It reveals fear. -- Julian Assange
> _______________________________________________
> 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".
Nicolas George Aug. 10, 2019, 1:43 p.m.
Paul B Mahol (12019-08-10):
> It is part of DNG specification. Do you want to not follow DNG
> specification?

The specification cannot specify what part of FFmpeg's code does the
conversion.
velocityra@gmail.com Aug. 10, 2019, 1:46 p.m.
> Why do you put all this color space convertion code into the decoders ?

The color space conversion needs to happen, camera sensor data is
linear. If it's not done images looks really dark as result.

> Colorspace and pixel format convertion is generally done outside decoders.
> The specification cannot specify what part of FFmpeg's code does the
conversion.

I had found some sRGB conversion code in libswscale, asked about using
it and was told not to.
It's only 4 simple lines, as opposed to probably more complicated code
to use libswscale or whatever.

Στις Σάβ, 10 Αυγ 2019 στις 4:43 μ.μ., ο/η Nicolas George
<george@nsup.org> έγραψε:
>
> Paul B Mahol (12019-08-10):
> > It is part of DNG specification. Do you want to not follow DNG
> > specification?
>
> The specification cannot specify what part of FFmpeg's code does the
> conversion.
>
> --
>   Nicolas George
> _______________________________________________
> 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".

Patch hide | download patch | download mbox

diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index d5673abb19..a118c37c41 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -731,14 +731,23 @@  static int tiff_unpack_strip(TiffContext *s, AVFrame *p, uint8_t *dst, int strid
     return 0;
 }
 
+static float av_always_inline linear_to_srgb(float value) {
+    if (value <= 0.0031308)
+        return value * 12.92;
+    else
+        return pow(value * 1.055, 1.0 / 2.4) - 0.055;
+}
+
 /**
- * Map stored raw sensor values into linear reference values.
- * See: DNG Specification - Chapter 5
+ * Map stored raw sensor values into linear reference values (see: DNG Specification - Chapter 5)
+ * Then convert to sRGB color space.
  */
-static uint16_t av_always_inline dng_raw_to_linear16(uint16_t value,
-                                                    const uint16_t *lut,
-                                                    uint16_t black_level,
-                                                    float scale_factor) {
+static uint16_t av_always_inline dng_process_color16(uint16_t value,
+                                                     const uint16_t *lut,
+                                                     uint16_t black_level,
+                                                     float scale_factor) {
+    float value_norm;
+
     // Lookup table lookup
     if (lut)
         value = lut[value];
@@ -747,16 +756,19 @@  static uint16_t av_always_inline dng_raw_to_linear16(uint16_t value,
     value = av_clip_uint16_c((unsigned)value - black_level);
 
     // Color scaling
-    value = av_clip_uint16_c((unsigned)(((float)value * scale_factor) * 0xFFFF));
+    value_norm = (float)value * scale_factor;
+
+    // Color space conversion (sRGB)
+    value = av_clip_uint16_c((uint16_t)(linear_to_srgb(value_norm) * 0xFFFF));
 
     return value;
 }
 
-static uint16_t av_always_inline dng_raw_to_linear8(uint16_t value,
+static uint16_t av_always_inline dng_process_color8(uint16_t value,
                                                     const uint16_t *lut,
                                                     uint16_t black_level,
                                                     float scale_factor) {
-    return dng_raw_to_linear16(value, lut, black_level, scale_factor) >> 8;
+    return dng_process_color16(value, lut, black_level, scale_factor) >> 8;
 }
 
 static void dng_blit(TiffContext *s, uint8_t *dst, int dst_stride,
@@ -774,7 +786,7 @@  static void dng_blit(TiffContext *s, uint8_t *dst, int dst_stride,
             uint16_t *src_u16 = (uint16_t *)src;
 
             for (col = 0; col < width; col++)
-                *dst_u16++ = dng_raw_to_linear16(*src_u16++, s->dng_lut, s->black_level, scale_factor);
+                *dst_u16++ = dng_process_color16(*src_u16++, s->dng_lut, s->black_level, scale_factor);
 
             dst += dst_stride * sizeof(uint16_t);
             src += src_stride * sizeof(uint16_t);
@@ -782,7 +794,7 @@  static void dng_blit(TiffContext *s, uint8_t *dst, int dst_stride,
     } else {
         for (line = 0; line < height; line++) {
             for (col = 0; col < width; col++)
-                *dst++ = dng_raw_to_linear8(*src++, s->dng_lut, s->black_level, scale_factor);
+                *dst++ = dng_process_color8(*src++, s->dng_lut, s->black_level, scale_factor);
 
             dst += dst_stride;
             src += src_stride;