[FFmpeg-devel] [PATCH 2/2] vf_drawtext: fontconfig support.

[Nicolas George]

Le decadi 20 germinal, an CCXX, Stefano Sabatini a écrit :
> A link to the doc/site of fontconfig may be useful somewhere.

Added.

> Nit: Capitalized, and use final dots consistently with the rest of drawtext.

Fixed the dot. The capitalization seems ok, since this error message is
appended to a generic one.

> 1+i*nit: I'd avoid the term "error" in error messages, otherwise you
> end up with log messages like this:
> "Error occurred with fontconfig: error parsing fontconfig pattern";
> 
> which should rather be:
> "Error occurred with fontconfig: could not parse fontconfig pattern";
> 
> which is less redundant and more specific.

Changed.

> nit+++: vertical align

Done.

> nit: s/err/ret/,

That would be inconsistent with the rest of the code, including parts this
patch leave unchanged.

>		   if (ret >= 0) return ret; more readable to me

"if no error return success" seems pretty readable.

> stray return?

Copy paste error, fixed.

Thanks for the review, new patch (with Clement's comments too) sent.

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/20120410/3f922b09/attachment.asc>