Skip to content

Commit 514a713

Browse files
committed
[MSE] Infinite loop in sample eviction when duration is NaN
https://bugs.webkit.org/show_bug.cgi?id=218228 Reviewed by Darin Adler. Source/WebCore: Avoid infinite loop in evictCodedFrames for live streams When playing live streams the MediaSource DOM duration attribute has no meaning and would thus be set as +inf. When seeks are triggered to positions prior to the current playback position the SourceBuffer might attempt to free some space in order to keep the amount of memory used under control. It proceeds in 2 steps: 1. Attempt to free space represented by buffered range from media start up until current playback position - 30 seconds. 2. If step 1 didn't free enough memory, attempt to release memory represented by buffered ranges starting from current playback position + 30 seconds until media duration. Step 2 here wasn't taking into account the case where MediaSource.duration is actually invalid, and thus was entering an infinite loop. Test: media/media-source/live-rewind-seek-and-evict.html * Modules/mediasource/SourceBuffer.cpp: (WebCore::SourceBuffer::evictCodedFrames): Source/WTF: * wtf/MediaTime.h: New isFinite() method, to check if the MediaTime is valid and represents a finite value, eg not NaN or Infinite. LayoutTests: * media/media-source/live-rewind-seek-and-evict-expected.txt: Renamed from LayoutTests/platform/glib/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt. * media/media-source/live-rewind-seek-and-evict.html: Added. * media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt: * media/media-source/mock-media-source.js: (makeAInit): * platform/mac/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt: Copied from LayoutTests/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt. Canonical link: https://commits.webkit.org/231817@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270106 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 2aae0f7 commit 514a713

10 files changed

Lines changed: 301 additions & 10 deletions

LayoutTests/ChangeLog

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2020-11-20 Philippe Normand <pnormand@igalia.com>
2+
3+
[MSE] Infinite loop in sample eviction when duration is NaN
4+
https://bugs.webkit.org/show_bug.cgi?id=218228
5+
6+
Reviewed by Darin Adler.
7+
8+
* media/media-source/live-rewind-seek-and-evict-expected.txt: Renamed from LayoutTests/platform/glib/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt.
9+
* media/media-source/live-rewind-seek-and-evict.html: Added.
10+
* media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt:
11+
* media/media-source/mock-media-source.js:
12+
(makeAInit):
13+
* platform/mac/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt: Copied from LayoutTests/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt.
14+
115
2020-11-20 Fujii Hironori <Hironori.Fujii@sony.com>
216

317
[TextureMapper] Hidden surface removal issue for nested transform-style:perserve-3d in Snow Stack demo
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
This tests rewind seeks in a live stream. The sample eviction algorithm should attempt to remove some samples without going into an infinite loop.
2+
EVENT(sourceopen)
3+
EVENT(updateend)
4+
EXPECTED (isNaN(source.duration) == 'true') OK
5+
Appending PTS=120
6+
EVENT(updateend)
7+
Appending PTS=121
8+
EVENT(updateend)
9+
Appending PTS=122
10+
EVENT(updateend)
11+
Appending PTS=123
12+
EVENT(updateend)
13+
Appending PTS=124
14+
EVENT(updateend)
15+
Appending PTS=125
16+
EVENT(updateend)
17+
Appending PTS=126
18+
EVENT(updateend)
19+
Appending PTS=127
20+
EVENT(updateend)
21+
Appending PTS=128
22+
EVENT(updateend)
23+
Appending PTS=129
24+
EVENT(updateend)
25+
Appending PTS=130
26+
EVENT(updateend)
27+
Appending PTS=131
28+
EVENT(updateend)
29+
Appending PTS=132
30+
EVENT(updateend)
31+
Appending PTS=133
32+
EVENT(updateend)
33+
Appending PTS=134
34+
EVENT(updateend)
35+
Appending PTS=135
36+
EVENT(updateend)
37+
Appending PTS=136
38+
EVENT(updateend)
39+
Appending PTS=137
40+
EVENT(updateend)
41+
Appending PTS=138
42+
EVENT(updateend)
43+
Appending PTS=139
44+
EVENT(updateend)
45+
Appending PTS=140
46+
EVENT(updateend)
47+
Appending PTS=141
48+
EVENT(updateend)
49+
Appending PTS=142
50+
EVENT(updateend)
51+
Appending PTS=143
52+
EVENT(updateend)
53+
Appending PTS=144
54+
EVENT(updateend)
55+
Appending PTS=145
56+
EVENT(updateend)
57+
Appending PTS=146
58+
EVENT(updateend)
59+
Appending PTS=147
60+
EVENT(updateend)
61+
Appending PTS=148
62+
EVENT(updateend)
63+
Appending PTS=149
64+
EVENT(updateend)
65+
Appending PTS=150
66+
EVENT(updateend)
67+
Appending PTS=151
68+
EVENT(updateend)
69+
Appending PTS=152
70+
EVENT(updateend)
71+
Appending PTS=153
72+
EVENT(updateend)
73+
Appending PTS=154
74+
EVENT(updateend)
75+
Appending PTS=155
76+
EVENT(updateend)
77+
Appending PTS=156
78+
EVENT(updateend)
79+
Appending PTS=157
80+
EVENT(updateend)
81+
Appending PTS=158
82+
EVENT(updateend)
83+
Appending PTS=159
84+
EVENT(updateend)
85+
Appending PTS=160
86+
EVENT(updateend)
87+
Appending PTS=161
88+
EVENT(updateend)
89+
Appending PTS=162
90+
EVENT(updateend)
91+
Appending PTS=163
92+
EVENT(updateend)
93+
Appending PTS=164
94+
EVENT(updateend)
95+
Appending PTS=165
96+
EVENT(updateend)
97+
Appending PTS=166
98+
EVENT(updateend)
99+
Appending PTS=167
100+
EVENT(updateend)
101+
Appending PTS=168
102+
EVENT(updateend)
103+
Appending PTS=169
104+
EVENT(updateend)
105+
Appending PTS=170
106+
EVENT(updateend)
107+
Appending PTS=171
108+
EVENT(updateend)
109+
Appending PTS=172
110+
EVENT(updateend)
111+
Appending PTS=173
112+
EVENT(updateend)
113+
Appending PTS=174
114+
EVENT(updateend)
115+
Appending PTS=175
116+
EVENT(updateend)
117+
Appending PTS=176
118+
EVENT(updateend)
119+
EXPECTED (video.currentTime == '175') OK
120+
EXPECTED (bufferedRanges() == '[ 120...132, 162...177 ]') OK
121+
EXPECTED (video.currentTime == '115') OK
122+
Appending PTS=109
123+
EVENT(updateend)
124+
Appending PTS=110
125+
EVENT(updateend)
126+
Appending PTS=111
127+
EVENT(updateend)
128+
Appending PTS=112
129+
EVENT(updateend)
130+
Appending PTS=113
131+
EVENT(updateend)
132+
Appending PTS=114
133+
EVENT(updateend)
134+
Appending PTS=115
135+
EVENT(updateend)
136+
Appending PTS=116
137+
EVENT(updateend)
138+
Appending PTS=117
139+
EVENT(updateend)
140+
Appending PTS=118
141+
EVENT(updateend)
142+
Appending PTS=119
143+
EVENT(updateend)
144+
EXPECTED (bufferedRanges() == '[ 109...132, 162...177 ]') OK
145+
END OF TEST
146+
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>mock-media-source</title>
5+
<script src="mock-media-source.js"></script>
6+
<script src="../video-test.js"></script>
7+
<script>
8+
var source;
9+
var sourceBuffer;
10+
var initSegment;
11+
var exception;
12+
13+
function bufferedRanges() {
14+
var bufferedRanges = '[ ';
15+
var timeRanges = sourceBuffer.buffered;
16+
for (var i = 0 ; i < timeRanges.length ; i++) {
17+
if (i)
18+
bufferedRanges += ', ';
19+
bufferedRanges += timeRanges.start(i) + '...' + timeRanges.end(i);
20+
}
21+
bufferedRanges += ' ]';
22+
return bufferedRanges;
23+
}
24+
25+
async function appendPtsRange(firstPts, lastPts) {
26+
var resultException = null;
27+
for (var pts = firstPts; pts <= lastPts; pts++) {
28+
try {
29+
consoleWrite('Appending PTS='+pts);
30+
sourceBuffer.appendBuffer(makeASample(pts, pts, 1, 1, 1, SAMPLE_FLAG.SYNC, 1));
31+
await waitFor(sourceBuffer, 'updateend');
32+
} catch (e) {
33+
resultException = e;
34+
sourceBuffer.abort();
35+
break;
36+
}
37+
}
38+
return resultException;
39+
}
40+
41+
if (window.internals)
42+
internals.initializeMockMediaSource();
43+
44+
window.addEventListener('load', async() => {
45+
findMediaElement();
46+
source = new MediaSource();
47+
48+
const videoSource = document.createElement('source');
49+
videoSource.type = 'video/mock; codecs=mock';
50+
videoSource.src = URL.createObjectURL(source);
51+
video.appendChild(videoSource);
52+
53+
await waitFor(source, 'sourceopen');
54+
sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock");
55+
initSegment = makeAInit(-1, [makeATrack(1, 'mock', TRACK_KIND.VIDEO)]);
56+
sourceBuffer.appendBuffer(initSegment);
57+
await waitFor(sourceBuffer, 'updateend');
58+
59+
testExpected('isNaN(source.duration)', true, '==');
60+
61+
// This should allow bufering up to 177 (empirically tested).
62+
internals.settings.setMaximumSourceBufferSize(3000);
63+
64+
exception = await appendPtsRange(120, 176);
65+
66+
video.currentTime = 175;
67+
testExpected('video.currentTime', 175, '==');
68+
69+
testExpected('bufferedRanges()', '[ 120...132, 162...177 ]', '==');
70+
71+
video.currentTime = 115;
72+
testExpected('video.currentTime', 115, '==');
73+
await appendPtsRange(109, 119);
74+
testExpected('bufferedRanges()', '[ 109...132, 162...177 ]', '==');
75+
endTest();
76+
});
77+
</script>
78+
</head>
79+
<body>
80+
This tests rewind seeks in a live stream. The sample eviction algorithm should attempt to remove
81+
some samples without going into an infinite loop.
82+
<video></video>
83+
</body>
84+
</html>

LayoutTests/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,8 @@ EVENT(updateend)
115115
Appending PTS=175
116116
EVENT(updateend)
117117
Appending PTS=176
118-
EVENT(updateend)
119-
EXPECTED (exception == 'QuotaExceededError: The quota has been exceeded.'), OBSERVED 'null' FAIL
120-
EXPECTED (bufferedRanges() == '[ 120...176 ]'), OBSERVED '[ 120...177 ]' FAIL
118+
EXPECTED (exception == 'QuotaExceededError: The quota has been exceeded.') OK
119+
EXPECTED (bufferedRanges() == '[ 120...176 ]') OK
121120
EXPECTED (video.currentTime == '115') OK
122121
Appending PTS=115
123122
EVENT(updateend)

LayoutTests/media/media-source/mock-media-source.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,13 @@ function makeAInit(duration, tracks) {
7575
var array = new Uint8Array(buffer);
7676
array.set(stringToArray('init'));
7777

78-
var view = new DataView(buffer);
79-
var timeScale = 1000;
80-
view.setUint32(4, byteLength, true);
81-
view.setInt32(8, duration * timeScale, true);
82-
view.setInt32(12, timeScale, true);
78+
if (!isNaN(duration)) {
79+
var view = new DataView(buffer);
80+
var timeScale = 1000;
81+
view.setUint32(4, byteLength, true);
82+
view.setInt32(8, duration * timeScale, true);
83+
view.setInt32(12, timeScale, true);
84+
}
8385

8486
var offset = 16;
8587
tracks.forEach(function(track){

LayoutTests/platform/glib/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt renamed to LayoutTests/platform/mac/media/media-source/media-source-append-before-last-range-no-quota-exceeded-expected.txt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,9 @@ EVENT(updateend)
115115
Appending PTS=175
116116
EVENT(updateend)
117117
Appending PTS=176
118-
EXPECTED (exception == 'QuotaExceededError: The quota has been exceeded.') OK
119-
EXPECTED (bufferedRanges() == '[ 120...176 ]') OK
118+
EVENT(updateend)
119+
EXPECTED (exception == 'QuotaExceededError: The quota has been exceeded.'), OBSERVED 'null' FAIL
120+
EXPECTED (bufferedRanges() == '[ 120...176 ]'), OBSERVED '[ 120...177 ]' FAIL
120121
EXPECTED (video.currentTime == '115') OK
121122
Appending PTS=115
122123
EVENT(updateend)

Source/WTF/ChangeLog

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2020-11-20 Philippe Normand <pnormand@igalia.com>
2+
3+
[MSE] Infinite loop in sample eviction when duration is NaN
4+
https://bugs.webkit.org/show_bug.cgi?id=218228
5+
6+
Reviewed by Darin Adler.
7+
8+
* wtf/MediaTime.h: New isFinite() method, to check if the MediaTime is valid and represents
9+
a finite value, eg not NaN or Infinite.
10+
111
2020-11-19 Ada Chan <adachan@apple.com>
212

313
Turn on ENABLE_WEBXR for Cocoa

Source/WTF/wtf/MediaTime.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ class WTF_EXPORT_PRIVATE MediaTime final {
9696
bool isPositiveInfinite() const { return m_timeFlags & PositiveInfinite; }
9797
bool isNegativeInfinite() const { return m_timeFlags & NegativeInfinite; }
9898
bool isIndefinite() const { return m_timeFlags & Indefinite; }
99+
bool isFinite() const { return !isInvalid() && !isIndefinite() && !isPositiveInfinite() && !isNegativeInfinite(); }
99100
bool hasDoubleValue() const { return m_timeFlags & DoubleValue; }
100101
uint8_t timeFlags() const { return m_timeFlags; }
101102

Source/WebCore/ChangeLog

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,32 @@
1+
2020-11-20 Philippe Normand <pnormand@igalia.com>
2+
3+
[MSE] Infinite loop in sample eviction when duration is NaN
4+
https://bugs.webkit.org/show_bug.cgi?id=218228
5+
6+
Reviewed by Darin Adler.
7+
8+
Avoid infinite loop in evictCodedFrames for live streams
9+
10+
When playing live streams the MediaSource DOM duration attribute has no meaning
11+
and would thus be set as +inf. When seeks are triggered to positions prior to
12+
the current playback position the SourceBuffer might attempt to free some space
13+
in order to keep the amount of memory used under control. It proceeds in 2
14+
steps:
15+
16+
1. Attempt to free space represented by buffered range from media start up until
17+
current playback position - 30 seconds.
18+
2. If step 1 didn't free enough memory, attempt to release memory represented by
19+
buffered ranges starting from current playback position + 30 seconds until media
20+
duration.
21+
22+
Step 2 here wasn't taking into account the case where MediaSource.duration is
23+
actually invalid, and thus was entering an infinite loop.
24+
25+
Test: media/media-source/live-rewind-seek-and-evict.html
26+
27+
* Modules/mediasource/SourceBuffer.cpp:
28+
(WebCore::SourceBuffer::evictCodedFrames):
29+
130
2020-11-20 Fujii Hironori <Hironori.Fujii@sony.com>
231

332
[TextureMapper] Hidden surface removal issue for nested transform-style:perserve-3d in Snow Stack demo

Source/WebCore/Modules/mediasource/SourceBuffer.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,11 @@ void SourceBuffer::evictCodedFrames(size_t newDataSize)
996996
MediaTime minimumRangeStart = currentTime + thirtySeconds;
997997

998998
rangeEnd = m_source->duration();
999+
if (!rangeEnd.isFinite()) {
1000+
rangeEnd = buffered.maximumBufferedTime();
1001+
DEBUG_LOG(LOGIDENTIFIER, "MediaSource duration is not a finite value, using maximum buffered time: ", rangeEnd);
1002+
}
1003+
9991004
rangeStart = rangeEnd - thirtySeconds;
10001005
while (rangeStart > minimumRangeStart) {
10011006

0 commit comments

Comments
 (0)