New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 662251 link

Starred by 2 users
Status: Fixed
Owner:
dalecur...@chromium.org
Closed: Nov 2016
Cc:
█ w...@chromium.org
liber...@chromium.org
█ renganat...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature

Blocking:
issue 664366



Sign in to add a comment

Add support for MediaCodec.setOutputSurface() on Marshmallow+ devices.

Project Member Reported by dalecur...@chromium.org, Nov 4 2016 
As titled, on newer devices we can avoid having to restart the MediaCodec when switching in and out of fullscreen using this API.

https://developer.android.com/reference/android/media/MediaCodec.html#setOutputSurface(android.view.Surface)

Since DialogSurface and AVDAv2 are a milestone or two away, it makes sense to implement this sooner than later if it's not too complicated (and indeed it turned out not to be).

This results in a much smoother fullscreen transition for users and avoids any audio dropouts during the transition.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 4 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d65783431437f61ed9962c4e6968fce6103ca2e1

commit d65783431437f61ed9962c4e6968fce6103ca2e1
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Nov 04 23:23:42 2016

Split out AVDA cleanups from SetSurface() change.

This shifts various pieces of AVDA code around to make it easier
to implement MediaCodec.setSurface(). There are no functional
changes within this CL, just cleanups and relocations. Namely:

- OnFrameAvailableHandler moves into AVDASharedState since we'll
  change AVDASharedState instances in the future when the surface
  changes.
- The map of codec images is now owned by AVDAPictureBufferManager;
  consequently, self-registration in the AVDACodecImage is removed.
- Elided some methods to remove duplicate internal work.
- AVDACodecImages are now held by refptr in AVDAPictureBufferManager
  since we'll need to call code which would otherwise drop their
  last ref in the future.

BUG= 662251 
TEST=playback still works.

Review-Url: https://codereview.chromium.org/2475133002
Cr-Commit-Position: refs/heads/master@{#430064}

[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/android_video_decode_accelerator.cc
[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/avda_codec_image.cc
[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/avda_codec_image.h
[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/avda_picture_buffer_manager.cc
[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/avda_picture_buffer_manager.h
[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/avda_shared_state.cc
[modify] https://crrev.com/d65783431437f61ed9962c4e6968fce6103ca2e1/media/gpu/avda_shared_state.h
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 9 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3704fcabb140cd633890479194658742863c881

commit d3704fcabb140cd633890479194658742863c881
Author: dalecurtis <dalecurtis@chromium.org>
Date: Wed Nov 09 22:44:09 2016

Enforce no uncleared OES external textures. Clear in AVDA, not GVD.

A few of us on the team have been bitten by the fact that GVD is
the one clearing textures instead of AVDA when it comes to OES
external textures. Lets save a few hours of debugging for someone
in the future.

OES external textures are now always pre-cleared by AVDA (the only
VDA using these) and checked by GpuVideoDecoder. Instead of an opaque
GL_INVALID_ENUM error in GLES2DecoderImpl::PrepareTexturesForRender()
a DCHECK will now fire in ClearLevel() indicating the exact problem.

BUG= 662251 
TEST=implement fancy new feature, notice there are no holes in foot.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2481943003
Cr-Commit-Position: refs/heads/master@{#431064}

[modify] https://crrev.com/d3704fcabb140cd633890479194658742863c881/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/d3704fcabb140cd633890479194658742863c881/media/gpu/avda_picture_buffer_manager.cc
[modify] https://crrev.com/d3704fcabb140cd633890479194658742863c881/media/gpu/ipc/service/gpu_video_decode_accelerator.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b632fce277d847aa2c5a533478da715086c0b55

commit 4b632fce277d847aa2c5a533478da715086c0b55
Author: dalecurtis <dalecurtis@chromium.org>
Date: Thu Nov 10 00:52:17 2016

Use MediaCodec.setOutputSurface() for fullscreen transitions on M.

Turns out this isn't so hard to do that we need to wait for the
completion of DialogSurface and AVDAv2.

This dramatically improves the fullscreen transition; it's much
faster and does not cause audio jank.

BUG= 662251 
TEST=enter/exit fullscreen on M+ device.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2461073002
Cr-Commit-Position: refs/heads/master@{#431114}

[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/base/android/sdk_media_codec_bridge.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/base/android/sdk_media_codec_bridge.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/base/surface_manager.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/filters/gpu_video_decoder.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/android_video_decode_accelerator.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/android_video_decode_accelerator.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/avda_codec_image.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/avda_codec_image.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/avda_picture_buffer_manager.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/avda_picture_buffer_manager.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/avda_shared_state.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/ipc/client/gpu_video_decode_accelerator_host.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/ipc/common/media_messages.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/ipc/common/media_param_traits_macros.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/ipc/service/gpu_video_decode_accelerator.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/gpu/ipc/service/gpu_video_decode_accelerator.h
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/video/video_decode_accelerator.cc
[modify] https://crrev.com/4b632fce277d847aa2c5a533478da715086c0b55/media/video/video_decode_accelerator.h

Comment 4 by dalecur...@chromium.org, Nov 10 2016

Status: Fixed (was: Started)
This is available in the latest canary and should hit dev soon. I looked through the crash reports and don't see anything to fix, so closing this.

Comment 5 by kbr@chromium.org, Nov 11 2016

Blocking: 664366
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b3c4d675143972351fe4e2d5c3ac903455f0a64

commit 5b3c4d675143972351fe4e2d5c3ac903455f0a64
Author: dalecurtis <dalecurtis@chromium.org>
Date: Wed Nov 16 08:13:07 2016

Fix broken SurfaceView usage on < M devices.

This partially reverts commit 4b632fce277d847aa2c5a533478da715086c0b55
which added support for SurfaceView changes without a restart on M+
devices. Unfortunately we can't use SetSurface() prior to initialize
on <M devices since the VDA hasn't been created by this point. The
surface must be provided in the initial config.

BUG= 662251 
TEST=enter/exit fullscreen on <M and >M device.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2507463004
Cr-Commit-Position: refs/heads/master@{#432423}

[modify] https://crrev.com/5b3c4d675143972351fe4e2d5c3ac903455f0a64/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/5b3c4d675143972351fe4e2d5c3ac903455f0a64/media/filters/gpu_video_decoder.h
[modify] https://crrev.com/5b3c4d675143972351fe4e2d5c3ac903455f0a64/media/gpu/android_video_decode_accelerator.cc
[modify] https://crrev.com/5b3c4d675143972351fe4e2d5c3ac903455f0a64/media/gpu/android_video_decode_accelerator.h
[modify] https://crrev.com/5b3c4d675143972351fe4e2d5c3ac903455f0a64/media/gpu/ipc/common/media_param_traits_macros.h
[modify] https://crrev.com/5b3c4d675143972351fe4e2d5c3ac903455f0a64/media/video/video_decode_accelerator.h
►

Sign in to add a comment