[mythtv] [mythtv-commits] Ticket #7964: Predictively skip frames to smooth out timestretch

[Taylor Ralph]

Mark,

On Sat, Jan 8, 2011 at 7:34 AM, Mark Spieth <mark at digivation.com.au> wrote:
> On 1/6/2011 1:47 AM, MythTV wrote:
>>
>> #7964: Predictively skip frames to smooth out timestretch
>> -------------------------------------+--------------------------
>>  Reporter:  jppoet@…                 |          Owner:  tralph
>>      Type:  enhancement              |         Status:  assigned
>>  Priority:  minor                    |      Milestone:  0.25
>> Component:  MythTV - Video Playback  |        Version:  head
>>  Severity:  medium                   |     Resolution:
>>  Keywords:  timestretch playback     |  Ticket locked:  0
>> -------------------------------------+--------------------------
>>
>> Comment (by tralph):
>>
>>  Mark, I've committed the bulk of the changes and removed some dead code.
>>  I'd like to get everything wrapped up but need some questions answered.
>>
>>  vsync change questions
>>  1. For the hardware sync methods you changed the value of 'n' to a new
>>  equation. Could you explain this? I have no way of testing these changes.
>>
> what is needed is the whole integer/trunc of the delay wrt the refresh
> interval as that is the basis of the hardware vsync timing.
> if it is less, then the time for the next vsync wont have expired.
>>
>>  2. Also you are returning an 'int' for the WaitForFrame but it's only
>>  effective for the hardware vsync methods. Could also explain why this is
>>  needed?
>>
> the software sync mechanisms are exact.
> the hardware ones are quantized to the refresh interval. the value returned
> is the time sinc ethe refresh event/irq occurred.
> this allows the AVSync algo to completely account for all time. turn on the
> loggingand you will see the avdel %4 is exact except for slippage.
>>
>>  3. Which hardware vsync methods have you tested? Only DRM works anymore.
>>
> openglvsync only (re-enabled after daniel disabled it).
> it works so well for me on 2 machines so I didnt want to change. nvidia
> drivers are being used with Xv
> if you know fo an easy way to get drmsync going let me know and Ill test.
> check with jppoet on sync method. not sure what he is using.
>>
>>  4. Why do you avoid the negative trigger accumulation when doing
>>  predictive skip in vsync.cpp. Specifically the line, "if (ret_val<
>>  -m_frame_interval&&  m_frame_interval>= m_refresh_interval)"
>>
> see point 1. no negative sync time due to equation. delay is always after
> next/earliest sync event.
>>
>>  mythplayer questions
>>
>>  1. We used to allow +/- 1.5 frame intervals of AV-sync slip before adding
>>  compensation to the next trigger. Why did you change it +/- 1 refreshrate
>>  interval?
>>
> mainly due to the fact that refresh rate is the fundamental output timing
> rate, not the input rate which can be anything. the adjustment needs to be
> wrt the output rate so these variables are used.
> however, no real proof that this is any better or worse but just IMO more
> logically correct and consistent.
>>
>>  2. Why do you use the avsync_used variable instead of avsync_avg for the
>>  slip adjustment check?
>>
> avsync_used is the minimum of the delayed estimate and the current value of
> avsync (with the delay from last vsync event included).
> this improves lock time as the smallest value is the better one to use just
> in case the instantaneous value deviates from the steady state. also speeds
> up initial lock time as avdel is 0 initially.
>>
>>  3. You removed the 'lastsync = true' from the +/- 1 refreshrate interval
>>  block. Is this because you want the changes to occur on every new frame.
>>  It seems the old code would only make adjustments every other frame
>>  because of this and maybe that shouldn't have been the case.
>>
> lastsync is now only used for massive deviations so that the finegrained
> compensation is not invoked if a previous coarser adjustment was indicated.
>>
>>  4. Why is the prevtc != 0 check needed?
>>
> initial conditions. improves initial lock as prevtc is 0 when AVSync is
> first run.
>>
>>  5. It seems you've added a new function to retrieve currentaudiotime and
>>  you update the variable in more instances. What's the reason for this?
>>
> video only has no audio so there is no audio time. needs to be catered for
>>
>>  I believe that covers everything. Thanks again for the patch. The bulk
>>  that has been committed makes a substantial difference for time stretch
>> of
>>  60fps material.
>>
> cant see how as you only changed a few minor details. the important stuff is
> still missing.
>
> I think Ive answered everything correctly but its been a while and had to
> revisit the details.
>
> sorry for the delayed response too. had a few issues and time cutting over
> my tree to git with my patch tree (about 30) but mostly good now.
>
> cheers
> mark
>

One other question is why you are bypassing KeepPhase() with the
return in the hardware vsync methods?

I've checked in everything except for the vsync_delay_clock code in
mythplayer and the remaining vsync code. Once the KeepPhase is
explained I'll commit the rest.

Thanks.
--
Taylor