Closed Bug 1373577 Opened 2 years ago Closed 2 years ago

Devritualize MediaResource::MediaReadAt()

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(3 files)

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: nobody → jwwang
Blocks: 1373160
Priority: -- → P3
Attachment #8878390 - Flags: review?(jyavenard)
Attachment #8878391 - Flags: review?(jyavenard)
Attachment #8878392 - Flags: review?(jyavenard)
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 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 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+
Thanks!
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
You need to log in before you can comment on or make changes to this bug.