No more caching volatile bitmaps

No more caching volatile bitmaps

VideoFrames are always marked as volatile and load_yuv_texture() was caching everything, even volatile bitmaps. To solve this issue, we no longer cache volatile bitmaps' YUV planes in load_yuv_texture().

There is another issue which cause this to happen, related to how VideoImageGenerator::onGetYUV8Planes() is implemented, which is logged here: crbug.com/455235

BUG=450706, 450699
Committed: https://skia.googlesource.com/skia/+/ba18f91b9f025dc53ab3cb19ea1407c55ad85937

[sugoi1, 2015-02-04 17:13:54]

sugoi@chromium.org changed reviewers:
+ bsalomon@google.com

[sugoi1, 2015-02-04 17:13:55]

Let's see if this fixes the video frame issues.

[bsalomon, 2015-02-04 17:19:34]

https://codereview.chromium.org/900943002/diff/1/src/gpu/SkGr.cpp
File src/gpu/SkGr.cpp (right):

https://codereview.chromium.org/900943002/diff/1/src/gpu/SkGr.cpp#newcode318
src/gpu/SkGr.cpp:318: const SkBitmap& bm, const GrSurfaceDesc& desc, bool cache)
{
We don't need the cache bool. optionalKey should already be invalid if the
bitmap is not to be cached (setup in GrRefCachedBitmapTexture)

[sugoi1, 2015-02-04 17:59:51]

https://codereview.chromium.org/900943002/diff/1/src/gpu/SkGr.cpp
File src/gpu/SkGr.cpp (right):

https://codereview.chromium.org/900943002/diff/1/src/gpu/SkGr.cpp#newcode318
src/gpu/SkGr.cpp:318: const SkBitmap& bm, const GrSurfaceDesc& desc, bool cache)
{
On 2015/02/04 17:19:33, bsalomon wrote:
> We don't need the cache bool. optionalKey should already be invalid if the
> bitmap is not to be cached (setup in GrRefCachedBitmapTexture)

Done.

[bsalomon, 2015-02-04 18:25:10]

bsalomon@google.com changed reviewers:
+ reed@google.com

[bsalomon, 2015-02-04 18:25:10]

Adding Mike, I don't really know that the contracts here are supposed be.

[bsalomon, 2015-02-04 18:43:01]

Ok, I took another look. So this seems like a correct change to not cache the
YUV data when the bitmap is volatile. But it also seems like this doesn't really
fix the bug, which is in Chrome not Skia. I suppose it works around the issue
when the frame is rendered and the cache data is invalid.

So, lgtm, but I don't think this actually fixes the underlying bug.

[sugoi1, 2015-02-04 18:46:30]

On 2015/02/04 18:43:01, bsalomon wrote:
> Ok, I took another look. So this seems like a correct change to not cache the
> YUV data when the bitmap is volatile. But it also seems like this doesn't
really
> fix the bug, which is in Chrome not Skia. I suppose it works around the issue
> when the frame is rendered and the cache data is invalid.
> 
> So, lgtm, but I don't think this actually fixes the underlying bug.

You're right, but for the underlying bug (which I mentioned in the description:
crbug.com/455235), there's a cl to at least fix it temporarily here:
https://codereview.chromium.org/901573004

This adds a memcpy for now, but fixes the correctness of the function usage.
I've been talking to rileya@ and he'll look for ways to remove the memcpy and
regain performance once this fix is in.

[sugoi1, 2015-02-04 18:46:36]

The CQ bit was checked by sugoi@chromium.org

[commit-bot: I haz the power, 2015-02-04 18:47:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/900943002/20001

[reed1, 2015-02-04 18:48:22]

Here is the change to the YUV signature I have in flight.

https://codereview.chromium.org/863053002/

It should allow the caller to know explicitly how to allocate such that the
codec itself should not need a memcpy. Does that look correct?

[sugoi1, 2015-02-04 18:52:52]

On 2015/02/04 18:48:22, reed1 wrote:
> Here is the change to the YUV signature I have in flight.
> 
> https://codereview.chromium.org/863053002/
> 
> It should allow the caller to know explicitly how to allocate such that the
> codec itself should not need a memcpy. Does that look correct?

I'll cc rileya@ on it so that he can also take a look.

[commit-bot: I haz the power, 2015-02-04 18:53:08]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/ba18f91b9f025dc53ab3cb19ea1407c55ad85937