1374173 - Refactor MediaCacheStream::Read and Seek

Status: RESOLVED FIXED

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

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

MediaCacheStream::Read and Seek are only used internally by ReadAt, so they could be made private&internal.

Also, Seek is only used internally to perform a SEEK_SET, so we can remove everything else.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1374173 - Make MediaCacheStream::Seek and Read internal -

Review commit: https://reviewboard.mozilla.org/r/150284/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150284/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1374173 - SeekInternal is only used to SEEK_SET -

So its code can be simplified by removing the code doing other operations.

Review commit: https://reviewboard.mozilla.org/r/150286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150286/

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8879000 [details]
Bug 1374173 - Make MediaCacheStream::Seek and Read internal -

https://reviewboard.mozilla.org/r/150284/#review154932

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8879001 [details]
Bug 1374173 - SeekInternal is only used to SEEK_SET -

https://reviewboard.mozilla.org/r/150286/#review154934

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/876aa3618a72
Make MediaCacheStream::Seek and Read internal - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/1bfb595e6629
SeekInternal is only used to SEEK_SET - r=jwwang

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/876aa3618a72
https://hg.mozilla.org/mozilla-central/rev/1bfb595e6629

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Backed out in bug 1374441 ( https://hg.mozilla.org/mozilla-central/rev/a1ef8b483090 ) for intermittent MediaCache assertion failures.

See bug 1374441 comment 25 for some details, pointing the finger at the first part (Make MediaCacheStream::Seek and Read internal), which makes little sense as this should be a simple refactoring that doesn't change the behavior! To be investigated...

But it was a nice-to-have bug, so there is no hurry to get it done.

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Bug 1415766 does the same thing as part 2 here, so if someone comes back to this bug, only part 1 will need to be re-done.

Comment 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Bug 1428242 is taking care of this bug's part 1, so we can close this bug. Thank you JW.