[FFmpeg-devel] [PATCH] vf_fade: fade alpha

[Mark Himsley]

On 07/11/11 17:01, Stefano Sabatini wrote:
> On date Sunday 2011-11-06 16:56:09 +0000, Mark Himsley encoded:
>> On 06/11/2011 16:54, Mark Himsley wrote:
>>> Add alpha fading.
>>>
>>> On 06/11/2011 00:40, Stefano Sabatini wrote:
>>>> On date Saturday 2011-11-05 21:53:35 +0000, Mark Himsley encoded:
>>>>> On 05/11/2011 14:26, Stefano Sabatini wrote:
>>>>>> On date Saturday 2011-11-05 02:01:02 +0000, Mark Himsley encoded:
>>>>>>> add alpha fading
>>>> [...]
>>>>>
>>>>> Thanks for the review.
>>>>
>>>> Thanks for the patch ;-).
>>>>
>>>>>
>>>>> I'll send updated patches.
>>>
>>> Including all suggested changes, including removing spurious double
>>> alpha_expr variable, simplifying black_level determination, changing new
>>> function name, meaningful names for variables, reformatting function
>>> call and adding comments in function call.
>>>
>>> I also added macros for R G B A Y U V, although currently only 'A' is
>>> ever used...
>>
>> ...attached...
>>
>> --
>> Mark
>>

[...]

>> +    { "alpha",    "fade alpha if it's available on the input",  OFFSET(alpha),      AV_OPT_TYPE_INT,    {.dbl = 0    },        0,        1 },       {NULL},

[...]

>> +    fade->alpha = !!fade->alpha;
>
> this is not needed, as fade->alpha can only be 1 or 0.

Oh yes. I forgot about that checking. Thanks.

[...]

>> +        fade_plane(y, h, inlink->w,
>> +                fade->factor, fade->black_level, fade->black_level_scaled,
>> +                0, 1, // offset&  pixstep for Y plane or RGB packed format
>> +                fade->bpp, outpic->data[0], outpic->linesize[0]);
>
> weird indent (no need to post a new patch for this)

I'll assume you'd rather 'fade->factor' was directly under 'y'. Sorry, I 
do that next time.

> Looks fine to me, I'll post in a day or so if I read no more comments.

Thank you.

-- 
Mark