When evicting data, the next appendBuffer could render samples undecodable
RESOLVED
FIXED
in Firefox 41
Status
()
People
(Reporter: jya, Assigned: jya)
Tracking
Firefox Tracking Flags
(firefox41 fixed)
Details
Attachments
(2 attachments, 2 obsolete attachments)
3.25 KB,
patch
|
gerald
:
review+
|
Details | Diff | Splinter Review |
7.87 KB,
patch
|
gerald
:
review+
|
Details | Diff | Splinter Review |
This is an actual bug in the W3C MSE spec, or more accurately, a problem that is not handled at all. When appending a media segment, the coded frame processing algorithm (http://w3c.github.io/media-source/index.html#sourcebuffer-coded-frame-processing) describes the step to detect a discontinuity, or how to append data to the current "coded frame group" http://w3c.github.io/media-source/index.html#coded-frame-group. When appending a partial media segment, it is expected that the next appendBuffer will complete the media segment, and continue the current coded frame group. When data is removed in the coded frame algorithm, it is possible that frames from the current coded frame group will be removed. If this happens, and the next appendBuffer data doesn't start with a keyframe, we will start inserting frames that can't be decoded. I will open a W3C bug and in the mean time, implement what I believe should be done under this scenario.
(Assignee) | ||
Comment 1•3 years ago
|
||
Created attachment 8622278 [details] [diff] [review] P1. Update insertion index on the fly. Prevent reparsing the entire stream in the next call to appendBuffer.
Attachment #8622278 -
Flags: review?(cpearce)
(Assignee) | ||
Comment 2•3 years ago
|
||
Created attachment 8622279 [details] [diff] [review] P2. Properly handle removal of data within the current coded frame group. W3C bug pending.
Attachment #8622279 -
Flags: review?(cpearce)
(Assignee) | ||
Updated•3 years ago
|
Assignee: nobody → jyavenard
(Assignee) | ||
Comment 3•3 years ago
|
||
Created attachment 8622864 [details] [diff] [review] P1. Update insertion index on the fly. Prevent reparsing the entire stream in the next call to appendBuffer.
Attachment #8622864 -
Flags: review?(cpearce)
(Assignee) | ||
Comment 4•3 years ago
|
||
Created attachment 8622865 [details] [diff] [review] P2. Properly handle removal of data within the current coded frame group. W3C bug pending.
Attachment #8622865 -
Flags: review?(cpearce)
(Assignee) | ||
Updated•3 years ago
|
Attachment #8622278 -
Attachment is obsolete: true
Attachment #8622278 -
Flags: review?(cpearce)
(Assignee) | ||
Updated•3 years ago
|
Attachment #8622279 -
Attachment is obsolete: true
Attachment #8622279 -
Flags: review?(cpearce)
Comment 5•3 years ago
|
||
Comment on attachment 8622864 [details] [diff] [review] P1. Update insertion index on the fly. Review of attachment 8622864 [details] [diff] [review]: ----------------------------------------------------------------- Passing review to Alfredo. Thanks for taking on this responsibility Alfredo!
Attachment #8622864 -
Flags: review?(cpearce) → review?(ayang)
Comment 6•3 years ago
|
||
Comment on attachment 8622865 [details] [diff] [review] P2. Properly handle removal of data within the current coded frame group. Review of attachment 8622865 [details] [diff] [review]: ----------------------------------------------------------------- Passing review to Alfredo. Thanks for taking on this responsibility Alfredo!
Attachment #8622865 -
Flags: review?(cpearce) → review?(ayang)
Updated•3 years ago
|
Attachment #8622864 -
Flags: review?(ayang) → review?(gsquelart)
Comment 7•3 years ago
|
||
Comment on attachment 8622865 [details] [diff] [review] P2. Properly handle removal of data within the current coded frame group. Thank you for help, Gerald.
Attachment #8622865 -
Flags: review?(ayang) → review?(gsquelart)
Updated•3 years ago
|
Attachment #8622864 -
Flags: review?(gsquelart) → review+
Comment 8•3 years ago
|
||
Comment on attachment 8622865 [details] [diff] [review] P2. Properly handle removal of data within the current coded frame group. Review of attachment 8622865 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/mediasource/TrackBuffersManager.h @@ +229,5 @@ > + void ResetAppendState() > + { > + mLastDecodeTimestamp.reset(); > + mLastFrameDuration.reset(); > + mHighestEndTimestamp.reset(); That's a lot of code in a header, and for a private method! Please consider moving it to the cpp.
Attachment #8622865 -
Flags: review?(gsquelart) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2e4889a65a1 https://hg.mozilla.org/integration/mozilla-inbound/rev/b3f78edbfbe3
Comment 10•3 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c2e4889a65a1 https://hg.mozilla.org/mozilla-central/rev/b3f78edbfbe3
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•