[FFmpeg-devel] [PATCH] yadif does not copy video props

[Baptiste Coudurier]

On 11/21/10 9:11 AM, Michael Niedermayer wrote:
> On Sat, Nov 20, 2010 at 07:14:35PM -0800, Baptiste Coudurier wrote:
>> On 10/4/10 3:32 PM, Michael Niedermayer wrote:
>>> On Mon, Oct 04, 2010 at 12:25:44PM -0700, Baptiste Coudurier wrote:
>>>> On 10/04/2010 03:45 AM, Michael Niedermayer wrote:
>>>>> On Sun, Oct 03, 2010 at 08:56:33PM -0700, Baptiste Coudurier wrote:
>>>>>> Hi,
>>>>>>
>>>>>> $subject.
>>>>>>
>>>>>> Copy video props and unset interlaced :)
>>>>>>
>>>>>> --
>>>>>> Baptiste COUDURIER
>>>>>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
>>>>>> FFmpeg maintainer                                  http://www.ffmpeg.org
>>>>>
>>>>>>   vf_yadif.c |    2 ++
>>>>>>   1 file changed, 2 insertions(+)
>>>>>> 9e59687e1e0c65aab1fa4b25b861a64de82e4c97  yadif_video_props.patch
>>>>>> Index: libavfilter/vf_yadif.c
>>>>>> ===================================================================
>>>>>> --- libavfilter/vf_yadif.c	(revision 25329)
>>>>>> +++ libavfilter/vf_yadif.c	(working copy)
>>>>>> @@ -177,6 +177,8 @@
>>>>>>
>>>>>>       if (is_second)
>>>>>>           avfilter_start_frame(ctx->outputs[0], yadif->out);
>>>>>> +    avfilter_copy_buffer_ref_props(yadif->out, yadif->cur);
>>>>>> +    yadif->out->video->interlaced = 0;
>>>>>>       avfilter_draw_slice(ctx->outputs[0], 0, link->h, 1);
>>>>>>       avfilter_end_frame(ctx->outputs[0]);
>>>>>>
>>>>>
>>>>> this should remove yadif->out->pts = yadif->cur->pts;
>>>>
>>>> Ok, so the change be moved in start_frame ?
>>>
>>> probably not
>>>
>>> my pts copy was just in start_frame because that way the second field
>>> would end with the default pts which should be AV_NOPTS_VALUE
>>>
>>>
>>>>
>>>>> and the second frame with field->frame mode should have its pts set to
>>>>> unknown or interpolated
>>>>
>>>> I was fearing that.
>>>
>>> it should be trivial, we have the next frame already its a matter of
>>> if either is AV_NOPTS make it AV_NOPTS otherwise (a+b)/2
>>>
>>
>> Humm, something like that ?
>>
>> -- 
>> Baptiste COUDURIER
>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
>> FFmpeg maintainer                                  http://www.ffmpeg.org
> 
>>  vf_yadif.c |   12 ++++++++++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>> fa62f63f613efd8352ae7db258b1c8c865bccdbd  yadif_video_props.patch
>> Index: libavfilter/vf_yadif.c
>> ===================================================================
>> --- libavfilter/vf_yadif.c	(revision 25775)
>> +++ libavfilter/vf_yadif.c	(working copy)
>> @@ -175,8 +175,17 @@
>>  
>>      filter(ctx, yadif->out, tff ^ !is_second, tff);
>>  
>> -    if (is_second)
>> +    avfilter_copy_buffer_ref_props(yadif->out, yadif->cur);
>> +    yadif->out->video->interlaced = 0;
>> +    if (is_second) {
>> +        if (yadif->next->video->pts != AV_NOPTS_VALUE &&
>> +            yadif->cur->video->pts != AV_NOPTS_VALUE)
>> +            yadif->out->video->pts =
> 
>> +                (yadif->next->video->pts + yadif->cur->video->pts) / 2;
> 
> (a+b)/2 can overflow
> (a&b) + ((a^b)>>1) should be better
> we might want this as macro in libavutil

Well fixed.

> also nipikcery says if else should have a {} to minimize future diffs
> 
> except that it lgtm if tested

Tested, fixed and applied. See cvslog :)

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org