Alice0775 White Reporter
	Description • 9 years ago

Build Identifier:
https://hg.mozilla.org/mozilla-central/rev/4d2d97b3ba34
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0 ID:20150320030211


Reported http://forums.mozillazine.org/viewtopic.php?p=14079563#p14079563

Steps to reproduce:
1. open https://www.youtube.com/watch?v=fjNtMkcQ6lQ

Actual results:
~4.sec this Youtube HTML5 mp4 video just stuck and starts buffering forever


Setting media.mediasource.mp4.enabled = false helps

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 2 • 9 years ago

I was unable to reproduce on Win 7 with 37.0b7 or 39.0a1 (2015-03-23). Could this be network related?

Is this reproducible 100% of the time on your system or intermittently reproducible?

	Edo Karabas
	Comment 3 • 9 years ago

hmm, seems to be working now for me as well....
no, it "was" 100% reproducible, I tried several versions of Firefox from nightly to beta, x86/x64, HWA on/off... 
don't think it's network related, I have 5 Mbps connections but beside that, every other video which I played work fine...
don't know what happend, could be youtube issue, which is solved now

@Alice: Are you able to reproduce it now?

	John
	Comment 4 • 9 years ago

This still happens to me a majority of the time. It's not everytime, but most of the time.

Oddly, it seems to be much less of an issue if a manually select 720p60fps on youtube. If I just leave it on Auto (which selects 480p because I'm not watching full screen), it almost invariably ends up stalling at some point (normally before 10s in). I can then select a time just slightly later in the video, and it will play for a short time again and then stall.

	Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)
	Comment 5 • 9 years ago

Alice - can you install https://github.com/doublec/aboutmedia/raw/master/aboutmedia.xpi and paste in what you get from about:media?

	Alice0775 White Reporter
	Comment 6 • 9 years ago

I cannot reproduce the problem on the but build as well as latest Nightly.

	Alice0775 White Reporter
	Comment 7 • 9 years ago

s/but/bad/

	John
	Comment 8 • 9 years ago

I installed the addon (in a new profile) as I was still having problems and here is the output:

https://www.youtube.com/user/FRANKIEonPCin1080p/videos
	
	currentTime: 0
https://www.youtube.com/watch?v=mCB5uqAZNxQ
	mediasource:https://www.youtube.com/6e9d5ebb-936f-429d-9c2b-5c5a4b2fc087
	currentTime: 4.87619
		SourceBuffer 0
			start=0 end=4.899092
		SourceBuffer 1
			start=0 end=5.033333
			start=5.066666 end=20.233333
			start=20.266666 end=25.3
			start=25.333333 end=40.5
			start=40.533333 end=50.633333
			start=50.666666 end=70.9
			start=70.933333 end=86.1
	Internal Data:
	Dumping data for reader 3b685800:
		Dumping Audio Track Decoders: - mLastAudioTime: 4.876189
			Reader 0: d627c00 ranges=[(0.000000, 4.899092)] active=true size=82380
		Dumping Video Track Decoders - mLastVideoTime: 3.966666
			Reader 3: ddf2c00 ranges=[(70.933333, 86.100000)] active=false size=2097319
			Reader 2: 44720c00 ranges=[(40.533333, 50.633333), (50.666666, 70.900000)] active=false size=4185929
			Reader 1: 4471d800 ranges=[(25.333333, 40.500000)] active=false size=2091559
			Reader 0: c725400 ranges=[(0.000000, 5.033333), (5.066666, 20.233333), (20.266666, 25.300000)] active=true size=3506588

	Alice0775 White Reporter
	Comment 9 • 9 years ago

I can repro with https://www.youtube.com/watch?v=mCB5uqAZNxQ of comment#8

	Alice0775 White Reporter
	Comment 10 • 9 years ago

[Tracking Requested - why for this release]: Regression. I can repro latest beta as well

	Jean-Yves Avenard [:jya] Assignee
	Comment 11 • 9 years ago

Can reproduce it easily.

	Dumping Video Track Decoders - mLastVideoTime: 4.666666
			Reader 3: 1279e2800 ranges=[(50.666666, 55.700000), (55.733333, 75.966666), (76.000000, 111.433333), (111.466666, 116.500000)] active=false size=4829811
			Reader 2: 14930c800 ranges=[(20.266666, 25.300000), (25.333333, 40.500000), (40.533333, 50.633333)] active=false size=2267431
			Reader 1: 131a55800 ranges=[(10.133333, 20.266666)] active=false size=313508
			Reader 0: 130289800 ranges=[(0.000000, 5.033333), (5.066666, 10.100000)] active=true size=768672

Shockingly muxed file... Why the 0.0333s discontinuities aren't handled however ...

	Jean-Yves Avenard [:jya] Assignee
	Comment 12 • 9 years ago

It stalls waiting for audio, however after about 1 minute or so ; playback does resume...

Looks more like a YouTube issue than anything else .... unless got an idea..

	Jean-Yves Avenard [:jya] Assignee
	Comment 13 • 9 years ago

The first moof added is for 10s of audio, but there's only 4s worth of data, the mdat atom is truncated.

https://www.youtube.com/watch?v=mCB5uqAZNxQ
	mediasource:https://www.youtube.com/a4cc2bbc-b976-b645-8ece-436e116ad8e3
	currentTime: 4.87619
		SourceBuffer 0
			start=0 end=4.899092
		SourceBuffer 1
			start=0 end=5.033333
			start=5.066666 end=10.1
			start=10.133333 end=70.933333
	Internal Data:
	Dumping data for reader 1425f8c00:
		Dumping Audio Track Decoders: - mLastAudioTime: 4.899092
			Reader 0: 14261c800 ranges=[(0.000000, 4.899092)] active=false size=82380
		Dumping Video Track Decoders - mLastVideoTime: 5.266666
			Reader 1: 14bfbe000 ranges=[(10.133333, 70.933333)] active=false size=830419
			Reader 0: 140b97800 ranges=[(0.000000, 5.033333), (5.066666, 10.100000)] active=true size=768672
	
Attaching the first three segments received.

it takes sometimes 2 minutes before the rest is received.

	Jean-Yves Avenard [:jya] Assignee
	Comment 14 • 9 years ago

Anthony can you have a look on those mp4 fragments.

Chrome handles this video nicely...

Only thing I can think of, and is what we discussed on IRC yesterday. We report the buffered range with what we have, including partial.

The spec for MSE states:
http://w3c.github.io/media-source/#sourcebuffer-segment-parser-loop
"6. If the append state equals PARSING_MEDIA_SEGMENT, then run the following steps:

    1. If the first initialization segment received flag is false, then run the append error algorithm with the decode error parameter set to true and abort this algorithm.
    2. If the input buffer does not contain a complete media segment header yet, then jump to the need more data step below.
    3. If the input buffer contains one or more complete coded frames, then run the coded frame processing algorithm."

The coded frame processing algorithm states:
http://w3c.github.io/media-source/#sourcebuffer-coded-frame-processing
"When complete coded frames have been parsed by the segment parser loop then the following steps are run:"

so here, it could constructed as we can handle partial mdat and reports the frames we already have.

However, in http://w3c.github.io/media-source/isobmff-byte-stream-format.html
we have:
"The user agent must run the append error algorithm with the decode error parameter set to true if any of the following conditions are met:
...
The Media Data Boxes do not contain all the samples referenced by the Track Fragment Run Boxes (trun) of the Movie Fragment Box."

So we should have thrown an error there according to ISO BMFF ; we have a discrepancy. 

I'm going to check how this particular playback behaves when we only report the complete moof buffered once the mdat is complete.

but at this stage, everything points to a YouTube problem.

	Jean-Yves Avenard [:jya] Assignee
	Comment 15 • 9 years ago

only report data if all trun's data is there. The stall is removed with this. This proves my theory.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 16 • 9 years ago

(In reply to Jean-Yves Avenard [:jya] from comment #15)
> only report data if all trun's data is there. The stall is removed with
> this. This proves my theory.

Are there issues that prevent us from making this change? If not, do you want to request review and see about getting this small fix into 37? (We're building the RC today.)

	Jean-Yves Avenard [:jya] Assignee
	Comment 17 • 9 years ago

No this is a hack. It will cause to start playback much later than it could in all other videos. I get a stall later in a similar fashion we aren't fed data. Going to contact YouTube to find out what's going on.

Hopefully I get an answer very quickly.

	Jean-Yves Avenard [:jya] Assignee
	Comment 18 • 9 years ago

Don't report discontinuities of less than 35ms. We have 3 differents way of dealing with discontinuities, all of them using different values: 20ms in sourcebuffer.appendBuffer, 1us per frames in the MoofParser (so here 844us max), and 125ms in the MediaSourceReader./mach build Bump the first to to 35ms.

	Jean-Yves Avenard [:jya] Assignee
	Comment 19 • 9 years ago

Lawrence, I feel this patch is safe enough to go in the current beta. I probably won't get an answer from Anthony before tomorrow now.

I got the answer I was waiting for. Due to the small discontinuity in the video segments, and as it is over than our fuzz we reported it.

Seeing our MSE is only enabled for YouTube, getting YouTube to work properly is really the only thing that matters.

The chances of regression are non-existent with that patch. It doesn't affect how we play our file, just on how to report data. And we report it exactly as what YouTube is expecting to and is also how Chrome / IE and Safari smooth things. And according to youtube even more aggressively than that.

Here is the response I receive:
"Our buffering algorithm is based on the video's buffered ranges. We intentionally re-slice the audio data such that we don't append audio too far past the end of the video buffered range that contains the current time. If the current time is outside the video buffered range, the video buffered range ends early, or the video has discontiguous buffered ranges, audio appends will be capped. 

The audio append behavior works around bugs on several platforms where audio will continue to play out even if video isn't present. Such platform behavior violates the MSE spec, but the bugs are baked in deep and are in many old platforms, so our workaround will remain.

In any case, the issue here is that the video file at 360p produces a lot of discontinuities in the Media Source timeline. I note that other browsers don't present the same issue; then again, IE and Chrome both have substantial padding constants to eat up small gaps in the timeline, and Safari has some rather expensive timestamp rewriting code that runs over all BMFF segments to fix them up for a possibly-related issue in our media involving frame reordering and composition timestamp offsets.
"

It would be easy for YouTube to change the behaviour of theirs, but would they do it is another matter.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 20 • 9 years ago

We have already spun Firefox 37.0 RC. If we spin an RC2, I think we should take this fix. I don't know that it is worth spinning RC2 just for this fix as I don't know how widespread the impact of this bug is. Let's start by getting that r+ and an uplift approval request on the bug.

	Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)
	Comment 21 • 9 years ago

Comment on attachment 8583030 [details] [diff] [review]
Do not report discontinuities less than 35ms

Review of attachment 8583030 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libstagefright/binding/MoofParser.cpp
@@ +461,5 @@
>  
>      mMdatRange = mMdatRange.Extents(sample.mByteRange);
>    }
> +  // TODO: Adjust rounding error.
> +  // mMaxRoundingError += aMdhd.ToMicroseconds(sampleCount);

I'd prefer that we leave this adjustment in unless there is a compelling case to remove it.

	Jean-Yves Avenard [:jya] Assignee
	Comment 22 • 9 years ago

Comment on attachment 8583030 [details] [diff] [review]
Do not report discontinuities less than 35ms

Approval Request Comment
[Feature/regressing bug #]: 1145815
[User impact if declined]: YouTube may stall
[Describe test coverage new/current, TreeHerder]: little, ensured that particular video properly played.
[Risks and why]: Low. We provide YouTube what it was expecting in the first place. Ultimately, the issue is on Youtube's side. Both Chrome and IE applies even more aggressive smoothing (150ms from Chrome)
[String/UUID change made/needed]: None

	Jean-Yves Avenard [:jya] Assignee
	Comment 23 • 9 years ago

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6516456a0a63

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/6516456a0a63

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 25 • 9 years ago

jya - Now that this has landed on m-c, can you please spend a bit more time verifying the fix before we ship the change in 37 RC2?

Florin - Can your team please do some exploratory testing around this change in tomorrow's Nightly build?

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 26 • 9 years ago

Comment on attachment 8583030 [details] [diff] [review]
Do not report discontinuities less than 35ms

This is a low risk fix for which the approach has been confirmed with YouTube. As we're going to spin a 37 desktop RC2, let's take this patch to fix buffering issues on YouTube. Beta+ Aurora+

	Ryan VanderMeulen [:RyanVM]
	Comment 27 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/d9497593e7aa

	Ryan VanderMeulen [:RyanVM]
	Comment 28 • 9 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/b41c57eefd69

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 29 • 9 years ago

(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #25)
> jya - Now that this has landed on m-c, can you please spend a bit more time
> verifying the fix before we ship the change in 37 RC2?
> 
> Florin - Can your team please do some exploratory testing around this change
> in tomorrow's Nightly build?

Scratch that. We have the other fix that we need for RC2 so we're going to build tonight. Please test with 37.0 RC2.

	Ryan VanderMeulen [:RyanVM]
	Comment 30 • 9 years ago

https://hg.mozilla.org/releases/mozilla-release/rev/b41c57eefd69

	Vasilica Tamas, QA [:vtamas]
	Comment 31 • 9 years ago

I was unable to reproduce this issue with https://www.youtube.com/watch?v=fjNtMkcQ6lQ link from comment 0, but I successfully reproduced it with https://www.youtube.com/watch?v=mCB5uqAZNxQ link from comment 8 using Firefox 39.0a1 (2015-03-19) on Windows 7 64bit.

Verified fixed on Firefox 39.0a1 (2015-03-27), Firefox 38.0a2 (2015-03-27), Firefox 37 RC build 2 (20150326190726) using Windows 7 64bit. 

Exploratory testing was also done around this bug on Firefox 37 RC build 2, but no new issues were found.

Based on the above mentions I am marking this bug as verified.