[FFmpeg-devel] [PATCH] lavfi/hue: add process_command callback

[Stefano Sabatini]

On date Wednesday 2012-08-29 23:08:43 +0200, Jérémy Tran encoded:
> This allows dynamic reconfiguration of the filter.
> The callback uses some code that was in the init function. Hence this code
> has been moved in its own function.
> 
> Since the hue may now change multiple times, it is no longer possible to
> check if hue->hue_deg and hue->hue are both different to -FLT_MAX to verify
> that they have not been defined at the same time.

> The test now uses strchr on the 'args' string and looks for the options'
> name.

This is not very robust.

> ---
>  doc/filters.texi     | 12 ++++++++++++
>  libavfilter/vf_hue.c | 42 +++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index bef95f7..42f32a2 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2313,6 +2313,18 @@ hue=PI/2:1
>  @end example
>  @end itemize
>  
> + at subsection commands
> +
> +The filter supports the following command:
> + at table @option
> + at item reinit
> +Modify the hue and/or the saturation of the input video.
> +The command accepts the same named options and syntax than when calling the
> +filter from the command-line.
> +

> +If a parameter is omitted, it kept at its current value.

...it *is* kept...

> + at end table
> +
>  @section idet
>  
>  Interlaceing detect filter. This filter tries to detect if the input is
> diff --git a/libavfilter/vf_hue.c b/libavfilter/vf_hue.c
> index cf1fe5f..d1aa631 100644
> --- a/libavfilter/vf_hue.c
> +++ b/libavfilter/vf_hue.c
> @@ -63,22 +63,19 @@ static const AVOption hue_options[] = {
>  
>  AVFILTER_DEFINE_CLASS(hue);
>  
> -static av_cold int init(AVFilterContext *ctx, const char *args)
> +static inline int set_options(AVFilterContext *ctx, const char *args)
>  {
>      HueContext *hue = ctx->priv;
>      int n, ret;
>      char c1 = 0, c2 = 0;
>      char *equal;
>  
> -    hue->class = &hue_class;
> -    av_opt_set_defaults(hue);
> -
>      if (args) {
>          /* named options syntax */
>          if (equal = strchr(args, '=')) {
>              if ((ret = av_set_options_string(hue, args, "=", ":")) < 0)
>                  return ret;
> -            if (hue->hue != -FLT_MAX && hue->hue_deg != -FLT_MAX) {
> +            if (strchr(args, 'h') && strchr(args, 'H')) {

'h' may appear in the expressions.

This specific problem be possibly addressed at the API level (give the
possibility to tell which options have been set), but I think this can
be addressed even without that.

You can store the previous values for h and H, set them to undefined,
reset the values, perform the check. If both values are unspecified
you can set them back to the previous values.

[...]

> +static int command(AVFilterContext *ctx, const char *cmd, const char *args,
> +                   char *res, int res_len, int flags)

"process_command" looks more natural/consistent.

[...]
-- 
FFmpeg = Foolish & Faithless Moronic Ponderous Erudite Gangster