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

[Mark Spieth]

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


-- 
Mark Spieth, PhD
Digivation Pty Ltd
9 Catalina Ave
ASHBURTON VIC 3147
Australia
Phone: +61 4 11 515717 (0411515717)
Fax: +61 3 9885 5774