Merge MediaResourceCallback::NotifyDataArrived and NotifyBytesDownloaded

RESOLVED FIXED in Firefox 56

Status

()

Core
Audio/Video: Playback
P3
normal
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

6 months ago
NotifyDataArrived() is called before data written to the cache while NotifyBytesDownloaded() is called after writes.

It is confusing to have 2 of them which serve similar purposes. We should keep only NotifyDataArrived() (which might result in buffer ranges re-calculation) and call it after data written to the cache.

[1] http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/dom/media/MediaResource.cpp#467
[2] http://searchfox.org/mozilla-central/rev/20d16dadd336e0c6b97e3e19dc4ff907744b5734/dom/media/MediaResource.cpp#894
(Assignee)

Updated

6 months ago
Assignee: nobody → jwwang
Blocks: 1373160
Priority: -- → P3
(Assignee)

Updated

6 months ago
Summary: Merge MediaDecoder::NotifyDataArrived and NotifyBytesDownloaded → Merge MediaResourceCallback::NotifyDataArrived and NotifyBytesDownloaded
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

6 months ago
Attachment #8878979 - Flags: review?(gsquelart)
Attachment #8878980 - Flags: review?(gsquelart)
Attachment #8878981 - Flags: review?(gsquelart)

Comment 4

6 months ago
mozreview-review
Comment on attachment 8878979 [details]
Bug 1373595. P1 - merge MediaResourceCallback::NotifyDataArrived and NotifyBytesDownloaded.

https://reviewboard.mozilla.org/r/150244/#review154870
Attachment #8878979 - Flags: review?(gsquelart) → review+

Comment 5

6 months ago
mozreview-review
Comment on attachment 8878980 [details]
Bug 1373595. P2 - rename NotifyBytesDownloaded to NotifyDownloadProgressed to better describe what it actually does.

https://reviewboard.mozilla.org/r/150246/#review154872
Attachment #8878980 - Flags: review?(gsquelart) → review+

Comment 6

6 months ago
mozreview-review
Comment on attachment 8878981 [details]
Bug 1373595. P3 - devirtualize NotifyDownloadProgressed which has no overrides at all.

https://reviewboard.mozilla.org/r/150248/#review154876

::: commit-message-a0146:1
(Diff revision 1)
> +Bug 1373595. P3 -devirtualize NotifyDownloadProgressed which has no overrids at all.

Add space before "devirtualize".
"overrids" -> "overrides"
Attachment #8878981 - Flags: review?(gsquelart) → review+
(Assignee)

Comment 7

6 months ago
Thanks!
Comment hidden (mozreview-request)

Comment 9

6 months ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0a2aef1e3d4d
P1 - merge MediaResourceCallback::NotifyDataArrived and NotifyBytesDownloaded. r=gerald
https://hg.mozilla.org/integration/autoland/rev/ea7d7376cc08
P2 - rename NotifyBytesDownloaded to NotifyDownloadProgressed to better describe what it actually does. r=gerald
https://hg.mozilla.org/integration/autoland/rev/302e0f95efed
P3 - devirtualize NotifyDownloadProgressed which has no overrides at all. r=gerald

Comment 10

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0a2aef1e3d4d
https://hg.mozilla.org/mozilla-central/rev/ea7d7376cc08
https://hg.mozilla.org/mozilla-central/rev/302e0f95efed
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.