[mythtv] vsync and frame timing adjustments

[Doug Larrick]

Changing the subject line on this to something more descriptive.

Bruce Markey wrote:
> Doug, I hope I didn't sound like I was arguing for argument's
> sake. If so, I apologize. I want the code to work correctly
> and honestly believe that the cause of the problem you see
> can not be as you've describe. 

Absolutely.  I also want the code working properly.  I hope we can 
arrive at a solution that works properly for both of our systems :-).  I 
have a focus on HDTV, because (a) that's what I have, and (b) I'd like 
the FCC to see that an open-source, PC-based HDTV PVR is as good as or 
better than any commercial offering, and that their broadcast flag 
regulation is stifling innovation.  This focus necessarily means that I 
have modern, powerful hardware and a recent kernel; I realize not 
everyone has that luxury.

I'm not addressing all of what you wrote in this reply.  I'm short on 
time this morning and need to run some experiments to back up what I'm 
saying with data.

> Here's something we can agree on: I am ignoring the refresh rate of the 
> display. That's the point of the description. I am aware
> that the refresh rate of the display can be different and I know
> that each of the timing methods need to do the right things for the
> right reasons when the refresh rate of the monitor is different
> than the NTSC 29.97fps of the recording.

I believe this to be the root cause of the disagreement here.  In my 
master vision (such as it is) of this section of code, vsync.cpp is 
responsible for following the video hardware.  The a/v sync code in 
NuppelVideoPlayer is responsible for resolving differences between the 
actual and desired framerates.  I see now that this is not the vision of 
(your?) original ExAVSync code, and that's what we need to be working 
through.

The basic question that needs answering here is: on a frame-by-frame 
basis, do we want to follow the timing requested by the video stream 
blindly, letting the video hardware bin the frames into refresh 
intervals, or do we want to manage which frame goes into which interval?

Certainly for video-as-timebase a/v sync, it's very important that vsync 
not arbitrarily slip a cog outside the control of the a/v sync code. 
All adjustment (outside gross adjustments when (re)aquiring a/v sync 
initially) should be done to the audio stream.

I believe this same logic can be applied to audio-as-timebase: vsync 
very predictably follows the video card timing, and adjustments are done 
in NuppelVideoPlayer when a/v sync drifts too far.

This software architecture is cleaner as well as easier to explain and 
maintain; that was the primary goal of my rewrite.  And that's the root 
of my objection to your commit: both parts of it break this separation 
of duties.

> I see. I'd be interested to know a little more about what this
> crappy display looks like. Do you mean multiple stutters clumped
> together or frames tearing or something else?

It's multiple stutters clumped together.  Actually, it's as if every 
other frame were skipped.  Like I said, I hope to do more experiments 
tonight, but looking back at my notes, I think it's possible the 
following is happening:
* show frame n; m_delay has wrapped around to next refresh
* prepare frame n+1; show it immediately because we think we're
    behind where we belong.  It stomps on frame n.
* show frame n+2; m_delay is still very close to the edge, and may
   have wrapped around due to scheduler inconsistency.  If so:
   lather, rinse, and repeat.

Like I said, I need to rerun experiments to clarify here, so don't 
dissect that theory too closely :-)

[excellent description of process steps snipped]

> Or is the problem a cluster of stutters? You said it happens
> when this value approaches an entire refresh interval. Far away
> one refresh is the same as close to another. If the value is
> -16650 when the loop falls through, on the previous refresh it
> was 16 (plus or minus inaccuracies). If so, this may be a
> form of the straddle problem that is not being handled correctly.
> 
> If the delay values after it fell through were: -16644, -16661,
> -16638, -16647, ... no problem so far ... -16658, -1, ...opps...
> -16649, -11, -23, -16644, -16644. This would be the straddle
> problem. It's so close to zero on the previous refresh that it
> sometimes goes over the edge and some frames are drawn one
> refresh earlier than the others around them. However, this
> shouldn't happen because of KeepPhase. It doesn't matter which
> side it is drifting from, nexttrigger is moved the first time
> something falls between 0 and -1000. In this case, -1.

I'll post some data when I have it.

> Right, I knew that and that's why I took it out. It doesn't
> need to be penned into a range. This was forcing it to be
> tugged along with the refresh timing. However, nexttrigger
> should solely reflect the frame intervals and avsync and the
> frame timing should adjust to it and not the other way around.
> It's my bad that the straddle fix gave the impression that it
> should be otherwise.

Again we get to the philosophical difference between syncing to the 
video card's refresh and to the video stream's timecodes.  What's the 
point (aside from the straddle problem) of paying attention to when the 
video card refreshes if you're not going to do anything about it?  Might 
as well use /dev/rtc and let the frames fall where they may.

> Most glitches are
> a result of kernel scheduler events rather than adjustment
> or errors in the calculating the target time. 

An experiment I intend to try is to run the videoout thread with 
realtime priority (SCHED_FIFO).  The software architecture is:
  * set mythfrontend suid root
  * spawn a priveleged thread upon entry, then drop privs for the main 
execution of mythfrontend
  * priveleged thread accepts events from other threads, specifically 
requests to call pthread_setschedparam on their behalf.

I believe this scheme would alleviate much of the scheduler uncertainty, 
especially on slower hardware.  But that's a topic for another discussion.

-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/20040825/29fa7f5e/signature.pgp