| Message ID | 20210801132202.74239-1-fulinjie@zju.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] fftools/cmdutils: make the default swscale flags identical for simple and complex filter graph | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | fail | Make fate failed |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | warning | Make fate failed |
On Sun, Aug 1, 2021 at 9:25 PM Nicolas George <george@nsup.org> wrote: > Linjie Fu (12021-08-01): > > From: Linjie Fu <linjie.justin.fu@gmail.com> > > > > Currently the default swscale flags for simple filter graph is bicubic, > > however for complex filter graph it uses bilinear as decleared in scale > > filter. > > Why does ffmpeg set a default different from swscale? > Checked swscale, it uses bicubic by default. Scale filter sets the flags to bilinear if sws_scale_opt is not initialized by sws_dict. We should use the same default flags for both simple and complex filter graphs, either from swscale, or from scale filter. If prefer to keep it identical with swscale, we may modify the flags in the scale filter. - linjie
Linjie Fu (12021-08-01): > Checked swscale, it uses bicubic by default. > Scale filter sets the flags to bilinear if sws_scale_opt is not initialized > by sws_dict. > > We should use the same default flags for both simple and complex filter > graphs, either from swscale, or from scale filter. > If prefer to keep it identical with swscale, we may modify the flags in the > scale filter. Thanks. I also think that unless there is a good reason, we should rely on a single default, from the lowest level. So: - remove flags="bilinear" from vf_scale; - remove setting defaults in ffmpeg. Unless somebody can find a good reason for different defaults. Michael, it is your commit, 8155bbc944c314fc202d31b0e4a3c77b8efc8dde, that introduced it. Any comments? Also, the current code in libswscale looks nonsensical: if (dstW < srcW && dstH < srcH) flags |= SWS_BICUBIC; else if (dstW > srcW && dstH > srcH) flags |= SWS_BICUBIC; else flags |= SWS_BICUBIC; It was originally 6b3ff6f91a535d6383f41ca7bdf760165dcb6015 with some logic, but it was cancelled by the merge commit 5710f55e490d0c3cf7348b3ca91c8c0fe4274be2. "The default is left at bicubic until someone has compared the scalers properly speed and quality wise." But eight years later nobody did. Regards,
On 2021-08-01 19:19, Linjie Fu wrote: > On Sun, Aug 1, 2021 at 9:25 PM Nicolas George <george@nsup.org> wrote: > >> Linjie Fu (12021-08-01): >>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>> >>> Currently the default swscale flags for simple filter graph is bicubic, >>> however for complex filter graph it uses bilinear as decleared in scale >>> filter. >> Why does ffmpeg set a default different from swscale? >> > Checked swscale, it uses bicubic by default. > Scale filter sets the flags to bilinear if sws_scale_opt is not initialized > by sws_dict. > > We should use the same default flags for both simple and complex filter > graphs, either from swscale, or from scale filter. > If prefer to keep it identical with swscale, we may modify the flags in the > scale filter. Keeping bicubic the default is best. You'll need to update the refs for all the failed FATE tests. Regards, Gyan
On Sun, Aug 1, 2021 at 10:13 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > On 2021-08-01 19:19, Linjie Fu wrote: > > On Sun, Aug 1, 2021 at 9:25 PM Nicolas George <george@nsup.org> wrote: > > > >> Linjie Fu (12021-08-01): > >>> From: Linjie Fu <linjie.justin.fu@gmail.com> > >>> > >>> Currently the default swscale flags for simple filter graph is bicubic, > >>> however for complex filter graph it uses bilinear as decleared in scale > >>> filter. > >> Why does ffmpeg set a default different from swscale? > >> > > Checked swscale, it uses bicubic by default. > > Scale filter sets the flags to bilinear if sws_scale_opt is not > initialized > > by sws_dict. > > > > We should use the same default flags for both simple and complex filter > > graphs, either from swscale, or from scale filter. > > If prefer to keep it identical with swscale, we may modify the flags in > the > > scale filter. > > Keeping bicubic the default is best. > > You'll need to update the refs for all the failed FATE tests. Got the point, bicubic seems more reasonable if taking FATE tests into account. Let's wait for more comments, and maybe do some code clean too for libswscale as Nicolas has pointed out. - linjie
On Sun, Aug 01, 2021 at 04:08:17PM +0200, Nicolas George wrote: > Linjie Fu (12021-08-01): > > Checked swscale, it uses bicubic by default. > > Scale filter sets the flags to bilinear if sws_scale_opt is not initialized > > by sws_dict. > > > > We should use the same default flags for both simple and complex filter > > graphs, either from swscale, or from scale filter. > > If prefer to keep it identical with swscale, we may modify the flags in the > > scale filter. > > Thanks. I also think that unless there is a good reason, we should rely > on a single default, from the lowest level. So: > > - remove flags="bilinear" from vf_scale; > > - remove setting defaults in ffmpeg. > > Unless somebody can find a good reason for different defaults. > > Michael, it is your commit, 8155bbc944c314fc202d31b0e4a3c77b8efc8dde, > that introduced it. Any comments? > > Also, the current code in libswscale looks nonsensical: > > if (dstW < srcW && dstH < srcH) > flags |= SWS_BICUBIC; > else if (dstW > srcW && dstH > srcH) > flags |= SWS_BICUBIC; > else > flags |= SWS_BICUBIC; > > It was originally 6b3ff6f91a535d6383f41ca7bdf760165dcb6015 with some > logic, but it was cancelled by the merge commit > 5710f55e490d0c3cf7348b3ca91c8c0fe4274be2. > > "The default is left at bicubic until someone has compared the scalers > properly speed and quality wise." > > But eight years later nobody did. In case someone tests this, it should test 1. human subjective quality in addition to some objective quality 2. scaling + compression 3. various material, noisy and less crisp material may favor different scalers than super flawless crisp images thx [...]
On Tue, Aug 3, 2021 at 12:48 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Aug 01, 2021 at 04:08:17PM +0200, Nicolas George wrote: > > Linjie Fu (12021-08-01): > > > Checked swscale, it uses bicubic by default. > > > Scale filter sets the flags to bilinear if sws_scale_opt is not > initialized > > > by sws_dict. > > > > > > We should use the same default flags for both simple and complex filter > > > graphs, either from swscale, or from scale filter. > > > If prefer to keep it identical with swscale, we may modify the flags > in the > > > scale filter. > > > > Thanks. I also think that unless there is a good reason, we should rely > > on a single default, from the lowest level. So: > > > > - remove flags="bilinear" from vf_scale; > > > > - remove setting defaults in ffmpeg. > > > > Unless somebody can find a good reason for different defaults. > Michael, it is your commit, 8155bbc944c314fc202d31b0e4a3c77b8efc8dde, > > that introduced it. Any comments? > > > > Also, the current code in libswscale looks nonsensical: > > > > if (dstW < srcW && dstH < srcH) > > flags |= SWS_BICUBIC; > > else if (dstW > srcW && dstH > srcH) > > flags |= SWS_BICUBIC; > > else > > flags |= SWS_BICUBIC; > > > > It was originally 6b3ff6f91a535d6383f41ca7bdf760165dcb6015 with some > > logic, but it was cancelled by the merge commit > > 5710f55e490d0c3cf7348b3ca91c8c0fe4274be2. > > > "The default is left at bicubic until someone has compared the scalers > > properly speed and quality wise." > > > > But eight years later nobody did. > > In case someone tests this, it should test > 1. human subjective quality in addition to some objective quality > 2. scaling + compression > 3. various material, noisy and less crisp material may favor > different scalers than super flawless crisp images Just update the patch set as suggested and keep the default to "bicubic" for now, until we get a better choice, thx. - linjie
diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 0b1ef03a25..56b8ada5b1 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -83,7 +83,7 @@ enum show_muxdemuxers { void init_opts(void) { - av_dict_set(&sws_dict, "flags", "bicubic", 0); + av_dict_set(&sws_dict, "flags", "bilinear", 0); } void uninit_opts(void)