Closed Bug 1204401 Opened 4 years ago Closed 4 years ago
Change the buffering criteria in MDSM::Update
Rendered Video Frames
40 bytes, text/x-review-board-request
https://hg.mozilla.org/mozilla-central/file/68718290640b/dom/media/MediaDecoderStateMachine.cpp#l2576 If next video frame is 10 mins later, remainingTime will be as long as 10 mins. It doesn't make sense to ask for so much data in order not to enter buffering state. (See https://hg.mozilla.org/mozilla-central/file/68718290640b/dom/media/MediaDecoderStateMachine.cpp#l2605) We should just pass a configurable constant (that is EXHAUSTED_DATA_MARGIN_USECS) to HasLowDecodedData().
Since |remainingTime + EXHAUSTED_DATA_MARGIN_USECS| is exactly 100000 for a audio-only file, I will just change EXHAUSTED_DATA_MARGIN_USECS to 100000.
Bug 1204401 - Change the buffering criteria in MDSM::UpdateRenderedVideoFrames.
Comment on attachment 8660537 [details] MozReview Request: Bug 1204401 - Change the buffering criteria in MDSM::UpdateRenderedVideoFrames. https://reviewboard.mozilla.org/r/19133/#review17129 ::: dom/media/MediaDecoderStateMachine.cpp:2623 (Diff revision 1) > - shouldBuffer = HasLowDecodedData(remainingTime + EXHAUSTED_DATA_MARGIN_USECS) && > + shouldBuffer = HasLowDecodedData(EXHAUSTED_DATA_MARGIN_USECS) && I don't think that's right. If we want to avoid the issue of remainingTime being too long due to an invalid frame ; or too many frames dropped etc. Then doing std::min(40000, remainingTime) should be used instead. Now if remainingTime is < AUDIO_DURATION_USECS we will attempt to buffer more than we used it.
https://hg.mozilla.org/mozilla-central/file/68718290640b/dom/media/MediaDecoderStateMachine.cpp#l2605 The value passed to HasLowDecodedData() is used to determine whether we have enough decoded audio data. We will enter buffering state if HasLowDecodedData() returns false. For example, HasLowDecodedData(40000) returns false if we have only 30000us audio data. HasLowDecodedData(20) returns true if we have more than 20us audio data. However, HasLowDecodedData(100000000) will return false even if we have 5s audio. The smaller the value is, the less decoded audio data we need in order not to enter buffering state. It doesn't make sense to me that we take into account how far away the next video frame is. The value passed to HasLowDecodedData is about audio only. We should always enter buffering state if decoded audio is below some threshold (which I choose to be EXHAUSTED_DATA_MARGIN_USECS == 100ms).
Comment on attachment 8660537 [details] MozReview Request: Bug 1204401 - Change the buffering criteria in MDSM::UpdateRenderedVideoFrames. https://reviewboard.mozilla.org/r/19133/#review17215 Following discussion on IRC ; this is to make the threshold as a constant 100ms
Attachment #8660537 - Flags: review+
Thanks for the review!
You need to log in before you can comment on or make changes to this bug.