[FFmpeg-devel] [PATCH] lavfi/ass: extend syntax for ass filter

[Clément Bœsch]

On Mon, Oct 15, 2012 at 10:09:27AM +0200, Stefano Sabatini wrote:
> Make the filter accept named options for the first argument, and update
> documentation accordingly.
> 
> TODO: bump micro
> ---
>  doc/filters.texi     |   21 ++++++++++++++-------
>  libavfilter/vf_ass.c |   13 +++++++------
>  2 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 4a62d81..756fdae 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -1224,26 +1224,33 @@ using the libass library.
>  To enable compilation of this filter you need to configure FFmpeg with
>  @code{--enable-libass}.
>  
> -This filter accepts the syntax: @var{ass_filename}[:@var{options}],
> -where @var{ass_filename} is the filename of the ASS file to read, and
> - at var{options} is an optional sequence of @var{key}=@var{value} pairs,
> -separated by ":".
> -
> -A description of the accepted options follows.
> +This filter accepts the following named options, expressed as a
> +sequence of @var{key}=@var{value} pairs, separated by ":".
>  
>  @table @option
> + at item filename, f
> +Set the filename of the ASS file to read. It must be specified.
> +
>  @item original_size
> -Specifies the size of the original video, the video for which the ASS file
> +Specify the size of the original video, the video for which the ASS file
>  was composed. Due to a misdesign in ASS aspect ratio arithmetic, this is
>  necessary to correctly scale the fonts if the aspect ratio has been changed.
>  @end table
>  
> +If the first key is not specified, it is assumed that the first value
> +specifies the @option{filename}.
> +
>  For example, to render the file @file{sub.ass} on top of the input
>  video, use the command:
>  @example
>  ass=sub.ass
>  @end example
>  
> +which is equivalent to:
> + at example
> +ass=filename=sub.ass
> + at end example
> +
>  @section bbox
>  

Note: could be nice to add an example with both filename and original_size
to show the point of named argument

>  Compute the bounding box for the non-black pixels in the input frame
> diff --git a/libavfilter/vf_ass.c b/libavfilter/vf_ass.c
> index 6b20631..2fc8008 100644
> --- a/libavfilter/vf_ass.c
> +++ b/libavfilter/vf_ass.c
> @@ -54,6 +54,8 @@ typedef struct {
>  #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
>  
>  static const AVOption ass_options[] = {
> +    {"filename",       "set the filename of the ASS file to read",                 OFFSET(filename),   AV_OPT_TYPE_STRING,     {.str = NULL},  CHAR_MIN, CHAR_MAX, FLAGS },
> +    {"f",              "set the filename of the ASS file to read",                 OFFSET(filename),   AV_OPT_TYPE_STRING,     {.str = NULL},  CHAR_MIN, CHAR_MAX, FLAGS },
>      {"original_size",  "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL},  CHAR_MIN, CHAR_MAX, FLAGS },
>      {NULL},
>  };
> @@ -83,21 +85,20 @@ static void ass_log(int ass_level, const char *fmt, va_list args, void *ctx)
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
>      AssContext *ass = ctx->priv;
> +    static const char *shorthand[] = { "filename", NULL };
>      int ret;
>  
>      ass->class = &ass_class;
>      av_opt_set_defaults(ass);
>  
> -    if (args)
> -        ass->filename = av_get_token(&args, ":");
> -    if (!ass->filename || !*ass->filename) {
> +    if ((ret = av_opt_set_from_string(ass, args, shorthand, "=", ":")) < 0)
> +        return ret;
> +
> +    if (!ass->filename) {
>          av_log(ctx, AV_LOG_ERROR, "No filename provided!\n");
>          return AVERROR(EINVAL);
>      }
>  
> -    if (*args++ == ':' && (ret = av_set_options_string(ass, args, "=", ":")) < 0)
> -        return ret;
> -

BTW I was wondering, don't you need some av_opt_free()?

[...]

LGTM otherwise

-- 
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/20121015/a8c736b3/attachment.asc>