| Message ID | 20170604075331.28661-1-kmark937@gmail.com |
|---|---|
| State | Accepted |
| Commit | 4af496473a0684b798d5712d8c96357e9b036b43 |
| Headers | show |
On Sun, Jun 4, 2017 at 1:23 PM, Kevin Mark <kmark937@gmail.com> wrote: > > If we were to use "iw/4:-1" in place of "iw/4:ow/mdar": > 640 / 4 = 160. So the new width for [main] would be 160. > 360 / 4 = 90. So the new height for [main] would be 90. > 160 / 90 = 16 / 9 so [main] now has the same aspect ratio as [ref] > which is probably what you do not want. > Now that this feature has been applied, it may be helpful to check if one of the W/H arguments is of the form '-n' and log a warning that this will force the ref's display aspect ratio. The warning should also convey the expression to use: 'oh*mdar' or 'ow/mdar' , as the case may be. Additionally, since the output SAR will be adjusted to force the ref's DAR, it would be helpful to add a warning for that (tell users to append setsar=1 after the scale) --or-- (ideally) patch the filter so that the main's DAR is preserved.
On Sun, Jun 04, 2017 at 03:53:31AM -0400, Kevin Mark wrote: > This new FATE test for the scale2ref filter makes use of the recently > added scale2ref-specific variables to maintain the aspect ratio of a > test input. > > Filtergraph explanation: > [main] has an AR of 4:3. [ref] has an AR of 16:9. > 640 / 4 = 160. So the new width for [main] is 160. > 160 / ((320 / 240) * (1 / 1)) = 160 / (4 / 3) = 120. So the new > height for [main] is 120. > 160 / 120 = 4 / 3 so [main]'s aspect ratio has been maintained while > using [ref]'s width as a reference point. > > [ref] is nullsink'd since it is left unchanged by scale2ref (and so > shouldn't need to be tested). > > If we were to use "iw/4:-1" in place of "iw/4:ow/mdar": > 640 / 4 = 160. So the new width for [main] would be 160. > 360 / 4 = 90. So the new height for [main] would be 90. > 160 / 90 = 16 / 9 so [main] now has the same aspect ratio as [ref] > which is probably what you do not want. > > This is currently the only test for scale2ref. > > Signed-off-by: Kevin Mark <kmark937@gmail.com> > --- > tests/fate/filter-video.mak | 4 ++++ > tests/filtergraphs/scale2ref_keep_aspect | 5 +++++ > tests/ref/fate/filter-scale2ref_keep_aspect | 14 ++++++++++++++ > 3 files changed, 23 insertions(+) > create mode 100644 tests/filtergraphs/scale2ref_keep_aspect > create mode 100644 tests/ref/fate/filter-scale2ref_keep_aspect applied thx [...]
On Sun, Jun 4, 2017 at 4:19 AM, Gyan <gyandoshi@gmail.com> wrote: > Now that this feature has been applied, it may be helpful to check if one > of the W/H arguments is of the form '-n' and log a warning that this will > force the ref's display aspect ratio. The warning should also convey the > expression to use: 'oh*mdar' or 'ow/mdar' , as the case may be. > > Additionally, since the output SAR will be adjusted to force the ref's DAR, > it would be helpful to add a warning for that (tell users to append > setsar=1 after the scale) --or-- (ideally) patch the filter so that the > main's DAR is preserved. I was thinking about this as well. The next step for improving the scale2ref filter is probably patching it to preserve main's DAR, as you mentioned. It's definitely easy enough to correct with the setsar filter but given scale2ref's purpose the current behavior still feels dramatically unintuitive. Perhaps I'm just being unimaginative when it comes to use-cases but it certainly seems like the vast majority of uses would involve keeping main's DAR. This would be a breaking change, unlike the previous patches. The warning for the "-n" stuff sounds helpful. I'll take a look at this.
diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak index 1ca29e8d32..53fc7a6528 100644 --- a/tests/fate/filter-video.mak +++ b/tests/fate/filter-video.mak @@ -413,6 +413,10 @@ fate-filter-scale200: CMD = video_filter "scale=w=200:h=200" FATE_FILTER_VSYNTH-$(CONFIG_SCALE_FILTER) += fate-filter-scale500 fate-filter-scale500: CMD = video_filter "scale=w=500:h=500" +FATE_FILTER_VSYNTH-$(CONFIG_SCALE2REF_FILTER) += fate-filter-scale2ref_keep_aspect +fate-filter-scale2ref_keep_aspect: tests/data/filtergraphs/scale2ref_keep_aspect +fate-filter-scale2ref_keep_aspect: CMD = framemd5 -frames:v 5 -filter_complex_script $(TARGET_PATH)/tests/data/filtergraphs/scale2ref_keep_aspect -map "[main]" + FATE_FILTER_VSYNTH-$(CONFIG_SCALE_FILTER) += fate-filter-scalechroma fate-filter-scalechroma: tests/data/vsynth1.yuv fate-filter-scalechroma: CMD = framecrc -flags bitexact -s 352x288 -pix_fmt yuv444p -i tests/data/vsynth1.yuv -pix_fmt yuv420p -sws_flags +bitexact -vf scale=out_v_chr_pos=33:out_h_chr_pos=151 diff --git a/tests/filtergraphs/scale2ref_keep_aspect b/tests/filtergraphs/scale2ref_keep_aspect new file mode 100644 index 0000000000..f407460ec7 --- /dev/null +++ b/tests/filtergraphs/scale2ref_keep_aspect @@ -0,0 +1,5 @@ +sws_flags=+accurate_rnd+bitexact; +testsrc=size=320x240 [main]; +testsrc=size=640x360 [ref]; +[main][ref] scale2ref=iw/4:ow/mdar [main][ref]; +[ref] nullsink diff --git a/tests/ref/fate/filter-scale2ref_keep_aspect b/tests/ref/fate/filter-scale2ref_keep_aspect new file mode 100644 index 0000000000..ca03277446 --- /dev/null +++ b/tests/ref/fate/filter-scale2ref_keep_aspect @@ -0,0 +1,14 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 160x120 +#sar 0: 4/3 +#stream#, dts, pts, duration, size, hash +0, 0, 0, 1, 57600, 9a19c23dc3a557786840d0098606d5f1 +0, 1, 1, 1, 57600, e6fbdabaf1bb0d28afc648ed4d27e9f0 +0, 2, 2, 1, 57600, 52924ed0a751214c50fb2e7a626c8cc5 +0, 3, 3, 1, 57600, 67d5fd6ee71793f1cf8794d1c27afdce +0, 4, 4, 1, 57600, 85f7775f7b01afd369fc8919dc759d30
This new FATE test for the scale2ref filter makes use of the recently added scale2ref-specific variables to maintain the aspect ratio of a test input. Filtergraph explanation: [main] has an AR of 4:3. [ref] has an AR of 16:9. 640 / 4 = 160. So the new width for [main] is 160. 160 / ((320 / 240) * (1 / 1)) = 160 / (4 / 3) = 120. So the new height for [main] is 120. 160 / 120 = 4 / 3 so [main]'s aspect ratio has been maintained while using [ref]'s width as a reference point. [ref] is nullsink'd since it is left unchanged by scale2ref (and so shouldn't need to be tested). If we were to use "iw/4:-1" in place of "iw/4:ow/mdar": 640 / 4 = 160. So the new width for [main] would be 160. 360 / 4 = 90. So the new height for [main] would be 90. 160 / 90 = 16 / 9 so [main] now has the same aspect ratio as [ref] which is probably what you do not want. This is currently the only test for scale2ref. Signed-off-by: Kevin Mark <kmark937@gmail.com> --- tests/fate/filter-video.mak | 4 ++++ tests/filtergraphs/scale2ref_keep_aspect | 5 +++++ tests/ref/fate/filter-scale2ref_keep_aspect | 14 ++++++++++++++ 3 files changed, 23 insertions(+) create mode 100644 tests/filtergraphs/scale2ref_keep_aspect create mode 100644 tests/ref/fate/filter-scale2ref_keep_aspect