| 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 | expand |
| 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 |
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". >
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". >> >
Thanks. There is another path that sets it correctly. Please apply this. The code could be refactored later.
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
patch welcome
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 --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;