Implement MediaResource::ReadAt to allow thread safe seek+read

RESOLVED FIXED in mozilla25

Status

()

Core
Audio/Video
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cajbir, Assigned: cajbir)

Tracking

Trunk
mozilla25
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
This is spun off from bug 860599 comment 3:

> I think we should add MediaResource::ReadAt(char* buffer, uint32_t
> bufferLen, uint32_t* aOutBytesRead, int64_t offset), which allows us to
> seek+read in a safe way using the MediaCache's lock. We're having to code
> around this limitation in the WMF, OMX backends (and I had to in my mythical
> DirectShow backend too), so we may as well fix it properly here. Then you
> don't have to worry about concurrent reads and my comment about tell above;
> the read will be synchronized by the media cache.
(Assignee)

Comment 1

5 years ago
Created attachment 776073 [details] [diff] [review]
Implements ReadAt

Implements MediaResource::ReadAt. For the derived classes that use locks when Reading and Seeking the implementation obtains the lock and does the seek followed by the read to ensure the read happens at the correct offset.
Assignee: nobody → chris.double
Status: NEW → ASSIGNED
Attachment #776073 - Flags: review?(cpearce)
(Assignee)

Updated

5 years ago
Blocks: 860599
Comment on attachment 776073 [details] [diff] [review]
Implements ReadAt

Review of attachment 776073 [details] [diff] [review]:
-----------------------------------------------------------------

Nice & easy.

::: content/media/MediaResource.h
@@ +250,5 @@
>    virtual nsresult Read(char* aBuffer, uint32_t aCount, uint32_t* aBytes) = 0;
> +  // Read up to aCount bytes from the stream. The read starts at
> +  // aOffset in the stream, seeking to that location initially if
> +  // it is not the current stream offset. The remaining arguments,
> +  // resukts and requirements are the same as per the Read method.

s/resukts/results/
Attachment #776073 - Flags: review?(cpearce) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 776822 [details] [diff] [review]
Implements ReadAt

Fixes comment and also fixes build error on Mac OS X due to using an off64_t instead of int64_t in one place. Carried forward r+. Try run with this updated patch: https://tbpl.mozilla.org/?tree=Try&rev=9bfda5f95aa0
Attachment #776073 - Attachment is obsolete: true
Attachment #776822 - Flags: review+

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/a9ff16ddd0e5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.