Closed Bug 1286175 Opened 4 years ago Closed 4 years ago

Allow SourceBufferIterator users to specify how many bytes to advance

Categories

(Core :: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file, 1 obsolete file)

This is the last piece of the puzzle before we can implement yielding in bug 1284031: we need the ability to tell a SourceBufferIterator "only advance by this many bytes", and we need it to work efficiently.

In this case, we'll keep it efficient by having the SourceBufferIterator continue to grab as much data as it can from its owning SourceBuffer. It will then allow users to advance a few bytes at a time, if they so desire, through the data it has already grabbed. Only when it reaches the end of the data it has available does it need to talk to the owning SourceBuffer again to get more data. Since interacting with the SourceBuffer involves taking a lock, this greatly reduces the overhead required for advancing in many very small steps, which is what we want to support.

We want tests for this, obviously. They're included in the patch in bug 1286161, which adds a GTest suite for SourceBuffer (there wasn't one up to now).
Here's the patch. Basically the SourceBufferIterator now keeps track of the
amount of data it actually got from the SourceBuffer most recently
(mAvailableLength) as well as the amount that the caller wanted to read on the
last call to Advance() (mNextReadLength). Calls to Advance() don't have to
interact with the owning SourceBuffer and take the lock as long as
|mAvailableLength| isn't exhausted.

SourceBufferIterator's code could be a little cleaner; it's a bit weird how
SourceBufferIterator calls into SourceBuffer which calls back into
SourceBufferIterator to call methods like SetReady(). But this bug probably
isn't the place to fix that, so I've stuck with that approach here.
Attachment #8770045 - Flags: review?(edwin)
Thanks for the review!

This updated patch contains a small fix that makes sure that we don't read a new
chunk from the SourceBuffer if a zero-byte read was requested, even if we're at
the end of the previous chunk. I've updated the SourceBuffer GTest suite to
include a test for this.
Attachment #8770045 - Attachment is obsolete: true
Pushed by mfowler@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/515b58073c46
Allow SourceBufferIterator users to specify how many bytes to advance. r=edwin
https://hg.mozilla.org/mozilla-central/rev/515b58073c46
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.