[mythtv] AVSync2 Refinements

[Tim Pletcher]

On Mon, Dec 9, 2019 at 10:37 AM Tim Pletcher <pletchtd at gmail.com> wrote:

>
>
> On Mon, Dec 9, 2019 at 10:11 AM Tim Pletcher <pletchtd at gmail.com> wrote:
>
>>
>>
>> On Mon, Dec 9, 2019 at 9:06 AM Mark Kendall <mark.kendall at gmail.com>
>> wrote:
>>
>>> On Mon, 9 Dec 2019 at 00:32, Tim Pletcher <pletchtd at gmail.com> wrote:
>>>
>>> > Thank you for your insights Peter.  I have been testing and debating
>>> whether to actually include any filtering or not. I do think true
>>> proportional control with gain settings less than 1 is better as this
>>> minimizes the control oscillation once near setpoint while still giving
>>> somewhat faster initial sync. I do notice a significant difference in the
>>> amount of variability from my two intel apollo lake based front-ends using
>>> VAAPI versus my combined i5 frontend/backend using an NVidia GT1030 with
>>> NVDEC so I think the proposed change is more impactful on low end hardware.
>>> >
>>> > As David suggests, I will create a ticket and submit a patch.
>>>
>>> Tim
>>>
>>> Many thanks for this - very handy:)
>>>
>>> I recently got caught out by the fluctuating av sync on the raspberry
>>> pi. I've taken the liberty of producing a more extensive patch
>>> (attached). This adds your filtering as well as removing all of the
>>> the old a/v sync code and the settings for AVSync2 - i.e. it moves
>>> over completely to the new av sync code and removes the audio
>>> correction adjustment. (N.B. the patch is against today's  latest
>>> master - I pushed some cleanup of  MythDVDPlayer and the patch won't
>>> apply cleanly before that).
>>>
>>> It appears to work very well. On two (linux) desktop dev machines and
>>> a pi 4 I can't see any issues - playback is nice and smooth. As you
>>> say, initial sync seems to be much better as well.
>>>
>>> Certainly gets my seal of approval!
>>>
>>> One question - last_fix - should that be carried over between calls to
>>> AVSync()? At the moment it is a local that gets reset between each
>>> call - which I'm not sure is the intended behaviour.
>>>
>>> Thanks again and regards
>>> Mark
>>>
>>>
>> Hi Mark,
>>
>> Thank you for looking at this in detail.
>>
>> You are exactly right in that last_fix doesn't get carried over between
>> calls. The last_fix value is actually needed to correctly do the signal
>> filtering to the control loop.  Over the weekend, I eventually figured out
>> the filtering wasn't actually working after adding more debugging detail to
>> the logging for better visibility of what was happening mathematically
>> between calls.  This is also the reason I arrived at the conclusion that
>> filtering is probably not really needed since performance is good anyway
>> and a filter brings the drawback of adding lag to the control response.
>>
>> In reality, with the broken filter application, the code was effectively
>> using a control gain of (0.4) * (1 - 0.9) = (0.04).  This setting seems to
>> still provide rapid initial sync while preventing over-control in response
>> to natural audio timestamp jitter.
>>
>> The patch looks great.  Based on discussion above, I think we should
>> remove the filtering and set:
>>
>> s_av_control_gain = 04;
>>
>> The last_fix and s_sync_fc references should go away alltogther and the
>> new control calculation becomes:
>>
>> float fix_amount = audio_adjustment * sign * s_av_control_gain;
>>
>> Should I still create a Trac ticket for this?
>>
>
> Correction:
>
> It should be:
> s_av_control_gain = 0.04;
>
> Tim
>

I revised Mark's patch with the updates I discussed above and created
ticket #13536 <https://code.mythtv.org/trac/ticket/13536>.  The updated
patch is attached there.

Everything compiled fine for me and performance looked good in the limited
testing I performed.

Tim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mythtv.org/pipermail/mythtv-dev/attachments/20191209/ca3c5e3b/attachment.htm>