[FFmpeg-devel] [PATCH] Change default behaviour of scale filter from 'progressive' to 'auto'

[Baptiste Coudurier]

On 04/03/2012 09:07 AM, Tim Nicholson wrote:
> On 30/03/12 15:47, Michael Niedermayer wrote:
>> On Fri, Mar 30, 2012 at 10:51:28AM +0100, Tim Nicholson wrote:
>>> When the option of interlaced aware scaling was first introduced interlace detection
>>> was poor. This has been improved, and this proposed patch changes the default behaviour
>>> of the filter to depend upon the auto detected value, bringing it into line with
>>> many other options whose default value is -1 (auto).
>>>
>>> From 4d1ad96e209b18bdf86530d153a6a5acbc5aab8a Mon Sep 17 00:00:00 2001
>>> From: Tim Nicholson <Tim.Nicholson at bbc.co.uk>
>>> Date: Fri, 30 Mar 2012 10:43:46 +0100
>>> Subject: [PATCH] Make default setting of parameter interl=-1 (auto)
>>>
>>> Previously default was 0 (progressive), but interlace detection has been
>>> improved in 648e55ff, and this makes the behaviour consistent with other
>>> options that use 1|0|-1 with -1 the default.
>>
>> This patch changes several fate tests checksums
>>
>> please check if these changes are ok and if so update the checksums
>> with the patch
> 
> I have checked all the fate results. In every case the handling of
> progressive material within an interlaced stream has led to vertical
> softening of the final material leading to differences in the checksums.
> 
> This resubmitted version is the same as the original, but includes the
> updated checksums.
> 
> For interlaced material I have tested using the same settings as fate.
> The results show the integrity of the interlacing is maintained leading
> to an improvement in the final results.
> 
> To make best use of this patch more accurate interlace detection is
> required, perhaps based upon picture content and not flag settings.
> 
> Whilst this is beyond the scope of this patch it is already a work in
> progress by others.
> 
> 
> 0001-Make-default-setting-of-parameter-interl-1-auto.patch
> 
> 
> From 6723c74847acd9841f54ece7aa28c2c88f6ec9b7 Mon Sep 17 00:00:00 2001
> From: Tim Nicholson <Tim.Nicholson at bbc.co.uk>
> Date: Tue, 3 Apr 2012 16:54:40 +0100
> Subject: [PATCH] Make default setting of parameter interl=-1 (auto)
> 
> Previously default was 0 (progressive), this makes the behaviour
> consistent with other options that use 1|0|-1 with -1 the default.
> 
> 
> Signed-off-by: Tim Nicholson <Tim.Nicholson at bbc.co.uk>
> ---
>  doc/filters.texi              |    7 +++++--
>  libavfilter/vf_scale.c        |    4 +++-
>  tests/ref/vsynth1/dnxhd_1080i |    4 ++--
>  tests/ref/vsynth1/dv          |    4 ++--
>  tests/ref/vsynth1/dv50        |    4 ++--
>  tests/ref/vsynth1/dv_411      |    4 ++--
>  tests/ref/vsynth2/dnxhd_1080i |    4 ++--
>  tests/ref/vsynth2/dv          |    4 ++--
>  tests/ref/vsynth2/dv50        |    4 ++--
>  tests/ref/vsynth2/dv_411      |    4 ++--
>  10 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 2091242..0ab5a02 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2222,7 +2222,7 @@ can be used to test the monowhite pixel format descriptor definition.
>  
>  @section scale
>  
> -Scale the input video to @var{width}:@var{height}[:@var{interl}=@{1|-1@}] and/or convert the image format.
> +Scale the input video to @var{width}:@var{height}[:@var{interl}=@{1|0|-1@}] and/or convert the image format.
>  
>  The scale filter forces the output display aspect ratio to be the same
>  of the input, by changing the output sample aspect ratio.
> @@ -2276,12 +2276,15 @@ Valid values for the optional parameter @var{interl} are:
>  @item 1
>  force interlaced aware scaling
>  
> + at item 0
> +force progressive scaling
> +
>  @item -1
>  select interlaced aware scaling depending on whether the source frames
>  are flagged as interlaced or not
>  @end table
>  
> -Unless @var{interl} is set to one of the above options, interlaced scaling will not be used.
> +The default value for @var{interl} is -1 (auto).
>  
>  Some examples follow:
>  @example
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index deb7009..f3e77ec 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -93,7 +93,9 @@ static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
>          if (p) scale->flags = strtoul(p+6, NULL, 0);
>          if(strstr(args,"interl=1")){
>              scale->interlaced=1;
> -        }else if(strstr(args,"interl=-1"))
> +        }else if(strstr(args,"interl=0")){
> +            scale->interlaced=0;
> +        }else
>              scale->interlaced=-1;
>      }
>  
> diff --git a/tests/ref/vsynth1/dnxhd_1080i b/tests/ref/vsynth1/dnxhd_1080i
> index f8f6df0..de4732e 100644
> --- a/tests/ref/vsynth1/dnxhd_1080i
> +++ b/tests/ref/vsynth1/dnxhd_1080i
> @@ -1,4 +1,4 @@
>  027c985483caab9397592bf27477dce1 *./tests/data/vsynth1/dnxhd-1080i.mov
>  3031911 ./tests/data/vsynth1/dnxhd-1080i.mov
> -0c651e840f860592f0d5b66030d9fa32 *./tests/data/dnxhd_1080i.vsynth1.out.yuv
> -stddev:    6.29 PSNR: 32.15 MAXDIFF:   64 bytes:   760320/  7603200
> +3c3226518a0f56468bf56a6682e31fae *./tests/data/dnxhd_1080i.vsynth1.out.yuv
> +stddev:   14.22 PSNR: 25.07 MAXDIFF:  119 bytes:   760320/  7603200

A default change that produces a difference like this is unacceptable IMHO.
This is quite a big difference.

-- 
Baptiste COUDURIER
Key fingerprint          8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                           http://www.ffmpeg.org