| Message ID | 20220303120645.13076-1-martin@martin.st |
|---|---|
| State | Accepted |
| Commit | e645a1ddb90a863e129108aad9aa7e2d417f3615 |
| Headers | show |
| Series | [FFmpeg-devel] libavfilter: vf_scale: Properly take in->color_range into account | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On Thu, Mar 3, 2022 at 1:07 PM Martin Storsjö <martin@martin.st> wrote: > While swscale can be reconfigured with sws_setColorspaceDetails, > the in/out ranges also need to be set before calling > sws_init_context, otherwise the initialization might choose > fastpaths that don't take the ranges into account. > > Therefore, look at in->color_range too, when deciding on whether > the scaler needs to be reconfigured. > > Add a new member variable for keeping track of this, for being > able to differentiate between whether the scale filter parameter > "in_range" has been set (which should override whatever the input > frame has set) or whether it has been configured based on the > latest frame (which should trigger reconfiguring the scaler if > the input frame ranges change). > > Signed-off-by: Martin Storsjö <martin@martin.st> > --- > Tested by me to resolve https://trac.ffmpeg.org/ticket/9576. Thanks Martin! All the best, Dee
On Thu, Mar 03, 2022 at 02:06:45PM +0200, Martin Storsjö wrote: > While swscale can be reconfigured with sws_setColorspaceDetails, > the in/out ranges also need to be set before calling > sws_init_context, otherwise the initialization might choose > fastpaths that don't take the ranges into account. > > Therefore, look at in->color_range too, when deciding on whether > the scaler needs to be reconfigured. > > Add a new member variable for keeping track of this, for being > able to differentiate between whether the scale filter parameter > "in_range" has been set (which should override whatever the input > frame has set) or whether it has been configured based on the > latest frame (which should trigger reconfiguring the scaler if > the input frame ranges change). > > Signed-off-by: Martin Storsjö <martin@martin.st> > --- > To test this (without risking running many conflicting swscale > filters in one filter pipeline), we'd need to be able to tag > the incoming raw yuv data with colorspace and range without setting > the in_color_matrix and in_range options on the scale filter. > > When using the rawvideo demuxer, the pixel format is set via the > ffmpeg -pix_fmt option, but there's no corresponding option for > setting color matrix or range for it. > --- > libavfilter/vf_scale.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) This changes the output for: ffmpeg -i tickets/524/AVCI50.mov -vframes 3 file-avci50dec.nut ffmpeg -i tickets/4493/AVCI100.mov -vframes 3 file-avci100dec.nut https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket524/ Is that intended ? - 233903 file-avci100dec.nut - 383853 file-avci50dec.nut + 196558 file-avci100dec.nut + 333893 file-avci50dec.nut thx [...]
On Fri, 4 Mar 2022, Michael Niedermayer wrote: > On Thu, Mar 03, 2022 at 02:06:45PM +0200, Martin Storsjö wrote: >> While swscale can be reconfigured with sws_setColorspaceDetails, >> the in/out ranges also need to be set before calling >> sws_init_context, otherwise the initialization might choose >> fastpaths that don't take the ranges into account. >> >> Therefore, look at in->color_range too, when deciding on whether >> the scaler needs to be reconfigured. >> >> Add a new member variable for keeping track of this, for being >> able to differentiate between whether the scale filter parameter >> "in_range" has been set (which should override whatever the input >> frame has set) or whether it has been configured based on the >> latest frame (which should trigger reconfiguring the scaler if >> the input frame ranges change). >> >> Signed-off-by: Martin Storsjö <martin@martin.st> >> --- >> To test this (without risking running many conflicting swscale >> filters in one filter pipeline), we'd need to be able to tag >> the incoming raw yuv data with colorspace and range without setting >> the in_color_matrix and in_range options on the scale filter. >> >> When using the rawvideo demuxer, the pixel format is set via the >> ffmpeg -pix_fmt option, but there's no corresponding option for >> setting color matrix or range for it. >> --- >> libavfilter/vf_scale.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) > > This changes the output for: > ffmpeg -i tickets/524/AVCI50.mov -vframes 3 file-avci50dec.nut > ffmpeg -i tickets/4493/AVCI100.mov -vframes 3 file-avci100dec.nut > > https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket524/ > > Is that intended ? > > - 233903 file-avci100dec.nut > - 383853 file-avci50dec.nut > + 196558 file-avci100dec.nut > + 333893 file-avci50dec.nut Looks like these source files have full range content; for any data with full range input, this patch makes sure it uses the right intended conversion through swscale. So yes, I guess it's expected that these conversions change. // Martin
On Sat, Mar 05, 2022 at 11:33:15PM +0200, Martin Storsjö wrote: > On Fri, 4 Mar 2022, Michael Niedermayer wrote: > > > On Thu, Mar 03, 2022 at 02:06:45PM +0200, Martin Storsjö wrote: > > > While swscale can be reconfigured with sws_setColorspaceDetails, > > > the in/out ranges also need to be set before calling > > > sws_init_context, otherwise the initialization might choose > > > fastpaths that don't take the ranges into account. > > > > > > Therefore, look at in->color_range too, when deciding on whether > > > the scaler needs to be reconfigured. > > > > > > Add a new member variable for keeping track of this, for being > > > able to differentiate between whether the scale filter parameter > > > "in_range" has been set (which should override whatever the input > > > frame has set) or whether it has been configured based on the > > > latest frame (which should trigger reconfiguring the scaler if > > > the input frame ranges change). > > > > > > Signed-off-by: Martin Storsjö <martin@martin.st> > > > --- > > > To test this (without risking running many conflicting swscale > > > filters in one filter pipeline), we'd need to be able to tag > > > the incoming raw yuv data with colorspace and range without setting > > > the in_color_matrix and in_range options on the scale filter. > > > > > > When using the rawvideo demuxer, the pixel format is set via the > > > ffmpeg -pix_fmt option, but there's no corresponding option for > > > setting color matrix or range for it. > > > --- > > > libavfilter/vf_scale.c | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > This changes the output for: > > ffmpeg -i tickets/524/AVCI50.mov -vframes 3 file-avci50dec.nut > > ffmpeg -i tickets/4493/AVCI100.mov -vframes 3 file-avci100dec.nut > > > > https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket524/ > > > > Is that intended ? > > > > - 233903 file-avci100dec.nut > > - 383853 file-avci50dec.nut > > + 196558 file-avci100dec.nut > > + 333893 file-avci50dec.nut > > Looks like these source files have full range content; for any data with > full range input, this patch makes sure it uses the right intended > conversion through swscale. So yes, I guess it's expected that these > conversions change. patch ok then thx [...]
On Sun, 6 Mar 2022, Michael Niedermayer wrote: > On Sat, Mar 05, 2022 at 11:33:15PM +0200, Martin Storsjö wrote: >> On Fri, 4 Mar 2022, Michael Niedermayer wrote: >> >>> On Thu, Mar 03, 2022 at 02:06:45PM +0200, Martin Storsjö wrote: >>>> While swscale can be reconfigured with sws_setColorspaceDetails, >>>> the in/out ranges also need to be set before calling >>>> sws_init_context, otherwise the initialization might choose >>>> fastpaths that don't take the ranges into account. >>>> >>>> Therefore, look at in->color_range too, when deciding on whether >>>> the scaler needs to be reconfigured. >>>> >>>> Add a new member variable for keeping track of this, for being >>>> able to differentiate between whether the scale filter parameter >>>> "in_range" has been set (which should override whatever the input >>>> frame has set) or whether it has been configured based on the >>>> latest frame (which should trigger reconfiguring the scaler if >>>> the input frame ranges change). >>>> >>>> Signed-off-by: Martin Storsjö <martin@martin.st> >>>> --- >>>> To test this (without risking running many conflicting swscale >>>> filters in one filter pipeline), we'd need to be able to tag >>>> the incoming raw yuv data with colorspace and range without setting >>>> the in_color_matrix and in_range options on the scale filter. >>>> >>>> When using the rawvideo demuxer, the pixel format is set via the >>>> ffmpeg -pix_fmt option, but there's no corresponding option for >>>> setting color matrix or range for it. >>>> --- >>>> libavfilter/vf_scale.c | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>> >>> This changes the output for: >>> ffmpeg -i tickets/524/AVCI50.mov -vframes 3 file-avci50dec.nut >>> ffmpeg -i tickets/4493/AVCI100.mov -vframes 3 file-avci100dec.nut >>> >>> https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket524/ >>> >>> Is that intended ? >>> >>> - 233903 file-avci100dec.nut >>> - 383853 file-avci50dec.nut >>> + 196558 file-avci100dec.nut >>> + 333893 file-avci50dec.nut >> >> Looks like these source files have full range content; for any data with >> full range input, this patch makes sure it uses the right intended >> conversion through swscale. So yes, I guess it's expected that these >> conversions change. > > patch ok then Pushed, thanks. // Martin
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index 44f85cb019..996f7aaa5b 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -138,6 +138,7 @@ typedef struct ScaleContext { char *out_color_matrix; int in_range; + int in_frame_range; int out_range; int out_h_chr_pos; @@ -322,6 +323,8 @@ static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts) scale->opts = *opts; *opts = NULL; + scale->in_frame_range = AVCOL_RANGE_UNSPECIFIED; + return 0; } @@ -544,6 +547,9 @@ static int config_props(AVFilterLink *outlink) if (scale->in_range != AVCOL_RANGE_UNSPECIFIED) av_opt_set_int(s, "src_range", scale->in_range == AVCOL_RANGE_JPEG, 0); + else if (scale->in_frame_range != AVCOL_RANGE_UNSPECIFIED) + av_opt_set_int(s, "src_range", + scale->in_frame_range == AVCOL_RANGE_JPEG, 0); if (scale->out_range != AVCOL_RANGE_UNSPECIFIED) av_opt_set_int(s, "dst_range", scale->out_range == AVCOL_RANGE_JPEG, 0); @@ -690,6 +696,13 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) in->sample_aspect_ratio.den != link->sample_aspect_ratio.den || in->sample_aspect_ratio.num != link->sample_aspect_ratio.num; + if (in->color_range != AVCOL_RANGE_UNSPECIFIED && + scale->in_range == AVCOL_RANGE_UNSPECIFIED && + in->color_range != scale->in_frame_range) { + scale->in_frame_range = in->color_range; + frame_changed = 1; + } + if (scale->eval_mode == EVAL_MODE_FRAME || frame_changed) { unsigned vars_w[VARS_NB] = { 0 }, vars_h[VARS_NB] = { 0 };
While swscale can be reconfigured with sws_setColorspaceDetails, the in/out ranges also need to be set before calling sws_init_context, otherwise the initialization might choose fastpaths that don't take the ranges into account. Therefore, look at in->color_range too, when deciding on whether the scaler needs to be reconfigured. Add a new member variable for keeping track of this, for being able to differentiate between whether the scale filter parameter "in_range" has been set (which should override whatever the input frame has set) or whether it has been configured based on the latest frame (which should trigger reconfiguring the scaler if the input frame ranges change). Signed-off-by: Martin Storsjö <martin@martin.st> --- To test this (without risking running many conflicting swscale filters in one filter pipeline), we'd need to be able to tag the incoming raw yuv data with colorspace and range without setting the in_color_matrix and in_range options on the scale filter. When using the rawvideo demuxer, the pixel format is set via the ffmpeg -pix_fmt option, but there's no corresponding option for setting color matrix or range for it. --- libavfilter/vf_scale.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)