Closed Bug 1259916 Opened 8 years ago Closed 8 years ago

youtube video stops at approx 10:00

Categories

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

48 Branch
Unspecified
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 --- unaffected
firefox48 --- fixed

People

(Reporter: alice0775, Assigned: jya)

References

Details

(Keywords: regression, reproducible)

Attachments

(3 files)

[Tracking Requested - why for this release]: Playback problem of youtube video

Build Identifier:

Reproducible: always

Steps To Reproduce:
1. Open youtube long video  e.g., https://www.youtube.com/watch?v=eyU3bRy2x44


Actual Results:
Video stops at 09:54.
A throbber is spinning.

Expected Results:
Video should playback properly. Should not stop.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8d7ed429bf8537950c26158d9044b6cf1871b9ee&tochange=9ffa65c64e14209ebd0ce7a44b7254a6c5fc6b9e

Regressed by: Bug 1216460
Flags: needinfo?(jyavenard)
Flags: needinfo?(esawin)
Forgot 
Build Identifier:
https://hg.mozilla.org/mozilla-central/rev/b2dbee5ca727e87bdaeab9ab60fb83df2a9846a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 ID:20160325085113


Graphics
--------

Adapter Description: AMD Radeon HD 6450
Adapter Drivers: aticfx64 aticfx64 aticfx64 aticfx32 aticfx32 aticfx32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM: 1024
Asynchronous Pan/Zoom: wheel input enabled; touch input enabled
ClearType Parameters: Gamma: 2200 Pixel Structure: R ClearType Level: 100 Enhanced Contrast: 300
Device ID: 0x6779
Direct2D Enabled: true
DirectWrite Enabled: true (6.2.9200.17568)
Driver Date: 7-28-2015
Driver Version: 15.200.1062.1003
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 23111787
Supports Hardware H264 Decoding: Yes; Using D3D9 API
Vendor ID: 0x1002
WebGL Renderer: Google Inc. -- ANGLE (AMD Radeon HD 6450 Direct3D11 vs_5_0 ps_5_0)
windowLayerManagerRemote: true
AzureCanvasAccelerated: 0
AzureCanvasBackend: direct2d 1.1
AzureContentBackend: direct2d 1.1
AzureFallbackCanvasBackend: cairo



about:media

HTMLMediaElement debug data

https://www.youtube.com/watch?v=eyU3bRy2x44
	mediasource:https://www.youtube.com/2a63ea08-3e5c-43ef-a7c4-25ed9600c2a9
	currentTime: 594.7345
	Quality: 100% (total:17839 dropped:0 corrupted:0)
	Buffered ranges: [(172.301, 594.741)(615.481, 638.941)(660.001, 678.341)(700.701, 723.241)(745.681, 767.521)(790.781, 814.321)(835.801, 859.941)(880.001, 897.581)(920.921, 940.941)]
		SourceBuffer 0
			start=172.301 end=594.741
			start=615.481 end=638.941
			start=660.001 end=678.341
			start=700.701 end=723.241
			start=745.681 end=767.521
			start=790.781 end=814.321
			start=835.801 end=859.941
			start=880.001 end=897.581
			start=920.921 end=940.941
		SourceBuffer 1
			start=0 end=940.941
	Internal Data:
	audio decoder: opus audio decoder
	audio frames decoded: 29737
	video decoder: ffvpx video decoder
	hardware video decoding: disabled
	video frames decoded: 17838 (skipped:0)
	Dumping data for demuxer 193d59f0:
		Dumping Audio Track Buffer(audio/webm; codecs=opus): - mLastAudioTime: 594.741000
			NumSamples:29695 Size:15554983 NextGetSampleIndex:21122 NextInsertionIndex:29695
			Buffered: ranges=[(172.301000, 594.741000), (615.481000, 638.941000), (660.001000, 678.341000), (700.701000, 723.241000), (745.681000, 767.521000), (790.781000, 814.321000), (835.801000, 859.941000), (880.001000, 897.581000), (920.921000, 940.941000)]
		Dumping Video Track Buffer(video/webm; codecs=vp9) - mLastVideoTime: 595.228000
			NumSamples:28200 Size:52295010 NextGetSampleIndex:17839 NextInsertionIndex:28200
			Buffered: ranges=[(0.000000, 940.941000)]
I have the same issue on Linux, If I "scroll" then playback starts again.
In bug 1259713, audio buffer size threshold was changed from 100MB to 12MB.

It appears that YouTube doesn't check for eviction when appending audio data.

(940s buffered is rather over the top too)

Richard, we can increase the size allowed in a source buffer that contains only audio; which size would you recommend?
But having said that, YouTube player obviously doesn't handle this case properly and apparently makes invalid assumptions.
Flags: needinfo?(rleider)
Temporary workaround,

Create a integer preference media.mediasource.eviction_threshold.audio with the value 104857600
(In reply to Jean-Yves Avenard [:jya] from comment #6)
> Temporary workaround,
> 
> Create a integer preference media.mediasource.eviction_threshold.audio with
> the value 104857600

Unfortunately, Setting this preference does not fix the problem. The video stops at 32:25.
Now that would be surprising. Setting this pref to 100MB is identical to reverting bug 1259713
Flags: needinfo?(jyavenard)
I also continue to see the problem with the pref set...
(In reply to Jean-Yves Avenard [:jya] from comment #8)
> Setting this pref to 100MB is identical to reverting bug 1259713

Is it this changeset: https://hg.mozilla.org/mozilla-central/rev/9ffa65c64e14 ?

Because if so, there is one further change. Old code was saying:

-    (mEvictionThreshold > aLength) ? mEvictionThreshold - aLength : aLength;

New code replaced it with:

+    std::max(EvictionThreshold() - aLength, aLength);

which is

     (mEvictionThreshold - aLength > aLength) ? mEvictionThreshold - aLength : aLength;

Apologies if I'm looking at the wrong thing or otherwise wasting your time.
regression is caused by https://hg.mozilla.org/integration/mozilla-inbound/rev/6495286a5790#l4.62

i should have noticed in my review :(
Priority: -- → P1
Bug 1216460 introduced a regression which would cause to always evict from both ends of the current track buffer.

Review commit: https://reviewboard.mozilla.org/r/42671/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42671/
Attachment #8735258 - Flags: review?(gsquelart)
Attachment #8735259 - Flags: review?(gsquelart)
Attachment #8735260 - Flags: review?(gsquelart)
20MB appears to work, but just to be safe until we get confirmation from YouTube on what is a safe value to use.

Review commit: https://reviewboard.mozilla.org/r/42673/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42673/
Also, remove no longer used code and update comments to properly reflect the current algorithms used.

Review commit: https://reviewboard.mozilla.org/r/42675/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42675/
Assignee: alice0775 → jyavenard
Comment on attachment 8735258 [details]
MozReview Request: Bug 1259916: [MSE] P1. Fix eviction. r?gerald

https://reviewboard.mozilla.org/r/42671/#review39139
Attachment #8735258 - Flags: review?(gsquelart) → review+
Comment on attachment 8735259 [details]
MozReview Request: Bug 1259916: [MSE] P2. Bump audio source buffer eviction threshold to 30MB. r?gerald

https://reviewboard.mozilla.org/r/42673/#review39141
Attachment #8735259 - Flags: review?(gsquelart) → review+
Comment on attachment 8735260 [details]
MozReview Request: Bug 1259916: [MSE] P3. Simplify eviction calculation logic. r?gerald

https://reviewboard.mozilla.org/r/42675/#review39143
Attachment #8735260 - Flags: review?(gsquelart) → review+
Could you please verify that this is now working for you?

Builds available at https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=bcd0f253bb32
(In reply to Jean-Yves Avenard [:jya] from comment #19)
> Could you please verify that this is now working for you?
> 
> Builds available at
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> inbound&revision=bcd0f253bb32

I can confirm the the problem is fixed on the m-i build[1].
The 3hr video was properly playback.


[1]
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcd0f253bb32a6d1db5235dd03ab5996b6e0efbf
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 ID:20160327165228
(In reply to Jean-Yves Avenard [:jya] from comment #19)
> Could you please verify that this is now working for you?

It is indeed working perfectly \o/
Would be interesting to find out what the lowest value we can get away with media.mediasource.eviction_threshold.audio and still have YouTube playing fine.

20MB (so set it to 20*1024*1024) worked for me.

I had been under the assumption that chrome used 15MB for audio, but obviously was mistaken..
(In reply to Jean-Yves Avenard [:jya] from comment #19)

Both 32 & 64-bit 'inbound' builds are playing fine (unlike regular updates).
Thanks for the quick fix, jya. The buffer-size situation is so fragile.

Side note:
We expect |buffer.Length() > 0| in TrackBuffersManager::DoEvictData, but there is nothing explicitly that would guard against it in that function. The early return for the case |buffer.Length() == 0| was there to prevent out-of-bounds access in [1] because of the arithmetic conversion in [2] (i would be initialized with |(int32_t) ((size_t) -1))|).

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/mediasource/TrackBuffersManager.cpp#502
[2] https://dxr.mozilla.org/mozilla-central/source/dom/media/mediasource/TrackBuffersManager.cpp#501
Flags: needinfo?(esawin)
(In reply to Eugen Sawin [:esawin] from comment #26)
> Thanks for the quick fix, jya. The buffer-size situation is so fragile.
> 
> Side note:
> We expect |buffer.Length() > 0| in TrackBuffersManager::DoEvictData, but
> there is nothing explicitly that would guard against it in that function.
> The early return for the case |buffer.Length() == 0| was there to prevent
> out-of-bounds access in [1] because of the arithmetic conversion in [2] (i
> would be initialized with |(int32_t) ((size_t) -1))|).

No you don't, because it wouldn't go beyond https://dxr.mozilla.org/mozilla-central/source/dom/media/mediasource/TrackBuffersManager.cpp#489

As mSizeSourceBuffer would be zero.

You only entered into the case you described where you read the array because you had changed the order in which finalSize was calculated; and more particularly mSizeSourceBuffer is updated during CodedFrameRemoval. As such, it is imperative to read mSizeSourceBuffer *before* calling CodedFrameRemoval

This was the real cause of this regression.

I shouldn't have noticed during my review, sorry for that.
QA Whiteboard: [good first verify]
I have reproduced this bug with Firefox Nightly 48.0a1 (Build Id : 20160326030430) on Windows 7(64-bit) with the instructions from comment 0.

Verified as fixed with Latest Firefox Beta 48.0b7 (Build ID : 20160711002726)

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

[bugday-20160713]
See Also: → 1280023
Flags: needinfo?(rleider)
You need to log in before you can comment on or make changes to this bug.