[FFmpeg-soc] libavfilter audio work - qualification task

[S.N. Hemanth Meenakshisundaram]

On 05/03/2010 01:32 PM, Stefano Sabatini wrote:
> On date Monday 2010-05-03 01:11:07 -0700, S.N. Hemanth Meenakshisundaram encoded:
>    
>> On 04/23/2010 05:03 PM, Stefano Sabatini wrote:
>>      
>>> On date Thursday 2010-04-22 17:19:16 -0700, S.N. Hemanth Meenakshisundaram encoded:
>>> [...]
>>>        
>> +
>> +    /* FIXME: av_parse_color currently sets alpha to 0 if no alpha is specified.
>> +     * So we force alpha = 0xFF (opaque), here in such a case.
>> +     */
>> +    if(rgba[3] != 0)
>> +        color[3] = rgba[3];
>> +    else
>> +        color[3] = 0xFF;
>> +    return 0;
>> +}
>>      
> What if the user wants to specify alpha = 0?
>
> I'd rather avoid the if (... != 0) etc check, the user can set the
> alpha with the 0xRRGGBBAA syntax if he wants, or we could extend the
> av_parse_color() syntax for specifing color+alpha (for example:
> color=red/0xff, a nicer syntax suggestion is welcome).
>    

Will fix parseutils fix as a separate patch. For now, this will work.

>
> So what about:
> if (alpha) {
> ...
> } else {
> ...
> }
>
> this should save efficiency if alpha composition is not required. Feel
> free to skip this and leave it to an eventual further patch.
>    

Done. Alpha now available for the background box as well.

>
> Condition (text == dtext->text) is always true, also we cannot
> distinguish between the two cases (imho is better like this), and I
> see no reason to treat '_' in a special way.
>
>    

Fixed. '-' is now like any other character.

Also fixed style nits and corrected handling of freetype error messages.

>
>> Index: Makefile
>
>> +libavfilter/vf_drawtext.o libavfilter/vf_drawtext.d: CFLAGS += $(`freetype-config --cflags`)
>> +FF_EXTRALIBS += $(`freetype-config --libs`)
>> +
>>      
> No please, both cflags and ldflags should be set in configure.
>    

Done. Moved to configure. No changes required now to original Makefile.

>> --- libavfilter.texi.nodt	2010-05-03 00:39:50.516428954 -0700
>> +++ libavfilter.texi	2010-05-03 00:28:42.221181682 -0700
>> @@ -148,6 +148,91 @@
>>   Draw a box with @var{x}:@var{y}:@var{width}:@var{height}:@var{color}
>>   dimensions in a chosen color.
>>
>> + at section drawtext
>> +
>> +Draws text string or text from specified file on top of video.
>>      
> Impersonal form (Draws ->  Draw) seems preferred in libavfilter.texi,
> same for the parameter descriptions
>    

Fixed.

> Nit, add empty newline between @end table and @example, improve
> readability.
>    

Done.

The patch to soc/libavfilter repo is attached. Has changes to 
allfilters.c, libavfilter Makefile and vf_drawtext.c only.

Please let me know if further changes are required.

Regards,
Hemanth

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: drawtext.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100509/038d3573/attachment.txt>