Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320
Committed: https://crrev.com/9dab4cbf48d2a8828915beaf7c90c6607d48d68b
Cr-Commit-Position: refs/heads/master@{#374971}

[magjed_chromium, 2016-02-08 16:55:07]

Description was changed from

==========
Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW
VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an
invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and
GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it
as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320
==========

to

==========
Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW
VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an
invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and
GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it
as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320
==========

[magjed_chromium, 2016-02-08 16:55:07]

magjed@chromium.org changed reviewers:
+ qinmin@chromium.org

[magjed_chromium, 2016-02-08 21:58:04]

qinmin - Please take a look.

https://codereview.webrtc.org/1681613002/diff/1/media/base/android/java/src/o...
File media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
(left):

https://codereview.webrtc.org/1681613002/diff/1/media/base/android/java/src/o...
media/base/android/java/src/org/chromium/media/MediaCodecBridge.java:317:
mMediaCodec.getInputBuffer(index);
qinmin: What was the purpose of adding this call in https://crrev.com/669623004?

https://codereview.webrtc.org/1681613002/diff/1/media/base/android/java/src/o...
media/base/android/java/src/org/chromium/media/MediaCodecBridge.java:372:
mMediaCodec.getOutputBuffer(index);
ditto

[qinmin, 2016-02-09 20:09:00]

lgtm

[magjed_chromium, 2016-02-10 13:00:31]

magjed@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[magjed_chromium, 2016-02-10 13:01:05]

+dalecurtis for content/common/gpu/media/android_video_decode_accelerator.*
OWNER

[DaleCurtis, 2016-02-10 17:52:47]

dalecurtis@chromium.org changed reviewers:
+ liberato@chromium.org

[DaleCurtis, 2016-02-10 17:52:47]

dalecurtis@chromium.org changed required reviewers:
+ liberato@chromium.org

[DaleCurtis, 2016-02-10 17:52:48]

+liberato for avda changes. I'll stamp after he's happy.

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
File content/common/gpu/media/android_video_encode_accelerator.cc (right):

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
content/common/gpu/media/android_video_encode_accelerator.cc:168: unsigned int
frame_input_count;
We don't use unsigned in Chromium code, this should be uint32_t, size_t, or just
plain int if you don't need an unsigned value.

[liberato (no reviews please), 2016-02-10 18:40:09]

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
File content/common/gpu/media/android_video_encode_accelerator.cc (right):

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
content/common/gpu/media/android_video_encode_accelerator.cc:405:
media_codec_->GetOutputFormat(&width, &height);
i don't understand the goal of asking for output size during encoding.  can you
provide some background on why it does this?  checking in Encode (as you've
already done) seems like the right place to do it.

admittedly, i'm not very familiar with how MC behaves when encoding.  is there
something subtle here?

[magjed_chromium, 2016-02-10 19:06:02]

Patchset #2 (id:20001) has been deleted

[magjed_chromium, 2016-02-10 20:25:12]

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
File content/common/gpu/media/android_video_encode_accelerator.cc (right):

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
content/common/gpu/media/android_video_encode_accelerator.cc:168: unsigned int
frame_input_count;
On 2016/02/10 17:52:48, DaleCurtis wrote:
> We don't use unsigned in Chromium code, this should be uint32_t, size_t, or
just
> plain int if you don't need an unsigned value.

Done, changed to uint32_t. The reason I used 'unsigned int' was to be consistent
with the declaration of
VideoEncodeAccelerator::Client::RequireBitstreamBuffers()
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/video_...
where |frame_input_count| is used.

https://codereview.chromium.org/1681613002/diff/1/content/common/gpu/media/an...
content/common/gpu/media/android_video_encode_accelerator.cc:405:
media_codec_->GetOutputFormat(&width, &height);
On 2016/02/10 18:40:09, liberato wrote:
> i don't understand the goal of asking for output size during encoding.  can
you
> provide some background on why it does this?  checking in Encode (as you've
> already done) seems like the right place to do it.
> 
> admittedly, i'm not very familiar with how MC behaves when encoding.  is there
> something subtle here?

It's just a sanity check to make sure the resolution of the encoded video hasn't
changed. I tried to make it similar to the previous DoOutputBuffersSuffice().
I'm pretty sure MEDIA_CODEC_OUTPUT_FORMAT_CHANGED should never be called at all
for video encoder so I replaced this with a simple RETURN_ON_FAILURE() instead.

[liberato (no reviews please), 2016-02-11 15:29:54]

android_video_encode_accelerator.cc: lgtm

[DaleCurtis, 2016-02-11 17:26:44]

lgtm

[magjed_chromium, 2016-02-11 17:33:00]

The CQ bit was checked by magjed@chromium.org

[magjed_chromium, 2016-02-11 17:33:01]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/1681613002/#ps40001
(title: "Addressing comments")

[commit-bot: I haz the power, 2016-02-11 17:35:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1681613002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1681613002/40001

[commit-bot: I haz the power, 2016-02-11 20:21:44]

Description was changed from

==========
Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW
VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an
invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and
GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it
as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320
==========

to

==========
Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW
VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an
invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and
GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it
as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320
==========

[commit-bot: I haz the power, 2016-02-11 20:21:45]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-02-16 22:37:38]

Description was changed from

==========
Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW
VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an
invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and
GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it
as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320
==========

to

==========
Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

The calls were deprecated here: https://crrev.com/669623004, and this broke HW
VP8 encoding. This is because GetOutputBuffersCapacity() returns -1, causing an
invalid shared memory allocation.

This CL removes the need for GetOutputBuffersCount() and
GetOutputBuffersCapacity() and replaces them with reasonable defaults instead.

HW VP8 encoding will still not work after this CL, because another CL broke it
as well: https://crrev.com/1472943002. That will be fixed in a follow-up CL.

BUG=445320

Committed: https://crrev.com/9dab4cbf48d2a8828915beaf7c90c6607d48d68b
Cr-Commit-Position: refs/heads/master@{#374971}
==========

[commit-bot: I haz the power, 2016-02-16 22:37:39]

Patchset 2 (id:??) landed as
https://crrev.com/9dab4cbf48d2a8828915beaf7c90c6607d48d68b
Cr-Commit-Position: refs/heads/master@{#374971}