1105066 - [MSE] Sound synch issues with MP4 YouTube videos

Status: VERIFIED FIXED

(Core :: Audio/Video, defect, P1)

Description

[Jet Villegas (inactive)]

When MSE is enabled and the video is using MP4, seeking into videos causes sound to play back from earlier seek points. This may be a dupe of other seeking issues, so feel free to dupe as needed. Please let me know if I can provide anymore information.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Is this still an issue?

Comment 2

[Karl Tomlinson (:karlt)]

I saw this with a build based on 7b33ee7fd162 after seeking on https://www.youtube.com/watch?v=fhcZ6b2FSsk but then couldn't reproduce.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

I saw this in today's Nightly on my wife's Win 7 ultrabook and on my Win 8 work Desktop on this video:
https://www.youtube.com/watch?v=KqBin1GSkF4

It happened (on both occurrences) when I skipped an ad while playing in fullscreen mode.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Chris Pearce (:cpearce) from comment #3)
> I saw this in today's Nightly on my wife's Win 7 ultrabook and on my Win 8
> work Desktop on this video:
> https://www.youtube.com/watch?v=KqBin1GSkF4
> 
> It happened (on both occurrences) when I skipped an ad while playing in
> fullscreen mode.

I playing that video on Android and the AV sync is up the boohai.

Comment 5

[Matt Woodrow (:mattwoodrow)]

I see logs like this when it's happening:

1342992384[14020b670]: Decoder=12aab53a0 playing video frame 3632866666 (queued=12, state-machine=7, decoder-queued=5)
1356075008[12e5cf710]: Decoder=12aab53a0 EnsureVideoDecodeTaskQueued isDecoding=1 status=1
1342992384[14020b670]: Decoder=12aab53a0 UpdatePlaybackPositionInternal(3632866930) (mStartTime=0)
1573171200[140209070]: AudioSink=13fe37030 playing 1024 frames of audio at time 3633876235

For some reason it looks like our audio is a full second ahead of the playback position, not sure why the AdvanceFrame logic in MDSM isn't fixing it.

Comment 6

[Matt Woodrow (:mattwoodrow)]

The GetClock() value being returned in AdvanceFrame matches (is close to) the video frame time.

We're definitely getting the value from ::GetAudioClock, though part of it comes from mAudioStartTime and part from mAudioSink->GetPosition().

I'm not sure how these two values are meant to match up with the presentation timestamp of the audio frames being played, but they don't seem be doing it right.

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Is there a seek before A/V async happenes?

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

The MDSM decodes a full second of audio ahead. It looks like that video is just broken becuse the AV sync is busted on IE as well.

Marcia - can you see if you can reproduce this issue and get a log? I did it in the past by seeking around while compiling.

Comment 9

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Chris - what do you make of c5?

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

The audio decode runs 1 second ahead of the video decode/playback position. When we skip the ad, maybe the MDSM's audio queue is full and not getting cleared properly when playback seeks ahead to after the ad?

Comment 11

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Steps to reproduce:

* Navigate to https://www.youtube.com/watch?v=HykiuFCly0A&t=4000
* Remove the |&t=4000| and press enter

Expected results:

The clip will start again at the beginning. This is what IE does.

Actual results:

The video indicates that it is at the same position but plays back a different point in the video with the wrong AV sync.

Comment 12

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

I can't reproduce this on IE or with MSE disabled.

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 1: make SeekPromise return the time we actually seeked to

Comment 14

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 2: Chain seeks in MediaSourceReader so that we seek audio to the same time as video

Comment 15

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Seek after switching reader

Comment 16

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f433e2cda307
https://hg.mozilla.org/integration/mozilla-inbound/rev/db991158a2fe
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0954dd9d40a

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f433e2cda307
https://hg.mozilla.org/mozilla-central/rev/db991158a2fe
https://hg.mozilla.org/mozilla-central/rev/a0954dd9d40a

Comment 18

[Ralph Giles (:rillian)]

Comment on attachment 8547231 [details] [diff] [review]
Part 1: make SeekPromise return the time we actually seeked to

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Audio/Video sync issues with YouTube playback.
[Describe test coverage new/current, TBPL]: Landed on m-c. Manual testing.
[Risks and why]: This isn't a small change, but is relatively straightforward, and fixes a clear problem. Only Part 1 affects non-MSE playback. We can't ship the feature without this one.
[String/UUID change made/needed]: None.

Comment 19

[Ralph Giles (:rillian)]

Beta approval request is for all three patches on this bug.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e16f64387888
https://hg.mozilla.org/releases/mozilla-beta/rev/154dac808616
https://hg.mozilla.org/releases/mozilla-beta/rev/a0ffac1b2851

Comment 21

[Marcia Knous [:marcia]]

I have spot checked a few different Windows machines including Win 7, Vista and Win 8. Following the STR in Comment 11 I haven't been able to reproduce any issues using the latest Nightly.

Mozilla/5.0 (Windows NT 6.0; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150121030203 CSet: 540077a30866

I will also spot check the betas to verify that this is no longer being seen.

Comment 22

[Bogdan Maris, Desktop Test Engineering]

I was unable to reproduce the initial issue using a Nightly build before the fix. 
I also used Firefox 36 beta 3 and latest Aurora builds on Windows 7 64bit, Windows 10 and Windows 8.1 64bit and did not reproduce either. Anthony, since you reproduced this can you please also verify that this is not reproducible anymore for you on 36 beta and Aurora?

Comment 23

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Works for me.

Comment 24

[Bogdan Maris, Desktop Test Engineering]

Great, thanks Anthony.