On Fri, 14 Feb 2014 06:01:44 +0100, Vittorio Giovara 
<[email protected]> wrote:
> ---
> Turning off the lowpass filter while interlacing is a terrible idea
> as it can easily gernerate terrible results; the effect would be
> similar to decimating vertically by dropping the odd lines.
> 
> So I propose to remove this option from the filter and keep the lowpass
> filter always applied, like by default.
> 
> I'm not sure if/which version bump should be made or if I have to wrap
> the code-to-be-removed in FF_API_ clauses.
> 
> Cheers,
>     Vittorio
> 
>  doc/filters.texi           |  7 ++----
>  libavfilter/vf_interlace.c | 57 
> +++++++++++++++++++++++-----------------------
>  2 files changed, 30 insertions(+), 34 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 8c83b4e..51f208a 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -1426,7 +1426,8 @@ a float number which specifies chroma temporal 
> strength, defaults to
>  
>  Simple interlacing filter from progressive contents. This interleaves upper 
> (or
>  lower) lines from odd frames with lower (or upper) lines from even frames,
> -halving the frame rate and preserving image height.
> +halving the frame rate and preserving image height. A vertical lowpass filter
> +is always applied in order to avoid twitter effect and reduce moire patterns.
>  
>  @example
>     Original        Original             New Frame
> @@ -1446,10 +1447,6 @@ It accepts the following optional parameters:
>  @item scan
>  determines whether the interlaced frame is taken from the even (tff - 
> default)
>  or odd (bff) lines of the progressive frame.
> -
> -@item lowpass
> -Enable (default) or disable the vertical lowpass filter to avoid twitter
> -interlacing and reduce moire patterns.
>  @end table
>  
>  @section lut, lutrgb, lutyuv
> diff --git a/libavfilter/vf_interlace.c b/libavfilter/vf_interlace.c
> index a05ab03..51aded5 100644
> --- a/libavfilter/vf_interlace.c
> +++ b/libavfilter/vf_interlace.c
> @@ -58,7 +58,7 @@ static const AVOption options[] = {
>          AV_OPT_TYPE_CONST, {.i64 = MODE_TFF }, INT_MIN, INT_MAX, .flags = V, 
> .unit = "scan" },
>      { "bff", "bottom field first", 0,
>          AV_OPT_TYPE_CONST, {.i64 = MODE_BFF }, INT_MIN, INT_MAX, .flags = V, 
> .unit = "scan" },
> -    { "lowpass", "enable vertical low-pass filter", OFFSET(lowpass),
> +    { "lowpass", "(deprecated, the filter is always set)", OFFSET(lowpass),
>          AV_OPT_TYPE_INT,   {.i64 = 1 },        0, 1, .flags = V },
>      { NULL }
>  };
> @@ -100,6 +100,9 @@ static int config_out_props(AVFilterLink *outlink)
>      AVFilterLink *inlink = outlink->src->inputs[0];
>      InterlaceContext *s = ctx->priv;
>  
> +    if (!s->lowpass)
> +        av_log(ctx, AV_LOG_WARNING, "this is not the lowpass you were 
> looking for\n");
> +

Something more informative please.

Wrapp lowpass itself and all its uses in #if AVFILTER_VERSION_BLAH_BLAH, no need
to define FF_API i think, since it says within one file.

A mention in the Changelog would be nice.

Otherwise fine with me.

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to