Closed
Bug 1228187
Opened 9 years ago
Closed 8 years ago
Add a throttling argument to MediaDecoderReader::NotifyDataArrived()
Categories
(Core :: Audio/Video: Playback, defect, P2)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: jwwang, Assigned: jwwang)
References
Details
Attachments
(2 files)
Per bug 1227396 comment 21, we need to throttle calls to MediaDecoderReader::GetBuffered() which is heavy.
Assignee | ||
Comment 1•9 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a05d0aa0813d
Assignee | ||
Comment 2•9 years ago
|
||
Bug 1228187. Part 1 - Add a throttling argument to MediaDecoderReader::NotifyDataArrived(). r=jya.
Attachment #8692679 -
Flags: review?(jyavenard)
Assignee | ||
Comment 3•9 years ago
|
||
Bug 1228187. Part 2 - assert functions that shouldn't be called after Shutdown(). r=jya.
Attachment #8692680 -
Flags: review?(jyavenard)
Comment 4•9 years ago
|
||
Comment on attachment 8692679 [details] MozReview Request: Bug 1228187. Part 1 - Add a throttling argument to MediaDecoderReader::NotifyDataArrived(). r=jya. https://reviewboard.mozilla.org/r/26305/#review23753 ::: dom/media/MediaDecoderReader.cpp:207 (Diff revision 1) > +MediaDecoderReader::NotifyDataArrived(bool aThrottleUpdates) I would much prefer this to be handled in MediaDecoder like it used to be rather than in the reader.
Attachment #8692679 -
Flags: review?(jyavenard)
Assignee | ||
Comment 5•9 years ago
|
||
Not sure what you mean by handling it in MediaDecoder. We used to have a function in MediaDecoderReader: void DispatchNotifyDataArrived(bool aThrottleUpdates) { RefPtr<nsRunnable> r = NS_NewRunnableMethod( this, aThrottleUpdates ? &MediaDecoderReader::ThrottledNotifyDataArrived : &MediaDecoderReader::NotifyDataArrived); OwnerThread()->Dispatch( r.forget(), AbstractThread::DontAssertDispatchSuccess); }
Updated•8 years ago
|
Attachment #8692679 -
Flags: review+
Comment 6•8 years ago
|
||
Comment on attachment 8692679 [details] MozReview Request: Bug 1228187. Part 1 - Add a throttling argument to MediaDecoderReader::NotifyDataArrived(). r=jya. https://reviewboard.mozilla.org/r/26305/#review23867
Comment 7•8 years ago
|
||
Comment on attachment 8692680 [details] MozReview Request: Bug 1228187. Part 2 - assert functions that shouldn't be called after Shutdown(). r=jya. https://reviewboard.mozilla.org/r/26307/#review23869
Attachment #8692680 -
Flags: review?(jyavenard) → review+
Updated•8 years ago
|
Attachment #8692679 -
Flags: review+
Comment 8•8 years ago
|
||
Comment on attachment 8692679 [details] MozReview Request: Bug 1228187. Part 1 - Add a throttling argument to MediaDecoderReader::NotifyDataArrived(). r=jya. https://reviewboard.mozilla.org/r/26305/#review23871 note that this bug is no longer pressing as the computation times required to calculate the buffered range has been dropped to almost zero with bug 1227396.
Comment 9•8 years ago
|
||
Comment on attachment 8692679 [details] MozReview Request: Bug 1228187. Part 1 - Add a throttling argument to MediaDecoderReader::NotifyDataArrived(). r=jya. https://reviewboard.mozilla.org/r/26305/#review23873
Attachment #8692679 -
Flags: review+
Assignee | ||
Comment 10•8 years ago
|
||
Do we still need this bug after bug 1227396 is landed? If performance is good enough, I would rather not to land this bug which makes the code a bit messy.
Comment 11•8 years ago
|
||
I guess it can be a wait & see.. however, calculating the buffered range every 32kB is still silly IMHO.
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Comment 12•8 years ago
|
||
Looks like we don't need this bug anymore.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•