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

[Stefano Sabatini]

On date Sunday 2013-03-24 19:55:54 +0100, Clément Bœsch encoded:
> On Sun, Mar 24, 2013 at 01:48:27PM +0100, Stefano Sabatini wrote:
> [...]
> > -static const char *color_modes[] = {"bt709", "FCC", "bt601", "smpte240m"};
> > -
> > -static int get_color_mode_index(const char *name)
> > -{
> > -    int i;
> > -
> > -    for (i = 0; i < FF_ARRAY_ELEMS(color_modes); i++)
> > -        if (!av_strcasecmp(color_modes[i], name))
> > -            return i;
> > -    return -1;
> > -}
> > +static const char *color_modes[] = {"bt709", "fcc", "bt601", "smpte240m"};
> >  
> >  static av_cold int init(AVFilterContext *ctx, const char *args)
> >  {
> >      ColorMatrixContext *color = ctx->priv;
> >  
> > -    if (!args)
> > -        goto usage;
> > -    if (sscanf(args, "%255[^:]:%255[^:]", color->src, color->dst) != 2) {
> > -    usage:
> > -        av_log(ctx, AV_LOG_ERROR, "usage: <src>:<dst>\n");
> > -        av_log(ctx, AV_LOG_ERROR, "possible options: bt709,bt601,smpte240m,fcc\n");
> > -        return -1;
> > -    }
> > -
> > -    color->source = get_color_mode_index(color->src);
> > -    if (color->source < 0) {
> > -        av_log(ctx, AV_LOG_ERROR, "unknown color space %s\n", color->src);
> > -        return AVERROR(EINVAL);
> > -    }
> > -
> > -    color->dest = get_color_mode_index(color->dst);
> > -    if (color->dest < 0) {
> > -        av_log(ctx, AV_LOG_ERROR, "unknown color space %s\n", color->dst);
> > +    if (color->source == COLOR_MODE_NONE || color->dest == COLOR_MODE_NONE) {
> > +        av_log(ctx, AV_LOG_ERROR, "Unspecified source or destination color space\n");
> >          return AVERROR(EINVAL);
> >      }
> >  
> > @@ -313,7 +312,8 @@ static int config_input(AVFilterLink *inlink)
> >      color->hsub = pix_desc->log2_chroma_w;
> >      color->vsub = pix_desc->log2_chroma_h;
> >  
> > -    av_log(ctx, AV_LOG_VERBOSE, "%s -> %s\n", color->src, color->dst);
> > +    av_log(ctx, AV_LOG_VERBOSE, "%s -> %s\n",
> > +           color_modes[color->source], color_modes[color->dest]);
> >  
> >      return 0;
> >  }
> > @@ -375,13 +375,17 @@ static const AVFilterPad colormatrix_outputs[] = {
> >      { NULL }
> >  };
> >  
> > +static const char *colormatrix_shorthand[] = { "src", "dst", NULL };
> > +
> 
> nit: we don't use a prefix for shorthand currently; also we tend to use
> the static const char *const type.
> 
> >  AVFilter avfilter_vf_colormatrix = {
> >      .name          = "colormatrix",
> > -    .description   = NULL_IF_CONFIG_SMALL("Color matrix conversion"),
> > +    .description   = NULL_IF_CONFIG_SMALL("Convert color matrix."),
> >  
> >      .priv_size     = sizeof(ColorMatrixContext),
> >      .init          = init,
> >      .query_formats = query_formats,
> >      .inputs        = colormatrix_inputs,
> >      .outputs       = colormatrix_outputs,
> > +    .priv_class    = &colormatrix_class,
> > +    .shorthand     = colormatrix_shorthand,
> 
> LGTM anyway, thanks.

Fixed&applied, thanks.
-- 
FFmpeg = Free and Formidable Martial Programmable Educated Game