diff mbox series

[FFmpeg-devel] avfilter/vf_zscale: fix output color_range discrepancy

Message ID 20210917021716.19055-1-pkoshevoy@gmail.com
State Accepted
Commit 4fc0b75973d20425df22a9178fc2b735710a5f40
Headers show
Series [FFmpeg-devel] avfilter/vf_zscale: fix output color_range discrepancy
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Pavel Koshevoy Sept. 17, 2021, 2:17 a.m. UTC
This filter chain was supposed to convert from narrow range
to full range yuv444p, but didn't:

buffer=width=1280:height=720:pix_fmt=yuv444p:frame_rate=25/1:\
time_base=1/25:sar=1/1,zscale=min=709:rin=limited:pin=709:\
tin=709:t=linear,format=gbrpf32le,zscale=tin=linear:p=709:m=709:\
r=full:t=709,format=pix_fmts=yuv444p,buffersink
---
 libavfilter/vf_zscale.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paul B Mahol Sept. 17, 2021, 6:45 a.m. UTC | #1
I doubt this is correct, something is fishy here.

On Fri, Sep 17, 2021 at 4:17 AM Pavel Koshevoy <pkoshevoy@gmail.com> wrote:

> This filter chain was supposed to convert from narrow range
> to full range yuv444p, but didn't:
>
> buffer=width=1280:height=720:pix_fmt=yuv444p:frame_rate=25/1:\
> time_base=1/25:sar=1/1,zscale=min=709:rin=limited:pin=709:\
> tin=709:t=linear,format=gbrpf32le,zscale=tin=linear:p=709:m=709:\
> r=full:t=709,format=pix_fmts=yuv444p,buffersink
> ---
>  libavfilter/vf_zscale.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
> index dfea00f9eb..06a025e6e6 100644
> --- a/libavfilter/vf_zscale.c
> +++ b/libavfilter/vf_zscale.c
> @@ -676,7 +676,7 @@ static int filter_frame(AVFilterLink *link, AVFrame
> *in)
>          out->color_primaries = (int)s->dst_format.color_primaries;
>
>      if (s->range != -1)
> -        out->color_range = (int)s->dst_format.pixel_range;
> +        out->color_range = (int)s->dst_format.pixel_range + 1;
>
>      if (s->trc != -1)
>          out->color_trc = (int)s->dst_format.transfer_characteristics;
> --
> 2.26.2
>
> _______________________________________________
> 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".
>
Pavel Koshevoy Sept. 17, 2021, 5:51 p.m. UTC | #2
On Fri, Sep 17, 2021 at 12:45 AM Paul B Mahol <onemda@gmail.com> wrote:

> I doubt this is correct, something is fishy here.
>

The issue is that you are assigning a value of type zimg_color_family_e to
a variable of type AVColorRange.

typedef enum zimg_pixel_range_e {
ZIMG_RANGE_INTERNAL = -1, /**< Not part of the API. */
ZIMG_RANGE_LIMITED  = 0,  /**< Studio (TV) legal range, 16-235 in 8 bits. */
ZIMG_RANGE_FULL     = 1   /**< Full (PC) dynamic range, 0-255 in 8 bits. */
} zimg_pixel_range_e;

enum AVColorRange {
    AVCOL_RANGE_UNSPECIFIED = 0,
    AVCOL_RANGE_MPEG        = 1,
    AVCOL_RANGE_JPEG        = 2,
    AVCOL_RANGE_NB               ///< Not part of ABI
};

As you can see, (int)ZIMG_RANGE_FULL is not the same as
(int)AVCOL_RANGE_JPEG.
And you've apparently had run into this once before, 6 years ago:

$ git annotate libavfilter/vf_zscale.c | grep 'color_range' | grep out
b5cf307d        (Paul B Mahol   2016-09-02 16:03:58 +0200       580)
|| s->out_range != out->color_range
416e35e5        (Paul B Mahol   2015-09-21 15:34:15 +0200       620)
     out->color_range = (int)s->dst_format.pixel_range + 1;
416e35e5        (Paul B Mahol   2015-09-21 15:34:15 +0200       640)
 s->out_range      = out->color_range;
416e35e5        (Paul B Mahol   2015-09-21 15:34:15 +0200       679)
 out->color_range = (int)s->dst_format.pixel_range;


I hope this clears your doubt.

Thank you,
    Pavel.



>
> On Fri, Sep 17, 2021 at 4:17 AM Pavel Koshevoy <pkoshevoy@gmail.com>
> wrote:
>
>> This filter chain was supposed to convert from narrow range
>> to full range yuv444p, but didn't:
>>
>> buffer=width=1280:height=720:pix_fmt=yuv444p:frame_rate=25/1:\
>> time_base=1/25:sar=1/1,zscale=min=709:rin=limited:pin=709:\
>> tin=709:t=linear,format=gbrpf32le,zscale=tin=linear:p=709:m=709:\
>> r=full:t=709,format=pix_fmts=yuv444p,buffersink
>> ---
>>  libavfilter/vf_zscale.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
>> index dfea00f9eb..06a025e6e6 100644
>> --- a/libavfilter/vf_zscale.c
>> +++ b/libavfilter/vf_zscale.c
>> @@ -676,7 +676,7 @@ static int filter_frame(AVFilterLink *link, AVFrame
>> *in)
>>          out->color_primaries = (int)s->dst_format.color_primaries;
>>
>>      if (s->range != -1)
>> -        out->color_range = (int)s->dst_format.pixel_range;
>> +        out->color_range = (int)s->dst_format.pixel_range + 1;
>>
>>      if (s->trc != -1)
>>          out->color_trc = (int)s->dst_format.transfer_characteristics;
>> --
>> 2.26.2
>>
>> _______________________________________________
>> 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 Sept. 17, 2021, 6:38 p.m. UTC | #3
Thanks. There is another path that sets it correctly. Please apply this.
The code could be refactored later.
Andreas Rheinhardt Sept. 17, 2021, 6:43 p.m. UTC | #4
Paul B Mahol:
> Thanks. There is another path that sets it correctly. Please apply this.
> The code could be refactored later.

Shouldn't one use something like convert_range() (but for converting
back) instead of hardcoding the assumption that ZIMG_RANGE_* + 1 =
AVCOL_RANGE_* for matching ranges?

- Andreas
Paul B Mahol Sept. 17, 2021, 6:45 p.m. UTC | #5
patch welcome
Pavel Koshevoy Sept. 17, 2021, 7:39 p.m. UTC | #6
On Fri, Sep 17, 2021 at 12:38 PM Paul B Mahol <onemda@gmail.com> wrote:

>
> Thanks. There is another path that sets it correctly. Please apply this.
> The code could be refactored later.
>


Applied and pushed.

Thank you,
    Pavel.

(sending 2nd time, to the list this time)
diff mbox series

Patch

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index dfea00f9eb..06a025e6e6 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -676,7 +676,7 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
         out->color_primaries = (int)s->dst_format.color_primaries;
 
     if (s->range != -1)
-        out->color_range = (int)s->dst_format.pixel_range;
+        out->color_range = (int)s->dst_format.pixel_range + 1;
 
     if (s->trc != -1)
         out->color_trc = (int)s->dst_format.transfer_characteristics;