Closed
Bug 1269177
Opened 8 years ago
Closed 8 years ago
MediaFormatReader could potentially not exit waiting for data mode
Categories
(Core :: Audio/Video: Playback, defect, P2)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: jya, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
As I'm investigating bug 1258922, I thought I saw a potential problem with the way waiting for data is handled. I'm not 100% positive it can actually happen, but the code could be made to explicitly handle this case. If there's a pending task to notify the reader that new data has arrived, but also a demuxing request pending to indicate that it's waiting for data. The last task to arrive will set the mode, so even though we now have new data ready to be parsed, as the last demux request was rejected with WAITING_FOR_DATA, the MFR will enter waiting mode and not exit until new data is pushed. We shouldn't handle the NotifyDataArrived task if there's a pending demuxing task.
Reporter | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/49913/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49913/
Attachment #8747512 -
Flags: review?(gsquelart)
Comment on attachment 8747512 [details] MozReview Request: Bug 1269177: Wait for demuxing request to complete before treating new data notification. r?gerald https://reviewboard.mozilla.org/r/49913/#review46637 r+ with nit: ::: dom/media/MediaFormatReader.cpp:787 (Diff revision 1) > - if (decoder.mDrainComplete || decoder.mDraining) { > + if (decoder.mDrainComplete || decoder.mDraining || > + decoder.mDemuxRequest.Exists()) { > // We do not want to clear mWaitingForData or mDemuxEOS while > // a drain is in progress in order to properly complete the operation. You should update the comment, as it's not only about draining anymore.
Attachment #8747512 -
Flags: review?(gsquelart) → review+
Reporter | ||
Comment 3•8 years ago
|
||
https://reviewboard.mozilla.org/r/49913/#review46659 ::: dom/media/MediaFormatReader.cpp:787 (Diff revision 1) > - if (decoder.mDrainComplete || decoder.mDraining) { > + if (decoder.mDrainComplete || decoder.mDraining || > + decoder.mDemuxRequest.Exists()) { > // We do not want to clear mWaitingForData or mDemuxEOS while > // a drain is in progress in order to properly complete the operation. I did so in bug 1269178 as I had more exceptions to add as well.
Reporter | ||
Comment 4•8 years ago
|
||
Comment on attachment 8747512 [details] MozReview Request: Bug 1269177: Wait for demuxing request to complete before treating new data notification. r?gerald Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49913/diff/1-2/
Updated•8 years ago
|
Priority: -- → P2
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e0086d2ba0d1
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•