[mythtv] [mythtv-commits] Ticket #7964: Predictively skip frames to smooth out timestretch
taylor.ralph at gmail.com
Sat Jan 8 22:26:34 UTC 2011
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
> 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
> 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
>> 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.
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.
More information about the mythtv-dev