| Message ID | 20190809162959.17924-3-velocityra@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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 [...]
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".
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.
> 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".
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;