Chromium Code Reviews Chromium Code Reviews
Issue
1866243003:
Revert of Eliminate copies of encoded image data (Closed)
DescriptionRevert of Eliminate copies of encoded image data (patchset #23 id:440001 of https://codereview.chromium.org/1812273003/ )
Reason for revert:
Speculative fix for crbug.com/601416
Original issue's description:
> Eliminate copies of encoded image data
>
> Remove the ThreadSafeDataTransport, which required making two copies of
> the encoded data. Instead, copy the data (once) into an SkRWBuffer,
> which allows creating read only snapshots that can be safely read in
> another thread.
>
> Design doc can be found at https://docs.google.com/document/d/1aNmMaWfjkSSkdnVkAJoBUN7mNwdCXT1wLghrhDhhSUw/edit#heading=h.hvbfr8rfyj9k
> Benchmark results can be found at https://docs.google.com/spreadsheets/d/1Y_9UbzBvyg1HRs2uieeV2Twb7seJFKZulRVg7OQOqEo/
>
> DeferredImageDecoder:
> Copy into an SkRWBuffer, and create snapshots to pass to each
> DecodingImageGenerator.
>
> DecodingImageGenerator:
> Store a snapshot of the data, along with whether the snapshot contains
> all of the data. Pass this to the ImageFrameGenerator when it is time
> to decode. Since it already knows whether it has received all the data,
> never attempt to YUV decode if not.
> When created from an SkData, skip the copy into a SharedBuffer. Instead,
> wrap in a SegmentReader.
>
> ImageFrameGenerator:
> Its decoding APIs now take the encoded data and whether it's complete as
> parameters. Does not own the encoded data, and no longer has the
> implementation of onRefEncodedData.
> Make the object thread-safe where possible, so we can only lock the
> mutex during operations that truly need it.
>
> SegmentReader:
> Add a new interface that looks like a read only SharedBuffer. Add three
> implementations:
> - SharedBufferSegmentReader:
> Used when we already have a SharedBuffer (e.g. in the main thread).
> - ROBufferSegmentReader:
> Facilitates decoding in a worker thread without requiring mutex locks/
> an extra copy of the data.
> - DataSegmentReader:
> Allows using an SkData directly without copying.
>
> ImageDecoders/FastSharedBufferReader:
> Read data from a SegmentReader instead of directly from a SharedBuffer.
> (Add an ImageDecoder helper that takes a SharedBuffer parameter, reducing
> the need to change code that already has a SharedBuffer.)
> Since the API is the same, the changes here are small, except for
> WEBPImageDecoder, which no longer calls SharedBuffer::data(), and
> instead calls SegmentReader::refAsSkData().
> Use PassRefPtr instead of raw pointers for parameter passing in setData.
>
> Tests:
> Adapt where a SharedBuffer is no longer used.
>
> PictureSnapshot.cpp
> Wrap SkData in SegmentReader.
>
> BUG=467772
> BUG=483369
>
> BUG=335694
> This may also help with crrbug.com/335694. The DecodingImageGenerator
> will no longer attempt to read beyond the data that had been received
> when it was created. However, it theoretically could use a decoder that
> had previously been used to decode more data than it was created with.
>
> e.g.
>
> DeferredImageDecoder did = ...
> did.setData(...);
> SkImage imageA = did.createFrameAtIndex(0); // DecodingImageGenerator
>
> did.setData(...); // More data
> SkImage imageB = did.createFrameAtIndex(0);
>
> // draw imageB - this decodes to the end of data provided in the second
> // call to setData
>
> // draw imageA - this can use the same ImageDecoder retrieved from the
> // ImageDecodingStore, meaning it will have already
> // decoded beyond the data supplied to imageA
>
> I do not know whether this can happen in practice.
>
> Committed: https://crrev.com/33cfb9bd4aec6d9ae36231057436c78b47de8585
> Cr-Commit-Position: refs/heads/master@{#385470}
TBR=reed@google.com,noel@chromium.org,pkasting@chromium.org,fmalita@chromium.org,urvang@chromium.org,scroggo@google.com
Skipping tries as they succeeded except for a test that failed without my patch.
NO_TRY=true
BUG=467772
Committed: https://crrev.com/20d3d134faf15c1b88ce55dff0b80b7a698e2753
Cr-Commit-Position: refs/heads/master@{#385902}
Patch Set 1 #Messages
Total messages: 16 (7 generated)
|
