[FFmpeg-devel] [PATCH] drawtext: unescape fontconfig patterns

[Nicolas George]

Le quartidi 14 prairial, an CCXXI, bn a écrit :
> This fixes drawtext's handling of fontconfig patterns with ':' attached
> properties, e.g. 'Times-12:italic' which when escaped to get past the
> options parser ('Times-12\:italic') will naturally fail in fontconfig
> itself unless unescaped again.

This is wrong, the options parsing system already takes care of unescaping.
If it does not work for you, please post your exact command line, including
information about the shell (it may change escaping rules).

> Please note that I took the expedient approach of altering the string
> in-place, which may or may not be ideal.

That would have been fine, since the string belongs to the filter.

Below, technical review for reference in case you need to submit another
patch sometime:

> -----------------------------------------------------------
> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index 2358e35..6a54d46 100644
> --- a/libavfilter/vf_drawtext.c
> +++ b/libavfilter/vf_drawtext.c
> @@ -322,6 +322,25 @@ static int load_font_file(AVFilterContext *ctx, const char *path, int index,
>      return 0;
>  }
>  
> +static char* unescape(char *text)

The preferred coding style is "char *name" rather than "char* name".

> +{
> +    char *p = text;
> +    int shift = 0;
> +
> +    while (*p) {
> +        if (*p == '\\' && p[1])) {
> +            shift++;
> +            p++;
> +        }
> +        if (shift) {
> +            *(p - shift) = *p;
> +        }
> +        p++;
> +    }
> +    *(p - shift) = '\0';

It looks rather complex, using two pointers would be simpler:

p = q = text;
while (*p) {
    if (*p == '\\' && p[1])
	p++;
    *(q++) = *(p++);
}

> +    return text;
> +}
> +
>  #if CONFIG_FONTCONFIG
>  static int load_font_fontconfig(AVFilterContext *ctx, const char **error)
>  {
> @@ -338,7 +357,7 @@ static int load_font_fontconfig(AVFilterContext *ctx, const char **error)
>          *error = "impossible to init fontconfig\n";
>          return AVERROR(EINVAL);
>      }
> -    pattern = FcNameParse(dtext->fontfile ? dtext->fontfile :
> +    pattern = FcNameParse(dtext->fontfile ? (uint8_t *)unescape(dtext->fontfile) :
>                            (uint8_t *)(intptr_t)"default");
>      if (!pattern) {
>          *error = "could not parse fontconfig pattern";

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/20130603/410cf8cc/attachment.asc>