Media playback stalls when out of data without dispatching "waiting" (or transitioning to readyState < HAVE_FUTURE_DATA)
Categories
(Core :: Audio/Video: Playback, defect)
Tracking
()
People
(Reporter: karlt, Assigned: karlt)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(8 files, 2 obsolete files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
The test for bug 1915045 times out intermittently on Linux at least for the reason described below.
The test removes data from the audio SourceBuffer after "canplay".
Logging indicates that the MediaFormatReader
- Requests demux of more audio,
- Sets
mAudio.mReceivedNewData
, inNotifyTrackDemuxers()
, presumably in response to the SourceBuffer operation, - Triggers a drain, is response to demux failure with NS_ERROR_DOM_MEDIA_WAITING_FOR_DATA,
- Completes the drain of the audio decoder,
- Starts an
InternalSeek()
, - Skips rejection of the
RequestAudioData()
promise becausemReceivedNewData
is set, - Clears
decoder.mDrainState
, - Fails the seek from 5,
- Clears
mReceivedNewData
, - Starts another
InternalSeek()
becausemReceivedNewData
was set, - Sets
decoder.mWaitingForDataStartTime
again in response to the seek failing with NS_ERROR_DOM_MEDIA_WAITING_FOR_DATA, - Skips rejection of the
RequestAudioData()
promise again becausedecoder.mDrainState
has been reset.
Assignee | ||
Comment 1•1 month ago
•
|
||
This could be triggered when running out of data, and presumably appending data for the current playback position would trigger recovery through a successful seek, but the bug means that content is missing the notification that playback is waiting on more data.
Removal of the audio is not necessary to trigger 2. Other SourceBuffer operations would also do this.
Assignee | ||
Comment 2•1 month ago
|
||
I guess Update()
would have tried to demux more samples again before needInput
was changed to false when HasInternalSeekPending()
.
That changeset also introduced the logic to go directly to the second internal seek without another demux attempt.
Comment 3•1 month ago
|
||
Set release status flags based on info from the regressing bug 1269408
Assignee | ||
Comment 4•1 month ago
|
||
Assignee | ||
Comment 5•1 month ago
|
||
Assignee | ||
Comment 6•1 month ago
|
||
UpdateReceivedNewData() would have aborted Update() early on
mSeekRequest.Exists() if there were an external seek, so at this point any
seek request would be internal.
This doesn't change behavior because needInput
is false when
HasInternalSeekPending()
, but merely clarifies that nothing more will
happen.
Assignee | ||
Comment 7•1 month ago
|
||
No behavior change, except in logging when !decoder.HasPromise()
.
The mWaiting check is no longer necessary since
https://hg.mozilla.org/mozilla-central/rev/df20f438502df1a33c71aea95d3ded5b84a2d2a7#l1.34
needInput
is false if an internal seek is pending and so Update() will
return before doing anything further.
If decoder.HasPromise()
then this waiting for data test is not even reached
because a previous early return is taken on IsInternalSeekPending().
The behavior of continuing to demux and decode to skip frames when
mTimeThreshold->mHasSeeked
is maintained.
Assignee | ||
Comment 8•1 month ago
|
||
This came from
https://hg.mozilla.org/mozilla-central/rev/271a6b2de3ad4be77b1c91305f5c5a49d4924120#l1.14
IsWaitingForData() always returns false because
mWaitingForDataStartTime.reset() is called above.
Assignee | ||
Comment 9•1 month ago
|
||
Comment 10•1 month ago
|
||
Comment 11•1 month ago
|
||
Comment 12•1 month ago
|
||
Comment 13•1 month ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6820e30c2a77
https://hg.mozilla.org/mozilla-central/rev/f594e6f00208
https://hg.mozilla.org/mozilla-central/rev/1216b45b86ea
Comment 14•1 month ago
|
||
Comment 15•1 month ago
|
||
bugherder |
Comment 16•1 month ago
|
||
Backed out changeset f594e6f00208 (Bug 1940883) for causing crashes in Bug 1941164
Backout: https://hg.mozilla.org/integration/autoland/rev/abc92a41910764a2dc98aea04074bc746fa2c194
Comment 17•1 month ago
|
||
Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/abc92a41910764a2dc98aea04074bc746fa2c194
Updated•1 month ago
|
Updated•1 month ago
|
Updated•1 month ago
|
Assignee | ||
Comment 18•1 month ago
•
|
||
Still intending to fix this, once Bug 1941164 is understood.
Comment 19•1 month ago
|
||
Backout merged to mozilla-central: https://hg.mozilla.org/mozilla-central/rev/45f68751c2a6677f127f1d1125aea72adce108af
Updated•1 month ago
|
Assignee | ||
Comment 20•1 month ago
•
|
||
The crash state could be reached via this sequence of events:
- An internal seek successfully seeks to the preceding random access point.
- Demux of samples is requested while decoder output is less than the seek target time.
- The demux fails with NS_ERROR_DOM_MEDIA_WAITING_FOR_DATA, requests a drain and calls
NotifyWaitingForData()
, which setsdecoder.mTimeThreshold->mWaiting
. HasInternalSeekWaiting()
now returns true even thoughHasInternalSeekPending()
is false.- mDrainState is cleared even though not complete.
- Another demux is requested and fails, which triggers another drain.
Comment 21•1 month ago
|
||
Backed out changeset 1216b45b86ea (Bug 1940883) due to risk of unproductive cpu usage : https://hg.mozilla.org/integration/autoland/rev/5ee87c6b2828c6f7d32fd939fe19f8d0e02fd68d
Assignee | ||
Comment 22•1 month ago
|
||
What I missed in writing https://hg.mozilla.org/mozilla-central/rev/1216b45b86ea also was that mWaiting
can be set even when the MediaTrackDemuxer::Seek()
has completed the seek to a random access point, but a demux has failed while Update()
is trying to advance through dependent frames before the target. HasInternalSeekPending()
is false, but Update()
should not (repeatedly) schedule another demux until mReceivedNewData
is set.
Updated•1 month ago
|
Updated•1 month ago
|
Assignee | ||
Comment 23•1 month ago
|
||
Assignee | ||
Comment 24•1 month ago
|
||
Assignee | ||
Comment 25•1 month ago
|
||
Assignee | ||
Comment 26•1 month ago
|
||
Updated•1 month ago
|
Comment 27•1 month ago
|
||
Backout merged to mozilla-central: https://hg.mozilla.org/mozilla-central/rev/5ee87c6b2828c6f7d32fd939fe19f8d0e02fd68d
Assignee | ||
Updated•1 month ago
|
Assignee | ||
Comment 28•1 month ago
|
||
Comment 29•1 month ago
|
||
Comment 30•1 month ago
|
||
Comment 31•1 month ago
|
||
Comment 32•1 month ago
|
||
Comment 33•1 month ago
|
||
bugherder |
Assignee | ||
Updated•1 month ago
|
Comment 34•1 month ago
|
||
Comment 35•1 month ago
|
||
Comment 36•29 days ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/742de48395f3
https://hg.mozilla.org/mozilla-central/rev/a47c8e651937
Updated•29 days ago
|
Description
•