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

[Bruce Markey]

Doug Larrick wrote:
> mythtv at cvs.mythtv.org wrote:
> 
>> ---------------------------------------------------------------------------- 
>>
>> Changes committed by bjm on Mon Aug 23 01:44:30 2004
>>
>> Modified Files:
>>    in mythtv/libs/libmythtv:
>>         vsync.h vsync.cpp Log Message:
>> Fix a couple regressions. Re-implement the delay clipping when A/V sync
>> is way off. This prevents the series of freezes that may happen if the
>> initial timecodes are not even close. Update the comment and correct the
>> code to workaround the 'straddle' problem now called KeepPhase.
> 
> 
> Thanks for the clarification you added to the comment in KeepPhase, but 
> I must take objection to your removing the correction in the other 
> direction.  The code you deleted is important for HDTV to function 
> properly.  If the display time drifts too far away from the actual 
> retrace, the video hardware just doesn't have time to display such a 
> large frame, and we get jerky motion.

I've seen you write something to that affect before but I think
this might be a mis-understanding. This should have no impact
on any amount of time available from one frame update to the
next. The display time of the previous frame was on a retrace
and the display time of the next frame will be on the next
retrace after delay reaches 0. Where the nexttrigger theoretical
target time falls in between the refreshes in that cycle has no
influence on how much time there will be until the next refresh
and therefore how much time the video hardware has to display
a large frame.

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.

Without refresh info, the frames should be drawn at this cadence.
If the updates were exactly one frame interval apart, the motion
would be rock steady. But knowing when to draw a frame is not
the problem. Acquiring the CPU from the kernel scheduler during
the timeslice when the frame is due is the problem.

As you know (again ;-), with refresh info, the goal is to hit the
update on the refreshes. Here nexttrigger serves as a suggestion
but the real update should come on the next refresh. Nexttrigger
should still reflect where the A/V timcodes dictate and should
not need to be fudged. However, there is this unfortunate exception
where if m_delay ends up being too close to 0, it can fall on
either side (which I call the 'straddle' problem). 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.

By including the straddle fix, I kind of opened the door to make
it look like video timing is supposed to do magical things to
nexttrigger to fix magical problems but that's not true. This
is just one ugly workaround for one specific ugly problem. There
is no "correction in the other direction" needed just as long
as it's not stuck near zero and that the fix doesn't cause too
many unnecessary fudges.

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. This pushes nexttrigger
farther from the timecodes so it will eventually need to be
adjusted again (this is also true of the straddle fix). 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.

I removed the other adjustment from my own personal tree the day
it went in (and have dreaded having to explain this ever since ;-).
I can see from the std dev numbers, delay values, avsync variables,
etc. that the right things weren't happening.

I have no doubt that you believe that this impacts your HDTV in
some way but I fail to see how or what it could be because this
doesn't change the amount of time available for any task as you
suggest. I do see with that code I get poorer stats, more
adjustments and more glitches and this may be related to problems
some people have reported on the lists. I know that the straddle
fix takes care of a specific need. The other adjustment is a
hypothetical that might seem like it might be a good idea but
there is no specific explanation or proof that this fixes
anything that needs fixing.

--  bjm