[vlc-devel] [PATCH] transcode: Correctly reset video drift

[Laurent Aimar]

On Thu, Jun 07, 2012 at 05:43:50PM +0200, Aurélien Nephtali wrote:
> Hi,
> 
> On Thu, Jun 7, 2012 at 5:29 PM, Hugo Beauzée-Luyssen <hugo at beauzee.fr> wrote:
> > On Thu, Jun 7, 2012 at 4:55 PM, Aurélien Nephtali
> > <aurelien.nephtali at gmail.com> wrote:
> >> Hi,
> >>
> >> See attached patch.
> >>
> >> Thanks.
> >>
> >> --
> >> Aurélien Nephtali
> >
> > Hello,
> >
> > Please use spaces instead of tabs.
> Fixed, sorry about that.

I think your patch is incomplete to fully fix the issue.
The code is:

>     if( p_sys->b_master_sync )
>     {
>         mtime_t i_pts = date_Get( &id->interpolated_pts ) + 1;
>         if (unlikely ( p_pic->date - i_pts > MASTER_SYNC_MAX_DRIFT
>               || p_pic->date - i_pts < -MASTER_SYNC_MAX_DRIFT ) )
>         {
>             msg_Dbg( p_stream, "drift is too high, resetting master sync" );
>             date_Set( &id->interpolated_pts, p_pic->date );
>             i_pts = p_pic->date + 1;
I don't understand the need of this line (after the comment below).
>         }
>         date_Increment( &id->interpolated_pts, 1 );
I think i_pts should be set to the result of the increment.
> 
>         if( unlikely( b_need_duplicate ) )
>         {
[Here your patch add a missing date_Increment() ]
>            if( p_sys->i_threads >= 1 )
>            {
>                /* We can't modify the picture, we need to duplicate it */
>                p_pic2 = video_new_buffer_decoder( id->p_decoder );
>                if( likely( p_pic2 != NULL ) )
>                {
>                    picture_Copy( p_pic2, p_pic );
>                    p_pic2->date = i_pts;
>                }
>            }
>            else
>            {
>                block_t *p_block;
>                p_pic->date = i_pts;
>                p_block = id->p_encoder->pf_encode_video(id->p_encoder, p_pic);
>                block_ChainAppend( out, p_block );
>            }
>        }
>     }

Could you recheck against your test cases?

Regards,

-- 
fenrir