Disable adaptive resolution support for MediaCodec use by AVDA.

Disable adaptive resolution support for MediaCodec use by AVDA.

There are several devices out there that can't create more than one
MediaCodec when these keys are set. Specifically the N4 and N7
devices will fail while creating the second MediaCodec. Looking
through the AOSP source code this appears to be because setting
max width/max height to 1080p allocates all available decoder
resources on these devices: https://goo.gl/rH7iht

More importantly, configuration changes as sent by MSE within the
Spitzer pipeline always trigger a flush(), so regardless of the
adaptative support setting we're taking the "slow" path per the
MediaCodec documentation.

For files with implicit mid-stream configuration changes (which are
rare), there seems to be no issue on a variety of devices with this
setting disabled. In fact, it doesn't seem any faster or slower even
on very old (4.3) devices w/ or w/o this setting.

Further, at least for mobile devices, I'm not sure we ever want to
set a max height or width even if the functionality did what's on
the label due to such a value being unknown to Chrome. Always using
the largest value works, but as found above allocates far more
resources than are ever realistically used in mobile playbacks.

Note: This is slightly different than general adapative playback as
supported by MSE, since that may just adapt the bitrate while keeping
the resolution the same.

BUG=none
TEST=http://storage.googleapis.com/dalecurtis-shared/buck_multi.html
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.webm
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.mp4
N4 (4.4), N7 (2012, 4.3), N4 (5.1), N5 (6.0), S4 (5.1)

Committed: https://crrev.com/f3b596f026ece5ca021f547d41193f9bbbeec604
Cr-Commit-Position: refs/heads/master@{#377170}

[DaleCurtis, 2016-02-23 22:28:15]

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

[DaleCurtis, 2016-02-23 22:35:34]

Description was changed from

==========
Disable adaptive playback support for MediaCodec use by AVDA.

There are several devices out there that can't create more than one
MediaCodec when these keys are set. Specifically the N4 and N7
devices will fail while creating the second MediaCodec. Looking
through the AOSP source code this appears to be because setting
max width/max height to 1080p allocates all available decoder
resources on these devices: https://goo.gl/rH7iht

More importantly, configuration changes as sent by MSE within the
Spitzer pipeline always trigger a flush(), so regardless of the
adaptative support setting we're taking the "slow" path per the
MediaCodec documentation.

For files with implicit mid-stream configuration changes (which are
rare), there seems to be no issue on a variety of devices with this
setting disabled. In fact, it doesn't seem any faster or slower even
on very old (4.3) devices w/ or w/o this setting.

Further, at least for mobile devices, I'm not sure we ever want to
set a max height or width even if the functionality did what's on
the label due to such a value being unknown to Chrome. Always using
the largest value works, but as found above allocates far more
resources than are ever realistically used in mobile playbacks.

BUG=none
TEST=http://storage.googleapis.com/dalecurtis-shared/buck_multi.html
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.webm
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.mp4
N4 (4.4), N7 (2012, 4.3), N4 (5.1), N5 (6.0), S4 (5.1)
==========

to

==========
Disable adaptive resolution support for MediaCodec use by AVDA.

There are several devices out there that can't create more than one
MediaCodec when these keys are set. Specifically the N4 and N7
devices will fail while creating the second MediaCodec. Looking
through the AOSP source code this appears to be because setting
max width/max height to 1080p allocates all available decoder
resources on these devices: https://goo.gl/rH7iht

More importantly, configuration changes as sent by MSE within the
Spitzer pipeline always trigger a flush(), so regardless of the
adaptative support setting we're taking the "slow" path per the
MediaCodec documentation.

For files with implicit mid-stream configuration changes (which are
rare), there seems to be no issue on a variety of devices with this
setting disabled. In fact, it doesn't seem any faster or slower even
on very old (4.3) devices w/ or w/o this setting.

Further, at least for mobile devices, I'm not sure we ever want to
set a max height or width even if the functionality did what's on
the label due to such a value being unknown to Chrome. Always using
the largest value works, but as found above allocates far more
resources than are ever realistically used in mobile playbacks.

Note: This is slightly different than general adapative playback as
supported by MSE, since that may just adapt the bitrate while keeping
the resolution the same.

BUG=none
TEST=http://storage.googleapis.com/dalecurtis-shared/buck_multi.html
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.webm
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.mp4
N4 (4.4), N7 (2012, 4.3), N4 (5.1), N5 (6.0), S4 (5.1)
==========

[liberato (no reviews please), 2016-02-23 22:53:43]

https://codereview.chromium.org/1726103002/diff/1/media/base/android/sdk_medi...
File media/base/android/sdk_media_codec_bridge.cc (right):

https://codereview.chromium.org/1726103002/diff/1/media/base/android/sdk_medi...
media/base/android/sdk_media_codec_bridge.cc:569: bool enable_adaptive_playback)
{
s/enable/allow/?

[qinmin, 2016-02-23 22:53:54]

"For files with implicit mid-stream configuration changes (which are
rare), there seems to be no issue on a variety of devices with this
setting disabled."

I don't think this is true. For VP8, MediaCodec will crash if stream
configuration changes on nexus 4. Some codecs, like h.264 might have better
fault tolerance on  configuration change. However, there is no guarantee that
all OEMs will play nice on all media formats.

[DaleCurtis, 2016-02-23 23:04:19]

On 2016/02/23 at 22:53:54, qinmin wrote:
> "For files with implicit mid-stream configuration changes (which are
> rare), there seems to be no issue on a variety of devices with this
> setting disabled."
> 
> I don't think this is true. For VP8, MediaCodec will crash if stream
configuration changes on nexus 4. Some codecs, like h.264 might have better
fault tolerance on  configuration change. However, there is no guarantee that
all OEMs will play nice on all media formats.

Well we're only really concerned with hardware VP8, VP9, and H264 codecs now,
since we'll use software libvpx for software vp8, vp9. Today midstream
resolution changes on hardware VP8 don't work anyways -- even on Marshmallow --
there is visible corruption during playback. So your point is fair, i.e. there
may indeed be broken vendors out there... but w/o these keys we can get 6+ H264
MediaCodec instances on something as old as N7 2012 w/ JellyBean/KitKat.
Normally we can only get 1 instance. I think the benefits of not setting these
keys far outweigh that of the purported "fast" path.

Further if this truly causes crashes we can find out in the field and put these
devices on a blacklist path.

[DaleCurtis, 2016-02-23 23:07:42]

https://codereview.chromium.org/1726103002/diff/1/media/base/android/sdk_medi...
File media/base/android/sdk_media_codec_bridge.cc (right):

https://codereview.chromium.org/1726103002/diff/1/media/base/android/sdk_medi...
media/base/android/sdk_media_codec_bridge.cc:569: bool enable_adaptive_playback)
{
On 2016/02/23 at 22:53:42, liberato wrote:
> s/enable/allow/?

Done.

[liberato (no reviews please), 2016-02-23 23:10:26]

lgtm

[DaleCurtis, 2016-02-23 23:10:50]

Also, if we really want (because it's an issue), we can detect mid-stream
resolution changes using the (H264|VP8|VP9)Parser classes and flush the codec
prior to sending in buffers with a resolution change.

[qinmin, 2016-02-23 23:29:23]

lgtm % nit

https://codereview.chromium.org/1726103002/diff/20001/media/base/android/sdk_...
File media/base/android/sdk_media_codec_bridge.h (right):

https://codereview.chromium.org/1726103002/diff/20001/media/base/android/sdk_...
media/base/android/sdk_media_codec_bridge.h:171: // supported.
nit: this fits into the previous line.

[DaleCurtis, 2016-02-23 23:31:27]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-02-23 23:31:28]

Thanks for review!

https://codereview.chromium.org/1726103002/diff/20001/media/base/android/sdk_...
File media/base/android/sdk_media_codec_bridge.h (right):

https://codereview.chromium.org/1726103002/diff/20001/media/base/android/sdk_...
media/base/android/sdk_media_codec_bridge.h:171: // supported.
On 2016/02/23 at 23:29:22, qinmin wrote:
> nit: this fits into the previous line.

Done.

[DaleCurtis, 2016-02-23 23:31:28]

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

[commit-bot: I haz the power, 2016-02-23 23:36:19]

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

[commit-bot: I haz the power, 2016-02-24 01:20:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-24 01:21:40]

Description was changed from

==========
Disable adaptive resolution support for MediaCodec use by AVDA.

There are several devices out there that can't create more than one
MediaCodec when these keys are set. Specifically the N4 and N7
devices will fail while creating the second MediaCodec. Looking
through the AOSP source code this appears to be because setting
max width/max height to 1080p allocates all available decoder
resources on these devices: https://goo.gl/rH7iht

More importantly, configuration changes as sent by MSE within the
Spitzer pipeline always trigger a flush(), so regardless of the
adaptative support setting we're taking the "slow" path per the
MediaCodec documentation.

For files with implicit mid-stream configuration changes (which are
rare), there seems to be no issue on a variety of devices with this
setting disabled. In fact, it doesn't seem any faster or slower even
on very old (4.3) devices w/ or w/o this setting.

Further, at least for mobile devices, I'm not sure we ever want to
set a max height or width even if the functionality did what's on
the label due to such a value being unknown to Chrome. Always using
the largest value works, but as found above allocates far more
resources than are ever realistically used in mobile playbacks.

Note: This is slightly different than general adapative playback as
supported by MSE, since that may just adapt the bitrate while keeping
the resolution the same.

BUG=none
TEST=http://storage.googleapis.com/dalecurtis-shared/buck_multi.html
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.webm
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.mp4
N4 (4.4), N7 (2012, 4.3), N4 (5.1), N5 (6.0), S4 (5.1)
==========

to

==========
Disable adaptive resolution support for MediaCodec use by AVDA.

There are several devices out there that can't create more than one
MediaCodec when these keys are set. Specifically the N4 and N7
devices will fail while creating the second MediaCodec. Looking
through the AOSP source code this appears to be because setting
max width/max height to 1080p allocates all available decoder
resources on these devices: https://goo.gl/rH7iht

More importantly, configuration changes as sent by MSE within the
Spitzer pipeline always trigger a flush(), so regardless of the
adaptative support setting we're taking the "slow" path per the
MediaCodec documentation.

For files with implicit mid-stream configuration changes (which are
rare), there seems to be no issue on a variety of devices with this
setting disabled. In fact, it doesn't seem any faster or slower even
on very old (4.3) devices w/ or w/o this setting.

Further, at least for mobile devices, I'm not sure we ever want to
set a max height or width even if the functionality did what's on
the label due to such a value being unknown to Chrome. Always using
the largest value works, but as found above allocates far more
resources than are ever realistically used in mobile playbacks.

Note: This is slightly different than general adapative playback as
supported by MSE, since that may just adapt the bitrate while keeping
the resolution the same.

BUG=none
TEST=http://storage.googleapis.com/dalecurtis-shared/buck_multi.html
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.webm
http://storage.googleapis.com/dalecurtis-shared/frame_size_change.mp4
N4 (4.4), N7 (2012, 4.3), N4 (5.1), N5 (6.0), S4 (5.1)

Committed: https://crrev.com/f3b596f026ece5ca021f547d41193f9bbbeec604
Cr-Commit-Position: refs/heads/master@{#377170}
==========

[commit-bot: I haz the power, 2016-02-24 01:21:42]

Patchset 3 (id:??) landed as
https://crrev.com/f3b596f026ece5ca021f547d41193f9bbbeec604
Cr-Commit-Position: refs/heads/master@{#377170}