Bug 1750388 Comment 33 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Martin Stránský [:stransky] (ni? me) from comment #32)
> I think this bug has two parts:
> 
> 1) When video decode is slow, we don't decode any frames but all ones are dropped. That applies to SW decode too.

This can be easily fixed by setting media.ruin-av-sync.enabled to true. That pref ensures that video queue contains at least one frame we can display:
https://searchfox.org/mozilla-central/rev/dd404f43c7198b1076fe5d7e05b1e6b1a03bdfeb/dom/media/mediasink/VideoSink.cpp#505
when this pref is set off (default) we drop all out-of-time frames regardless of the result. 

As for media.ruin-av-sync.enabled pref:
https://bugzilla.mozilla.org/show_bug.cgi?id=1414759#c4

media.ruin-av-sync.enabled causes the decode path to not drop frames that are late. This means we'll paint frames that are late, so A/V sync will be out. When the decode can't keep up video frames often come out of the decode pipeline after they're supposed to keep painting. There's a talos test that counts how many frames we can render per second. When I fixed our A/V sync logic to drop those frames, we got a talos regression, as we don't paint as many frames anymore (but the ones we do paint, we paint in time with the audio). So to placate the people who care about such things, we added a pref so that the gfx team can continue to test how fast their pipeline is at painting (the wrong thing).

I think that radical approach does not apply any more. When we're fails the playback in such radical way (nothing is visible) we should abandon the AV sync in flavor to show something at least.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #32)
> I think this bug has two parts:
> 
> 1) When video decode is slow, we don't decode any frames but all ones are dropped. That applies to SW decode too.

This can be easily fixed by setting media.ruin-av-sync.enabled to true. That pref ensures that video queue contains at least one frame we can display:
https://searchfox.org/mozilla-central/rev/dd404f43c7198b1076fe5d7e05b1e6b1a03bdfeb/dom/media/mediasink/VideoSink.cpp#505
when this pref is set off (default) we drop all out-of-time frames regardless of the result. 

As for media.ruin-av-sync.enabled pref:
https://bugzilla.mozilla.org/show_bug.cgi?id=1414759#c4

media.ruin-av-sync.enabled causes the decode path to not drop frames that are late. This means we'll paint frames that are late, so A/V sync will be out. When the decode can't keep up video frames often come out of the decode pipeline after they're supposed to keep painting. There's a talos test that counts how many frames we can render per second. When I fixed our A/V sync logic to drop those frames, we got a talos regression, as we don't paint as many frames anymore (but the ones we do paint, we paint in time with the audio). So to placate the people who care about such things, we added a pref so that the gfx team can continue to test how fast their pipeline is at painting (the wrong thing).

I think that radical approach does not apply any more. When we fail in such radical way (nothing is visible) we should abandon the AV sync in flavor to show something at least.
(In reply to Martin Stránský [:stransky] (ni? me) from comment #32)
> I think this bug has two parts:
> 
> 1) When video decode is slow, we don't decode any frames but all ones are dropped. That applies to SW decode too.

This can be easily fixed by setting media.ruin-av-sync.enabled to true. That pref ensures that video queue contains at least one frame we can display:
https://searchfox.org/mozilla-central/rev/dd404f43c7198b1076fe5d7e05b1e6b1a03bdfeb/dom/media/mediasink/VideoSink.cpp#505
when this pref is set off (default) we drop all out-of-time frames regardless of the result. 

As for media.ruin-av-sync.enabled pref:
https://bugzilla.mozilla.org/show_bug.cgi?id=1414759#c4

media.ruin-av-sync.enabled causes the decode path to not drop frames that are late. This means we'll paint frames that are late, so A/V sync will be out. When the decode can't keep up video frames often come out of the decode pipeline after they're supposed to keep painting. There's a talos test that counts how many frames we can render per second. When I fixed our A/V sync logic to drop those frames, we got a talos regression, as we don't paint as many frames anymore (but the ones we do paint, we paint in time with the audio). So to placate the people who care about such things, we added a pref so that the gfx team can continue to test how fast their pipeline is at painting (the wrong thing).

I think that radical approach does not apply any more. When we fail in such radical way (nothing is visible) we should abandon the AV sync in favor of showing something at least.

Back to Bug 1750388 Comment 33