Commit 6827e418 authored by liberato@chromium.org's avatar liberato@chromium.org Committed by Commit Bot

Turn off overlays for rotated video.

Previously, WMPI checked the video rotation and didn't enable
overlays if it was nonzero.  During the switch to AndroidOverlay,
this is now the decoder's job.  This CL tells MCVD to do that.

Bug: 847338
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ic76c91db1dd37e7e4160e5e7a17e2951b000884b
Reviewed-on: https://chromium-review.googlesource.com/1085691Reviewed-by: default avatarThomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564766}
parent 8bc43cc0
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
#include "media/base/android/android_overlay.h" #include "media/base/android/android_overlay.h"
#include "media/base/video_rotation.h"
#include "media/gpu/media_gpu_export.h" #include "media/gpu/media_gpu_export.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
...@@ -43,6 +44,9 @@ class MEDIA_GPU_EXPORT AndroidVideoSurfaceChooser { ...@@ -43,6 +44,9 @@ class MEDIA_GPU_EXPORT AndroidVideoSurfaceChooser {
// signals, like fs or secure, before we promote. // signals, like fs or secure, before we promote.
bool promote_aggressively = false; bool promote_aggressively = false;
// Default orientation for the video.
VideoRotation video_rotation = VIDEO_ROTATION_0;
// Hint to use for the initial position when transitioning to an overlay. // Hint to use for the initial position when transitioning to an overlay.
gfx::Rect initial_position; gfx::Rect initial_position;
}; };
......
...@@ -56,7 +56,8 @@ void AndroidVideoSurfaceChooserImpl::UpdateState( ...@@ -56,7 +56,8 @@ void AndroidVideoSurfaceChooserImpl::UpdateState(
// We don't want to pick TextureOwner permanently for that. // We don't want to pick TextureOwner permanently for that.
if (overlay_factory_ && if (overlay_factory_ &&
(current_state_.is_fullscreen || current_state_.is_secure || (current_state_.is_fullscreen || current_state_.is_secure ||
current_state_.is_required)) { current_state_.is_required) &&
current_state_.video_rotation == VIDEO_ROTATION_0) {
SwitchToOverlay(false); SwitchToOverlay(false);
} else { } else {
SwitchToTextureOwner(); SwitchToTextureOwner();
...@@ -132,13 +133,18 @@ void AndroidVideoSurfaceChooserImpl::Choose() { ...@@ -132,13 +133,18 @@ void AndroidVideoSurfaceChooserImpl::Choose() {
} }
// If an overlay is required, then choose one. The only way we won't is if we // If an overlay is required, then choose one. The only way we won't is if we
// don't have a factory or our request fails. // don't have a factory or our request fails, or if it's rotated.
if (current_state_.is_required) { if (current_state_.is_required) {
new_overlay_state = kUsingOverlay; new_overlay_state = kUsingOverlay;
// Required overlays don't need to be power efficient. // Required overlays don't need to be power efficient.
needs_power_efficient = false; needs_power_efficient = false;
} }
// Specifying a rotated overlay can NOTREACHED() in the compositor, so it's
// better to fail.
if (current_state_.video_rotation != VIDEO_ROTATION_0)
new_overlay_state = kUsingTextureOwner;
// If we have no factory, then we definitely don't want to use overlays. // If we have no factory, then we definitely don't want to use overlays.
if (!overlay_factory_) if (!overlay_factory_)
new_overlay_state = kUsingTextureOwner; new_overlay_state = kUsingTextureOwner;
......
...@@ -67,6 +67,7 @@ enum class IsSecure { No, Yes }; ...@@ -67,6 +67,7 @@ enum class IsSecure { No, Yes };
enum class IsCCPromotable { No, Yes }; enum class IsCCPromotable { No, Yes };
enum class IsExpectingRelayout { No, Yes }; enum class IsExpectingRelayout { No, Yes };
enum class PromoteAggressively { No, Yes }; enum class PromoteAggressively { No, Yes };
enum class IsVideoRotated { No, Yes };
using TestParams = std::tuple<ShouldUseOverlay, using TestParams = std::tuple<ShouldUseOverlay,
ShouldBePowerEfficient, ShouldBePowerEfficient,
...@@ -76,7 +77,8 @@ using TestParams = std::tuple<ShouldUseOverlay, ...@@ -76,7 +77,8 @@ using TestParams = std::tuple<ShouldUseOverlay,
IsSecure, IsSecure,
IsCCPromotable, IsCCPromotable,
IsExpectingRelayout, IsExpectingRelayout,
PromoteAggressively>; PromoteAggressively,
IsVideoRotated>;
// Useful macro for instantiating tests. // Useful macro for instantiating tests.
#define Either(x) Values(x::No, x::Yes) #define Either(x) Values(x::No, x::Yes)
...@@ -84,8 +86,8 @@ using TestParams = std::tuple<ShouldUseOverlay, ...@@ -84,8 +86,8 @@ using TestParams = std::tuple<ShouldUseOverlay,
// Check if a parameter of type |type| is Yes. |n| is the location of the // Check if a parameter of type |type| is Yes. |n| is the location of the
// parameter of that type. // parameter of that type.
// c++14 can remove |n|, and std::get() by type. // c++14 can remove |n|, and std::get() by type.
#define IsYes(type, n) (::testing::get<n>(GetParam()) == type::Yes); #define IsYes(type, n) (::testing::get<n>(GetParam()) == type::Yes)
#define IsIgnored(type, n) (::testing::get<n>(GetParam()) == type::Ignored); #define IsIgnored(type, n) (::testing::get<n>(GetParam()) == type::Ignored)
} // namespace } // namespace
...@@ -386,6 +388,8 @@ TEST_P(AndroidVideoSurfaceChooserImplTest, OverlayIsUsedOrNotBasedOnState) { ...@@ -386,6 +388,8 @@ TEST_P(AndroidVideoSurfaceChooserImplTest, OverlayIsUsedOrNotBasedOnState) {
chooser_state_.is_compositor_promotable = IsYes(IsCCPromotable, 6); chooser_state_.is_compositor_promotable = IsYes(IsCCPromotable, 6);
chooser_state_.is_expecting_relayout = IsYes(IsExpectingRelayout, 7); chooser_state_.is_expecting_relayout = IsYes(IsExpectingRelayout, 7);
chooser_state_.promote_aggressively = IsYes(PromoteAggressively, 8); chooser_state_.promote_aggressively = IsYes(PromoteAggressively, 8);
chooser_state_.video_rotation =
IsYes(IsVideoRotated, 9) ? VIDEO_ROTATION_90 : VIDEO_ROTATION_0;
MockAndroidOverlay* overlay = overlay_.get(); MockAndroidOverlay* overlay = overlay_.get();
...@@ -421,7 +425,8 @@ INSTANTIATE_TEST_CASE_P(NoFullscreenUsesTextureOwner, ...@@ -421,7 +425,8 @@ INSTANTIATE_TEST_CASE_P(NoFullscreenUsesTextureOwner,
Values(IsSecure::No), Values(IsSecure::No),
Either(IsCCPromotable), Either(IsCCPromotable),
Either(IsExpectingRelayout), Either(IsExpectingRelayout),
Values(PromoteAggressively::No))); Values(PromoteAggressively::No),
Either(IsVideoRotated)));
INSTANTIATE_TEST_CASE_P(FullscreenUsesOverlay, INSTANTIATE_TEST_CASE_P(FullscreenUsesOverlay,
AndroidVideoSurfaceChooserImplTest, AndroidVideoSurfaceChooserImplTest,
...@@ -433,7 +438,8 @@ INSTANTIATE_TEST_CASE_P(FullscreenUsesOverlay, ...@@ -433,7 +438,8 @@ INSTANTIATE_TEST_CASE_P(FullscreenUsesOverlay,
Values(IsSecure::No), Values(IsSecure::No),
Values(IsCCPromotable::Yes), Values(IsCCPromotable::Yes),
Values(IsExpectingRelayout::No), Values(IsExpectingRelayout::No),
Either(PromoteAggressively))); Either(PromoteAggressively),
Values(IsVideoRotated::No)));
INSTANTIATE_TEST_CASE_P(RequiredUsesOverlay, INSTANTIATE_TEST_CASE_P(RequiredUsesOverlay,
AndroidVideoSurfaceChooserImplTest, AndroidVideoSurfaceChooserImplTest,
...@@ -445,7 +451,8 @@ INSTANTIATE_TEST_CASE_P(RequiredUsesOverlay, ...@@ -445,7 +451,8 @@ INSTANTIATE_TEST_CASE_P(RequiredUsesOverlay,
Either(IsSecure), Either(IsSecure),
Either(IsCCPromotable), Either(IsCCPromotable),
Either(IsExpectingRelayout), Either(IsExpectingRelayout),
Either(PromoteAggressively))); Either(PromoteAggressively),
Values(IsVideoRotated::No)));
// Secure textures should use an overlay if the compositor will promote them. // Secure textures should use an overlay if the compositor will promote them.
// We don't care about relayout, since it's transient; either behavior is okay // We don't care about relayout, since it's transient; either behavior is okay
...@@ -460,7 +467,8 @@ INSTANTIATE_TEST_CASE_P(SecureUsesOverlayIfPromotable, ...@@ -460,7 +467,8 @@ INSTANTIATE_TEST_CASE_P(SecureUsesOverlayIfPromotable,
Values(IsSecure::Yes), Values(IsSecure::Yes),
Values(IsCCPromotable::Yes), Values(IsCCPromotable::Yes),
Values(IsExpectingRelayout::No), Values(IsExpectingRelayout::No),
Either(PromoteAggressively))); Either(PromoteAggressively),
Values(IsVideoRotated::No)));
// For all dynamic cases, we shouldn't use an overlay if the compositor won't // For all dynamic cases, we shouldn't use an overlay if the compositor won't
// promote it, unless it's marked as required. This includes secure surfaces, // promote it, unless it's marked as required. This includes secure surfaces,
...@@ -477,7 +485,8 @@ INSTANTIATE_TEST_CASE_P(NotCCPromotableNotRequiredUsesTextureOwner, ...@@ -477,7 +485,8 @@ INSTANTIATE_TEST_CASE_P(NotCCPromotableNotRequiredUsesTextureOwner,
Either(IsSecure), Either(IsSecure),
Values(IsCCPromotable::No), Values(IsCCPromotable::No),
Either(IsExpectingRelayout), Either(IsExpectingRelayout),
Either(PromoteAggressively))); Either(PromoteAggressively),
Either(IsVideoRotated)));
// If we're expecting a relayout, then we should never use an overlay unless // If we're expecting a relayout, then we should never use an overlay unless
// it's required. // it's required.
...@@ -491,7 +500,8 @@ INSTANTIATE_TEST_CASE_P(InsecureExpectingRelayoutUsesTextureOwner, ...@@ -491,7 +500,8 @@ INSTANTIATE_TEST_CASE_P(InsecureExpectingRelayoutUsesTextureOwner,
Either(IsSecure), Either(IsSecure),
Either(IsCCPromotable), Either(IsCCPromotable),
Values(IsExpectingRelayout::Yes), Values(IsExpectingRelayout::Yes),
Either(PromoteAggressively))); Either(PromoteAggressively),
Either(IsVideoRotated)));
// "is_fullscreen" should be enough to trigger an overlay pre-M. // "is_fullscreen" should be enough to trigger an overlay pre-M.
INSTANTIATE_TEST_CASE_P(NotDynamicInFullscreenUsesOverlay, INSTANTIATE_TEST_CASE_P(NotDynamicInFullscreenUsesOverlay,
...@@ -504,7 +514,8 @@ INSTANTIATE_TEST_CASE_P(NotDynamicInFullscreenUsesOverlay, ...@@ -504,7 +514,8 @@ INSTANTIATE_TEST_CASE_P(NotDynamicInFullscreenUsesOverlay,
Either(IsSecure), Either(IsSecure),
Either(IsCCPromotable), Either(IsCCPromotable),
Either(IsExpectingRelayout), Either(IsExpectingRelayout),
Either(PromoteAggressively))); Either(PromoteAggressively),
Values(IsVideoRotated::No)));
// "is_secure" should be enough to trigger an overlay pre-M. // "is_secure" should be enough to trigger an overlay pre-M.
INSTANTIATE_TEST_CASE_P(NotDynamicSecureUsesOverlay, INSTANTIATE_TEST_CASE_P(NotDynamicSecureUsesOverlay,
...@@ -517,7 +528,8 @@ INSTANTIATE_TEST_CASE_P(NotDynamicSecureUsesOverlay, ...@@ -517,7 +528,8 @@ INSTANTIATE_TEST_CASE_P(NotDynamicSecureUsesOverlay,
Values(IsSecure::Yes), Values(IsSecure::Yes),
Either(IsCCPromotable), Either(IsCCPromotable),
Either(IsExpectingRelayout), Either(IsExpectingRelayout),
Either(PromoteAggressively))); Either(PromoteAggressively),
Values(IsVideoRotated::No)));
// "is_required" should be enough to trigger an overlay pre-M. // "is_required" should be enough to trigger an overlay pre-M.
INSTANTIATE_TEST_CASE_P(NotDynamicRequiredUsesOverlay, INSTANTIATE_TEST_CASE_P(NotDynamicRequiredUsesOverlay,
...@@ -530,7 +542,8 @@ INSTANTIATE_TEST_CASE_P(NotDynamicRequiredUsesOverlay, ...@@ -530,7 +542,8 @@ INSTANTIATE_TEST_CASE_P(NotDynamicRequiredUsesOverlay,
Either(IsSecure), Either(IsSecure),
Either(IsCCPromotable), Either(IsCCPromotable),
Either(IsExpectingRelayout), Either(IsExpectingRelayout),
Either(PromoteAggressively))); Either(PromoteAggressively),
Values(IsVideoRotated::No)));
// If we're promoting aggressively, then we should request power efficient. // If we're promoting aggressively, then we should request power efficient.
INSTANTIATE_TEST_CASE_P(AggressiveOverlayIsPowerEfficient, INSTANTIATE_TEST_CASE_P(AggressiveOverlayIsPowerEfficient,
...@@ -543,6 +556,21 @@ INSTANTIATE_TEST_CASE_P(AggressiveOverlayIsPowerEfficient, ...@@ -543,6 +556,21 @@ INSTANTIATE_TEST_CASE_P(AggressiveOverlayIsPowerEfficient,
Values(IsSecure::No), Values(IsSecure::No),
Values(IsCCPromotable::Yes), Values(IsCCPromotable::Yes),
Values(IsExpectingRelayout::No), Values(IsExpectingRelayout::No),
Values(PromoteAggressively::Yes))); Values(PromoteAggressively::Yes),
Values(IsVideoRotated::No)));
// Rotated video is unsupported for overlays in all cases.
INSTANTIATE_TEST_CASE_P(IsVideoRotatedUsesTextureOwner,
AndroidVideoSurfaceChooserImplTest,
Combine(Values(ShouldUseOverlay::No),
Either(ShouldBePowerEfficient),
Either(AllowDynamic),
Either(IsRequired),
Either(IsFullscreen),
Either(IsSecure),
Either(IsCCPromotable),
Either(IsExpectingRelayout),
Either(PromoteAggressively),
Values(IsVideoRotated::Yes)));
} // namespace media } // namespace media
...@@ -185,6 +185,8 @@ void MediaCodecVideoDecoder::Initialize( ...@@ -185,6 +185,8 @@ void MediaCodecVideoDecoder::Initialize(
} }
decoder_config_ = config; decoder_config_ = config;
surface_chooser_helper_.SetVideoRotation(decoder_config_.video_rotation());
output_cb_ = output_cb; output_cb_ = output_cb;
#if BUILDFLAG(USE_PROPRIETARY_CODECS) #if BUILDFLAG(USE_PROPRIETARY_CODECS)
......
...@@ -89,6 +89,10 @@ void SurfaceChooserHelper::SetIsFullscreen(bool is_fullscreen) { ...@@ -89,6 +89,10 @@ void SurfaceChooserHelper::SetIsFullscreen(bool is_fullscreen) {
surface_chooser_state_.is_fullscreen = is_fullscreen; surface_chooser_state_.is_fullscreen = is_fullscreen;
} }
void SurfaceChooserHelper::SetVideoRotation(VideoRotation video_rotation) {
surface_chooser_state_.video_rotation = video_rotation;
}
void SurfaceChooserHelper::UpdateChooserState( void SurfaceChooserHelper::UpdateChooserState(
base::Optional<AndroidOverlayFactoryCB> new_factory) { base::Optional<AndroidOverlayFactoryCB> new_factory) {
surface_chooser_->UpdateState(std::move(new_factory), surface_chooser_state_); surface_chooser_->UpdateState(std::move(new_factory), surface_chooser_state_);
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "media/base/video_rotation.h"
#include "media/gpu/android/android_video_surface_chooser.h" #include "media/gpu/android/android_video_surface_chooser.h"
#include "media/gpu/android/promotion_hint_aggregator.h" #include "media/gpu/android/promotion_hint_aggregator.h"
#include "media/gpu/media_gpu_export.h" #include "media/gpu/media_gpu_export.h"
...@@ -74,6 +75,9 @@ class MEDIA_GPU_EXPORT SurfaceChooserHelper { ...@@ -74,6 +75,9 @@ class MEDIA_GPU_EXPORT SurfaceChooserHelper {
// Notify us about the fullscreen state. Does not update the chooser state. // Notify us about the fullscreen state. Does not update the chooser state.
void SetIsFullscreen(bool is_fullscreen); void SetIsFullscreen(bool is_fullscreen);
// Notify us about the default rotation for the video.
void SetVideoRotation(VideoRotation video_rotation);
// Update the chooser state using the given factory. // Update the chooser state using the given factory.
void UpdateChooserState(base::Optional<AndroidOverlayFactoryCB> new_factory); void UpdateChooserState(base::Optional<AndroidOverlayFactoryCB> new_factory);
......
...@@ -77,6 +77,13 @@ TEST_F(SurfaceChooserHelperTest, SetIsFullscreen) { ...@@ -77,6 +77,13 @@ TEST_F(SurfaceChooserHelperTest, SetIsFullscreen) {
// We don't really care if it sets expecting_relayout, clears it, or not. // We don't really care if it sets expecting_relayout, clears it, or not.
} }
TEST_F(SurfaceChooserHelperTest, SetVideoRotation) {
// VideoRotation should be forwarded to the chooser.
helper_->SetVideoRotation(VIDEO_ROTATION_90);
UpdateChooserState();
ASSERT_EQ(chooser_->current_state_.video_rotation, VIDEO_ROTATION_90);
}
TEST_F(SurfaceChooserHelperTest, SetIsOverlayRequired) { TEST_F(SurfaceChooserHelperTest, SetIsOverlayRequired) {
// The default helper was created without |is_required|, so verify that. // The default helper was created without |is_required|, so verify that.
UpdateChooserState(); UpdateChooserState();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment