[FFmpeg-devel] [PATCH] libavcodec/exr: fix incorrect translation of denorm mantissa

[Mark Reid]

On Mon, Sep 14, 2020 at 7:14 PM <mindmark at gmail.com> wrote:

> From: Mark Reid <mindmark at gmail.com>
>
> Hi,
> This fixes a very subtle error thats hard notice until up unpremultiply a
> image.
>
> This loop is suppose to stop at first 1, instead was stoping at first 0
> The comment is correct through!
>
> openexrs implementation is very similar.
>
> https://github.com/AcademySoftwareFoundation/openexr/blob/master/IlmBase/Half/toFloat.cpp#L85
>
> not all the exr tests needed to be fixed because only some have denorm
> values
>
> ---
>  libavcodec/exr.c                               | 2 +-
>  tests/ref/fate/exr-rgba-multiscanline-half-b44 | 2 +-
>  tests/ref/fate/exr-slice-raw                   | 2 +-
>  tests/ref/fate/exr-slice-rle                   | 2 +-
>  tests/ref/fate/exr-slice-zip1                  | 2 +-
>  tests/ref/fate/exr-slice-zip16                 | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> index 829d38143d..216d216785 100644
> --- a/libavcodec/exr.c
> +++ b/libavcodec/exr.c
> @@ -201,7 +201,7 @@ static union av_intfloat32 exr_half2float(uint16_t hf)
>              mantissa <<= 1;
>              exp = HALF_FLOAT_MIN_BIASED_EXP_AS_SINGLE_FP_EXP;
>              // check for leading 1 in denorm mantissa
> -            while ((mantissa & (1 << 10))) {
> +            while (!(mantissa & (1 << 10))) {
>                  // for every leading 0, decrement single precision
> exponent by 1
>                  // and shift half-float mantissa value to the left
>                  mantissa <<= 1;
> diff --git a/tests/ref/fate/exr-rgba-multiscanline-half-b44
> b/tests/ref/fate/exr-rgba-multiscanline-half-b44
> index 964bf2e65e..24525b92a7 100644
> --- a/tests/ref/fate/exr-rgba-multiscanline-half-b44
> +++ b/tests/ref/fate/exr-rgba-multiscanline-half-b44
> @@ -3,4 +3,4 @@
>  #codec_id 0: rawvideo
>  #dimensions 0: 935x251
>  #sar 0: 1/1
> -0,          0,          0,        1,  3754960, 0x4d48a1b2
> +0,          0,          0,        1,  3754960, 0x8d9af112
> diff --git a/tests/ref/fate/exr-slice-raw b/tests/ref/fate/exr-slice-raw
> index c7096e4d2a..1e7d3825ea 100644
> --- a/tests/ref/fate/exr-slice-raw
> +++ b/tests/ref/fate/exr-slice-raw
> @@ -3,4 +3,4 @@
>  #codec_id 0: rawvideo
>  #dimensions 0: 587x675
>  #sar 0: 1/1
> -0,          0,          0,        1,  6339600, 0x4f2b496b
> +0,          0,          0,        1,  6339600, 0xda3e31df
> diff --git a/tests/ref/fate/exr-slice-rle b/tests/ref/fate/exr-slice-rle
> index c7096e4d2a..1e7d3825ea 100644
> --- a/tests/ref/fate/exr-slice-rle
> +++ b/tests/ref/fate/exr-slice-rle
> @@ -3,4 +3,4 @@
>  #codec_id 0: rawvideo
>  #dimensions 0: 587x675
>  #sar 0: 1/1
> -0,          0,          0,        1,  6339600, 0x4f2b496b
> +0,          0,          0,        1,  6339600, 0xda3e31df
> diff --git a/tests/ref/fate/exr-slice-zip1 b/tests/ref/fate/exr-slice-zip1
> index c7096e4d2a..1e7d3825ea 100644
> --- a/tests/ref/fate/exr-slice-zip1
> +++ b/tests/ref/fate/exr-slice-zip1
> @@ -3,4 +3,4 @@
>  #codec_id 0: rawvideo
>  #dimensions 0: 587x675
>  #sar 0: 1/1
> -0,          0,          0,        1,  6339600, 0x4f2b496b
> +0,          0,          0,        1,  6339600, 0xda3e31df
> diff --git a/tests/ref/fate/exr-slice-zip16
> b/tests/ref/fate/exr-slice-zip16
> index c7096e4d2a..1e7d3825ea 100644
> --- a/tests/ref/fate/exr-slice-zip16
> +++ b/tests/ref/fate/exr-slice-zip16
> @@ -3,4 +3,4 @@
>  #codec_id 0: rawvideo
>  #dimensions 0: 587x675
>  #sar 0: 1/1
> -0,          0,          0,        1,  6339600, 0x4f2b496b
> +0,          0,          0,        1,  6339600, 0xda3e31df
> --
> 2.27.0
>
>
to see the problem
ffmpeg_g -y -i premult_test.exr -vf unpremultiply=inplace=1 out.jpg

here is that test file, and a before and after patch pic

https://www.dropbox.com/s/pgdxzxo454r95jn/premult_test.zip