Don't force clearing of textures that the HW decoder will overwrite.

Don't force clearing of textures that the HW decoder will overwrite.
Just mark them as cleared instead.

BUG=chromium:163268
TEST=Captured Youtube 1080p traces on Lumpy.

[Pawel Osciak, 2012-12-11 08:39:32]

lgtm

[Ami GONE FROM CHROMIUM, 2012-12-11 17:20:01]

I think you need someone from chrome-gpu to vet this change, since personally I
don't understand the implications of ClearRenderableLevels vs. SetLevelCleared. 
Al: are you that person? :)

Specifically I'm worried about these things:
a) what are the security implications; are there (error?) codepaths that will
lead to information leaking; and
b) why is this change even necessary?  In the linked bug posciak@ says
GLES2Decoder::ClearLevel() takes ~100ms per texture, but that's insane, isn't
it?  Can we make that not take so long?  Is there lock contention that can be
avoided, or is this really saturation of the memory bus?  100ms for a 1080p RGBA
texture being written twice and read once (memset to zero of the new[]'d space,
then memcpy for the teximage) implies a memory bandwidth of 720MB/s.  I thought
lumpy had ~10x that bandwidth, no?

https://codereview.chromium.org/11477004/diff/1/content/common/gpu/media/gpu_...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

https://codereview.chromium.org/11477004/diff/1/content/common/gpu/media/gpu_...
content/common/gpu/media/gpu_video_decode_accelerator.cc:228:
texture_manager->SetLevelCleared(info, GL_TEXTURE_2D, 0);
The texture target may not be GL_TEXTURE_2D (see ProvidePictureBuffers for
example).

[ilja, 2012-12-11 22:43:39]

In my understanding:
ClearRenderableLevels = wipes memory.
SetLevelCleared = mark as wiped.

I would like to add one observation: your ClearRenderableLevels is redundant. It
can be safely removed and the manager will still clear before a use (which is
good). I am thinking to remove the ClearRenderableLevels completely and put the
SetLevelCleared into PictureReady. As long as SetLevelCleared is cheap enough
(it would be called on every frame) that would be the safest place to mark it.
Thoughts?

[Ami GONE FROM CHROMIUM, 2012-12-11 22:52:35]

On 2012/12/11 22:43:39, ilja wrote:
> In my understanding:
> ClearRenderableLevels = wipes memory.
> SetLevelCleared = mark as wiped.

Right; that's my (uninformed) understanding as well; I'd just like to get a
confirmation from someone who knows this code deeply (if that describes you,
then that's fine by me).

> I would like to add one observation: your ClearRenderableLevels is redundant.
It
> can be safely removed and the manager will still clear before a use (which is
> good).

except that clear happens after the decoder has written the frame into the
texture.  This is why I added the Clear in the first place, because HW decode
was broken and all frames would come out as pure black (RGBA zeros).

> I am thinking to remove the ClearRenderableLevels completely and put the
> SetLevelCleared into PictureReady. As long as SetLevelCleared is cheap enough
> (it would be called on every frame) that would be the safest place to mark it.
> Thoughts?

I like it for avoiding worries about security of error paths (since the manager
will clear if we don't) .
If it works perf-wise and passes muster with GPU folk, SGTM.

I'm still curious why clearing a 1080p frame takes 100ms, fwiw...

[Ami GONE FROM CHROMIUM, 2013-10-31 21:31:35]

Time to close?