[FFmpeg-devel] [PATCH] lavfi/blackframe: add support for named options

[Clément Bœsch]

On Sat, Mar 16, 2013 at 12:07:44AM +0100, Stefano Sabatini wrote:
> TODO: bump micro
> ---
>  doc/filters.texi            |   22 ++++++++++++++--------
>  libavfilter/vf_blackframe.c |   38 +++++++++++++++++++++++++++++---------
>  2 files changed, 43 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index b1653ad..108718b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -1800,16 +1800,22 @@ the position in the file if known or -1 and the timestamp in seconds.
>  In order to display the output lines, you need to set the loglevel at
>  least to the AV_LOG_INFO value.
>  
> -The filter accepts the syntax:
> - at example
> -blackframe[=@var{amount}:[@var{threshold}]]
> - at end example
> +The filter accepts parameters as a list of @var{key}=@var{value}
> +pairs, separated by ":". If the key of the first options is omitted,
> +the arguments are interpreted according to the syntax
> +blackframe[=@var{amount}:[@var{threshold}]].
>  

blackframe[=@var{amount}[:@var{threshold}]].

> - at var{amount} is the percentage of the pixels that have to be below the
> -threshold, and defaults to 98.
> +A description of the accepted options follows.
> +
> + at table @option
> + at item amount
> +Set the percentage of pixels that have to be below the
> +threshold to enable black detection. Default value is 98.
>  
> - at var{threshold} is the threshold below which a pixel value is
> -considered black, and defaults to 32.
> + at item threshold
> +Set the threshold below which a pixel value is considered
> +black. Default value is 32.
> + at end table
>  
>  @section blend
>  
> diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c
> index 52c56d8..4a9016e 100644
> --- a/libavfilter/vf_blackframe.c
> +++ b/libavfilter/vf_blackframe.c
> @@ -31,6 +31,7 @@
>  #include <inttypes.h>
>  
>  #include "libavutil/internal.h"
> +#include "libavutil/opt.h"
>  #include "avfilter.h"
>  #include "internal.h"
>  #include "formats.h"
> @@ -38,6 +39,7 @@
>  #include "video.h"
>  
>  typedef struct {
> +    const AVClass *class;
>      unsigned int bamount; ///< black amount
>      unsigned int bthresh; ///< black threshold
>      unsigned int frame;   ///< frame number
> @@ -45,6 +47,17 @@ typedef struct {
>      unsigned int last_keyframe; ///< frame number of the last received key-frame
>  } BlackFrameContext;
>  
> +#define OFFSET(x) offsetof(BlackFrameContext, x)
> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
> +
> +static const AVOption blackframe_options[] = {
> +    { "amount", "set least percentual amount of pixels below the black threshold enabling black detection", OFFSET(bamount), AV_OPT_TYPE_INT, {.i64=98}, 0, 100, FLAGS },
> +    { "thresh", "set threshold below which a pixel value is considered black", OFFSET(bthresh), AV_OPT_TYPE_INT, {.i64=32}, 0, 255, FLAGS },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(blackframe);
> +
>  static int query_formats(AVFilterContext *ctx)
>  {
>      static const enum AVPixelFormat pix_fmts[] = {
> @@ -60,27 +73,31 @@ static int query_formats(AVFilterContext *ctx)
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
>      BlackFrameContext *blackframe = ctx->priv;
> +    static const char *shorthand[] = { "amount", "thresh", NULL };
> +    int ret;
>  
> -    blackframe->bamount = 98;
> -    blackframe->bthresh = 32;
>      blackframe->nblack = 0;
>      blackframe->frame = 0;
>      blackframe->last_keyframe = 0;

You could remove the other = 0 while at it.

>  
> -    if (args)
> -        sscanf(args, "%u:%u", &blackframe->bamount, &blackframe->bthresh);
> +    blackframe->class = &blackframe_class;
> +    av_opt_set_defaults(blackframe);
> +
> +    if ((ret = av_opt_set_from_string(blackframe, args, shorthand, "=", ":")) < 0)
> +        return ret;
>  
>      av_log(ctx, AV_LOG_VERBOSE, "bamount:%u bthresh:%u\n",
>             blackframe->bamount, blackframe->bthresh);
>  
> -    if (blackframe->bamount > 100 || blackframe->bthresh > 255) {
> -        av_log(ctx, AV_LOG_ERROR, "Too big value for bamount (max is 100) or bthresh (max is 255)\n");
> -        return AVERROR(EINVAL);
> -    }
> -
>      return 0;
>  }
>  
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    BlackFrameContext *blackframe = ctx->priv;
> +    av_opt_free(blackframe);
> +}
> +

The av_opt_free() could be in the init function, but that'snot important.

>  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>  {
>      AVFilterContext *ctx = inlink->dst;
> @@ -135,10 +152,13 @@ AVFilter avfilter_vf_blackframe = {
>  
>      .priv_size = sizeof(BlackFrameContext),
>      .init      = init,
> +    .uninit    = uninit,
>  
>      .query_formats = query_formats,
>  
>      .inputs    = avfilter_vf_blackframe_inputs,
>  
>      .outputs   = avfilter_vf_blackframe_outputs,
> +
> +    .priv_class = &blackframe_class,

Rest LGTM

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130316/a652d4e2/attachment.asc>