[FFmpeg-devel] [PATCH] Add a gamma flag to exr loader to avoid banding
Michael Niedermayer
michaelni at gmx.at
Wed May 7 01:31:02 CEST 2014
On Tue, May 06, 2014 at 04:30:16PM -0300, Gonzalo Garramuno wrote:
> On 05/05/14 16:47, Michael Niedermayer wrote:
> >On Mon, May 05, 2014 at 12:52:25PM -0300, Gonzalo Garramuno wrote:
> >>On 05/05/14 12:28, Michael Niedermayer wrote:
> >>>it seems to fail only with --enable-libopencv
> >>That's beyond my knowledge. So you got the fate tests to pass?
> >its basically the same as before, they pass on some platforms with
> >some configure options but fail on others.
> I replaced the exr_half2float() with one that does not work any
> magic with floating point (it works all in uint).
> Can you give it a try? Attached is the exr diff and the checksums
> which remain the same.
>
[...]
> @@ -77,6 +81,7 @@ typedef struct EXRThreadData {
> uint16_t *lut;
> } EXRThreadData;
>
> +
> typedef struct EXRContext {
> AVClass *class;
> AVFrame *picture;
unrelated
> @@ -106,8 +111,83 @@ typedef struct EXRContext {
> EXRThreadData *thread_data;
>
> const char *layer;
> +
> + float gamma;
> +
> + uint16_t gamma_table[65536];
> +
> } EXRContext;
>
> +// -15 stored using a single precision bias of 127
> +const unsigned int HALF_FLOAT_MIN_BIASED_EXP_AS_SINGLE_FP_EXP = 0x38000000;
> +// max exponent value in single precision that will be converted
> +// to Inf or Nan when stored as a half-float
> +const unsigned int HALF_FLOAT_MAX_BIASED_EXP_AS_SINGLE_FP_EXP = 0x47800000;
> +
> +// 255 is the max exponent biased value
> +const unsigned int FLOAT_MAX_BIASED_EXP = (0xFF << 23);
> +
> +const unsigned int HALF_FLOAT_MAX_BIASED_EXP = (0x1F << 10);
these should be #define ...
> +
> +/*
> + * Convert a half float as a uint16_t into a full float.
> + *
> + * @param h half float as uint16_t
> + *
> + * @return float value
> + */
> +inline static float
doesnt need to be inline
> +exr_half2float(uint16_t hf)
> +{
> + unsigned int sign = (unsigned int)(hf >> 15);
> + unsigned int mantissa = (unsigned int)(hf & ((1 << 10) - 1));
> + unsigned int exp = (unsigned int)(hf & HALF_FLOAT_MAX_BIASED_EXP);
> + unsigned int f;
> +
> + if (exp == HALF_FLOAT_MAX_BIASED_EXP)
> + {
> + // we have a half-float NaN or Inf
> + // half-float NaNs will be converted to a single precision NaN
> + // half-float Infs will be converted to a single precision Inf
> + exp = FLOAT_MAX_BIASED_EXP;
> + if (mantissa)
> + mantissa = (1 << 23) - 1; // set all bits to indicate a NaN
> + }
> + else if (exp == 0x0)
> + {
> + // convert half-float zero/denorm to single precision value
> + if (mantissa)
> + {
> + mantissa <<= 1;
> + exp = HALF_FLOAT_MIN_BIASED_EXP_AS_SINGLE_FP_EXP;
> + // check for leading 1 in denorm mantissa
> + while ((mantissa & (1 << 10)) == 0)
> + {
> + // for every leading 0, decrement single precision exponent by 1
> + // and shift half-float mantissa value to the left
> + mantissa <<= 1;
> + exp -= (1 << 23);
> + }
> + // clamp the mantissa to 10-bits
this could probably be simplified with av_log2(), not sure its worth
teh work considering this isnt speed relevant though
also please keep the formating consistent within a file
> + mantissa &= ((1 << 10) - 1);
> + // shift left to generate single-precision mantissa of 23-bits
> + mantissa <<= 13;
> + }
> + }
> + else
> + {
> + // shift left to generate single-precision mantissa of 23-bits
> + mantissa <<= 13;
> + // generate single precision biased exponent value
> + exp = (exp << 13) + HALF_FLOAT_MIN_BIASED_EXP_AS_SINGLE_FP_EXP;
> + }
> +
> + f = (sign << 31) | exp | mantissa;
> +
> + return *((float *)&f);
aliasing violation, undefined behavior
also it would be better if it outputted some kind of int
and float is limited to the pow() codepath
> +}
> +
> +
> /**
> * Convert from 32-bit float as uint32_t to uint16_t.
> *
> @@ -128,6 +208,8 @@ static inline uint16_t exr_flt2uint(uint32_t v)
> return (v + (1 << 23)) >> (127 + 7 - exp);
> }
>
> +
> +
> /**
> * Convert from 16-bit float as uint16_t to uint16_t.
> *
unrelated
and this one seems to pass here for the case that failed previously
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140507/d801d98a/attachment.asc>
More information about the ffmpeg-devel
mailing list