[MPlayer-dev-eng] Potential source of A-V sync issues

[Oded Shimon]

On Fri, Feb 24, 2006 at 04:17:22PM +0100, Matthias Hopf wrote:
> On Feb 18, 06 22:35:50 +0200, Oded Shimon wrote:
> > On Wed, Feb 01, 2006 at 12:51:34AM +0100, Matthias Hopf wrote:
> > > While analyzing the code, I noticed, that skip_flag is initialized with
> > > 0 at the begining of the while (!at_eof) - but as only one frame can be
> > > skipped at a time, skip_flag might still be positive (but not
> > > negative!). The following patch moves the definition of skip_flag
> > > outside the main loop, but I haven't really tested it.
> > 
> > Look at the code, everything that deals with skipping makes sure skip_flag 
> > never raises above 1. You can pretty much add an assert that skip_flag is 
> > zero by the end of the mess.
> 
> Actually, you are right. I overlooked that skip_flag is still zero at mencoder:1282:
> 
> if (v_timer_corr >= mux_frametime && (skip_limit<0 || skip_flag < skip_limit)) {
>     v_timer_corr-=mux_frametime;
>     ++skip_flag; // skip
> }
> 
> It still looks like a bug too me - it should read skip_flag < 1 )

Are you refferring to the 'skip_limit' ?
skip_limit is completely broken, the only thing it controls is how many 
frames you can _duplicate_ at _once_, which is hardly useful at all... It 
also works to limit from doing any frame skips and dups at all.

there's really no need to test 'skip_flag < 1' there, because it will 
ALWAYS be zero there, it was inited with zero and no change is made to it 
until that code.

Also, IMO, a more correct approach to that entire code is merging 
v_timer_corr and v_pts_corr to a single value and testing against that...

- ods15