Devritualize MediaResource::MediaReadAt()

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
The difference is the sub-class override doesn't call DispatchBytesConsumed(). It doesn't make sense not to call that after you do consume some data. For callers that want silent reads, they should call ReadFromCache() instead.
(Assignee)

Updated

6 months ago
Assignee: nobody → jwwang
Blocks: 1373160
Priority: -- → P3
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

6 months ago
Attachment #8878390 - Flags: review?(jyavenard)
Attachment #8878391 - Flags: review?(jyavenard)
Attachment #8878392 - Flags: review?(jyavenard)

Comment 4

6 months ago
mozreview-review
Comment on attachment 8878390 [details]
Bug 1373577. P1 - devritualize MediaResource::MediaReadAt().

https://reviewboard.mozilla.org/r/149730/#review154398
Attachment #8878390 - Flags: review?(jyavenard) → review+

Comment 5

6 months ago
mozreview-review
Comment on attachment 8878391 [details]
Bug 1373577. P2 - move the code of MediaResource::MediaReadAt() into MediaResourceIndex.

https://reviewboard.mozilla.org/r/149732/#review154400

::: commit-message-127e4:4
(Diff revision 1)
> +Bug 1373577. P2 - move the code of MediaResource::MediaReadAt() into MediaResourceIndex.
> +
> +MediaReadAt() accesses only public members of MediaResource. It doesn't has
> +to be a member of MediaResource.

it doesn't have to be
Attachment #8878391 - Flags: review?(jyavenard) → review+

Comment 6

6 months ago
mozreview-review
Comment on attachment 8878392 [details]
Bug 1373577. P3 - fix the comment since MediaResource::ReatAt() does update the offset of MediaResource.

https://reviewboard.mozilla.org/r/149734/#review154402
Attachment #8878392 - Flags: review?(jyavenard) → review+
(Assignee)

Comment 7

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

Comment 10

6 months ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0eacbd673e5a
P1 - devritualize MediaResource::MediaReadAt(). r=jya
https://hg.mozilla.org/integration/autoland/rev/5f983c055532
P2 - move the code of MediaResource::MediaReadAt() into MediaResourceIndex. r=jya
https://hg.mozilla.org/integration/autoland/rev/c8d764a48d58
P3 - fix the comment since MediaResource::ReatAt() does update the offset of MediaResource. r=jya

Comment 11

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0eacbd673e5a
https://hg.mozilla.org/mozilla-central/rev/5f983c055532
https://hg.mozilla.org/mozilla-central/rev/c8d764a48d58
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.