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