[mythtv] Re: [mythtv-commits] mythtv commits

[Doug Larrick]

Bruce Markey wrote:
> Nexttrigger should normally be incremented by exactly one frame
> interval and mark out the cadence of the frames. However, as
> you know, audio and video can drift so it is adjusted plus or
> minus one refreshrate which you pass as avsync_adjustment when
> necessary. Nothing else should really be fudging that cadence
> so that nexttrigger reflects the steady pace within a frame or
> so of matching the video exactly with the audio.

What you're ignoring in the above description is that the refresh rate 
of the display is often not exactly equal to the frame rate of the 
video, due to inaccuracy in modeline, driver or hardware limitations, or 
(mis)configuration (such as running a 29.97 Hz video on a 60 or (worse) 
75 Hz desktop system).

When this happens, the delay as measured by KeepPhase will creep in one 
direction or the other.  If it happens to creep upwards (toward 0), the 
errors will accumulate and the calculated delay will drift until it hits 
the limit you left in.  If it happens to creep downwards, it will 
eventually overflow one video refresh interval and we'll have a 
one-refresh hiccup that looks like an a/v sync correction (i.e. all but 
invisible).  Well, almost.  With HDTV-sized frames, the display starts 
to look crappy as this value approaches an entire refresh interval in 
magnitude.  My theory is that we're not giving the card enough time to 
process the frame in between sending it the data and telling it to show.

I have instrumented the code in the past to demonstrate this effect to 
my satisfaction; unfortunately I have not kept this instrumentation code 
up to date with the current reimplementation.  I have a feeling I will 
need to redo this work in order to convince you, but my notes and 
conclusions from before were quite clear.  The effect was also worse on 
an nVidia MX440 (slower) than on an FX5200.

> It isn't so
> important that nexttrigger is moved in one direction or the other
> or if it is checked for on one side or the other (but I do have
> good reasons for both choices =), it is only important that delay
> is deflected from being repeatedly near zero. Whether m_delay
> ends up at -1001 or -15500 (divide by two for bob and convert to
> metric for PAL =) or anywhere in between doesn't matter as long
> as the nexttriggers are one frame interval apart and we hit the
> next refresh after each.

Actually, for the reason I gave above, it *does* matter.  At least for 
HDTV-sized frames, if m_delay is too small (nearer to the -15500 end), 
the card doesn't output video properly as I described above.  The code 
you removed was simply keeping this value between -1000 and (roughly) 
-8000 rather than -15500.  The same amount of a/v divergence occurs 
either way, it's just a matter of when it gets corrected, and by what 
code.  Either the vsync code skips a tooth and inserts a hiccup, or the 
A/V sync code notices the out-of-sync condition and takes its own 
corrective action.

> There are a few problems with the other adjustment. First, it
> moves the position of nexttrigger in one direction independently
> from the avsync_adjustment calculations. 

...though they will be affected by these adjustments just like by other 
inaccuracy in when we send the frame.

> This pushes nexttrigger
> farther from the timecodes so it will eventually need to be
> adjusted again (this is also true of the straddle fix). 

Yes, this is true... but it will be taken care of by adjustments in the 
a/v sync code as it should be.  In fact, if a user is using 
video-as-timebase, the A/V sync code will warp the *audio* to fix this 
problem, and absolutely does not desire the vsync code to automatically 
hiccup by wrapping around to the next frame interval.

> Second,
> it strives to keep nexttrigger a certain distance from the
> refresh time so it ends up moving nexttrigger in parallel with
> the refresh cycle instead of parallel with the timecodes. This
> is counterproductive. Nexttrigger needs to be the mechanism for
> the avsync to shift the video timing when it drifts. It should
> be guided by the timecodes and not guided by the refreshes.

But it *is* guided by the timecodes... by the a/v sync method.  This 
videocard sync code should absolutely track what the videocard is doing.

For the record, I also agree with Jesper Sörensen that the other part of 
your patch belongs in NuppelVideoPlayer, as it's A/V sync logic, not 
video-display sync logic.  A major focus and reason for my rewrite was 
to clean up the distinction between the two, and I'd hate to see it 
clouded so quickly.

-Doug
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : http://mythtv.org/pipermail/mythtv-dev/attachments/20040823/6748c07c/signature.pgp