[FFmpeg-devel] [PATCH] Add "progressive" to the options that "setfield" can force

[Nicolas George]

Le duodi 2 floréal, an CCXX, Tim Nicholson a écrit :
> >From 1476ca01e2ac0cda618588a7b3c5f09263ae7593 Mon Sep 17 00:00:00 2001
> From: Tim Nicholson <Tim.Nicholson at bbc.co.uk>
> Date: Fri, 20 Apr 2012 10:51:42 +0100
> Subject: [PATCH] vf_setfield: add "progressive" option
> 
> "prog" parameter value added, and numeric values deprecated
> ---
>  doc/filters.texi          |   14 ++++++++------
>  libavfilter/vf_setfield.c |   20 ++++++++++++--------
>  2 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 455189c..4c5e9f4 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2538,19 +2538,21 @@ Force field for the output video frame.
>  The @code{setfield} filter marks the interlace type field for the
>  output frames. It does not change the input frame, but only sets the
>  corresponding property, which affects how the frame is treated by
> -followig filters (e.g. @code{fieldorder} or @code{yadif}).
> +following filters (e.g. @code{fieldorder} or @code{yadif}).
>  
> -It accepts a parameter representing an integer or a string, which can
> -assume the following values:
> +It accepts a string parameter, which can assume the following values:
>  @table @samp
> - at item -1, auto
> + at item auto
>  Keep the same field property.
>  
> - at item 0, bff
> + at item bff
>  Mark the frame as bottom-field-first.
>  
> - at item 1, tff
> + at item tff
>  Mark the frame as top-field-first.
> +
> + at item prog
> +Mark the frame as progressive.
>  @end table
>  
>  @section setpts
> diff --git a/libavfilter/vf_setfield.c b/libavfilter/vf_setfield.c
> index bfb8006..9f09687 100644
> --- a/libavfilter/vf_setfield.c
> +++ b/libavfilter/vf_setfield.c
> @@ -40,19 +40,22 @@ static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
>          if (sscanf(args, "%d%c", &setfield->top_field_first, &c) != 1) {
>              if      (!strcmp("tff",  args)) setfield->top_field_first = 1;
>              else if (!strcmp("bff",  args)) setfield->top_field_first = 0;
> +            else if (!strcmp("prog", args)) setfield->top_field_first = 2;
>              else if (!strcmp("auto", args)) setfield->top_field_first = -1;
>              else {
>                  av_log(ctx, AV_LOG_ERROR, "Invalid argument '%s'\n", args);
>                  return AVERROR(EINVAL);
>              }
>          }
> -    }
> -
> -    if (setfield->top_field_first < -1 || setfield->top_field_first > 1) {
> -        av_log(ctx, AV_LOG_ERROR,
> -               "Provided integer value %d must be included between -1 and +1\n",
> -               setfield->top_field_first);
> -        return AVERROR(EINVAL);
> +        else {

The usual coding style here is to put the else on the same line as the
closing brace (and to put braces on both branches if only one needs it). But
it does not matter much.

> +            if (setfield->top_field_first < -1 || setfield->top_field_first > 1) {
> +                av_log(ctx, AV_LOG_ERROR,
> +                       "Provided integer value %d must be included between -1 and +1\n",
> +                       setfield->top_field_first);
> +                return AVERROR(EINVAL);
> +            }
> +            av_log(ctx, AV_LOG_WARNING, "Using -1/0/1 is deprecated, please use auto/tff/bff/prog \n", args);

Is the space before the \n on purpose?

> +        }
>      }
>  
>      return 0;
> @@ -63,7 +66,8 @@ static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref)
>      SetFieldContext *setfield = inlink->dst->priv;
>      AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0);
>  
> -    if (setfield->top_field_first != -1) {
> +    if (setfield->top_field_first == 2) outpicref->video->interlaced = 0;
> +    else if (setfield->top_field_first != -1) {
>          outpicref->video->interlaced = 1;
>          outpicref->video->top_field_first = setfield->top_field_first;
>      }

No more remarks from me, I leave it to Stefano, who knows the filter better.
No need to send another patch to address the two trivial remarks above I
think.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120420/6d32deab/attachment.asc>