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

[Michael Niedermayer]

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

also nipikcery says if else should have a {} to minimize future diffs

except that it lgtm if tested
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101121/77f0a178/attachment.pgp>