Closed
Bug 1185892
Opened 9 years ago
Closed 9 years ago
Remove MediaDecoder::IsExpectingMoreData
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: jwwang, Assigned: jwwang)
References
Details
Attachments
(3 files)
As MediaDecoder::IsExpectingMoreData is all about asking MediaResource, we can create MediaResource::IsExpectingMoreData to replace it.
Assignee | ||
Comment 2•9 years ago
|
||
Assignee | ||
Comment 3•9 years ago
|
||
Bug 1185892. Part 1 - delegate the job of MediaSourceDecoder::IsExpectingMoreData to its MediaResource.
Attachment #8636936 -
Flags: review?(jyavenard)
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1185892. Part 2 - delegate the job of MediaDecoder::IsExpectingMoreData to its MediaResource.
Attachment #8636937 -
Flags: review?(jyavenard)
Assignee | ||
Comment 5•9 years ago
|
||
Bug 1185892. Part 3 - replace all calls to MediaDecoder::IsExpectingMoreData() with MediaResource::IsExpectingMoreData.
Attachment #8636938 -
Flags: review?(jyavenard)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jwwang
Comment 6•9 years ago
|
||
Comment on attachment 8636936 [details]
MozReview Request: Bug 1185892. Part 1 - delegate the job of MediaSourceDecoder::IsExpectingMoreData to its MediaResource.
https://reviewboard.mozilla.org/r/13763/#review12365
Attachment #8636936 -
Flags: review?(jyavenard) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8636937 [details]
MozReview Request: Bug 1185892. Part 2 - delegate the job of MediaDecoder::IsExpectingMoreData to its MediaResource.
https://reviewboard.mozilla.org/r/13765/#review12367
// MediaDecoder::mDecoderPosition is roughly the same as Tell() which
// returns a position updated by latest Read() or ReadAt().
return !IsDataCachedToEndOfResource(Tell()) && !IsSuspended();
}
I wonder how "rough" that is. Especially as Tell() is totally inaccurate, in particular with the MediaFormatReader() and the demuxer only using ReadAt() ; I'm guessing this will always return true with either the MP4Reader or the new WebMDemuxer even when we've cached the entire file.
Attachment #8636937 -
Flags: review?(jyavenard) → review+
Updated•9 years ago
|
Attachment #8636938 -
Flags: review?(jyavenard) → review+
Comment 8•9 years ago
|
||
Comment on attachment 8636938 [details]
MozReview Request: Bug 1185892. Part 3 - replace all calls to MediaDecoder::IsExpectingMoreData() with MediaResource::IsExpectingMoreData.
https://reviewboard.mozilla.org/r/13767/#review12369
Ship It!
Assignee | ||
Comment 9•9 years ago
|
||
https://reviewboard.mozilla.org/r/13765/#review12367
https://hg.mozilla.org/mozilla-central/file/8e5c888d0d89/dom/media/MediaDecoder.cpp#l989
|mDecoderPosition = aOffset + aBytes| which is exactly the stream positoin after Read() or ReadAt().
In fact, Tell() is the most updated version of mDecoderPosition since each Read() or ReadAt() will dispatch an async update to mDecoderPosition.
It is also true that MediaFormatReader() using ReadAt() will also trigger updates of mDecoderPosition to make it less accurate even without this bug. I think we can tell the MediaResource not to dispatch the update when it is a silent read to fix the problem.
Comment 10•9 years ago
|
||
SilentReadAt() is only used with older DecoderReader, the one relying on NotifyDataArrived() to update their buffered range (I believe this is limited to WebMReader, MP3Reader and the Ogg reader)
No MediaFormatDemuxer should be using it ..
But you're right, not dispatching the update with SilentReadAt() would fix it for those; though, does reading from the cache also trigger the updates ? SilentReadAt should only ever read from the cache now
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/file/8e5c888d0d89/dom/media/MediaResource.cpp#l736
Only Read() or ReadAt() will trigger the update. ReadFromCache() doesn't have the side effect.
Comment 12•9 years ago
|
||
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4f09e5cfeac1
https://hg.mozilla.org/mozilla-central/rev/5aca86d37171
https://hg.mozilla.org/mozilla-central/rev/e6d8399fa276
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•