Skip to content

Commit

Permalink
[Cast Streaming] Disable feature to disable letterboxing.
Browse files Browse the repository at this point in the history
More devices require letterboxing to render mirroring frames correctly.
Disable the feature for M110 until a better strategy for rolling out
this deprecation can be determined.

(cherry picked from commit 526cf4c)

Bug: 1363512,1400691
Change-Id: I5fb82c7ea8cbacd31c464022954f6d7ef9d764ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4112407
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
Reviewed-by: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Benjamin Zielinski <bzielinski@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1084096}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4134664
Reviewed-by: Ryan Keane <rwkeane@google.com>
Cr-Commit-Position: refs/branch-heads/5481@{#112}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
mfoltzgoogle authored and Chromium LUCI CQ committed Jan 4, 2023
1 parent 404cb94 commit 031a1c9
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 12 deletions.
2 changes: 1 addition & 1 deletion components/mirroring/service/mirroring_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace features {
// TODO(crbug.com/1363512): Remove support for sender side letterboxing.
BASE_FEATURE(kCastDisableLetterboxing,
"CastDisableLetterboxing",
base::FEATURE_ENABLED_BY_DEFAULT);
base::FEATURE_DISABLED_BY_DEFAULT);

// The mirroring service previously used a model name filter before even
// attempting to query the receiver for media remoting support. This
Expand Down
14 changes: 7 additions & 7 deletions components/mirroring/service/openscreen_session_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -582,28 +582,28 @@ TEST_F(OpenscreenSessionHostTest, AudioAndVideoMirroring) {
StopSession();
}

// TODO(crbug.com/1363512): Remove support for sender side letterboxing.
TEST_F(OpenscreenSessionHostTest, AnswerWithConstraints) {
SetAnswer(std::make_unique<openscreen::cast::Answer>(kAnswerWithConstraints));
media::VideoCaptureParams::SuggestedConstraints expected_constraints = {
.min_frame_size = gfx::Size(2, 2),
.min_frame_size = gfx::Size(320, 180),
.max_frame_size = gfx::Size(1920, 1080),
.fixed_aspect_ratio = false};
.fixed_aspect_ratio = true};
CreateSession(SessionType::AUDIO_AND_VIDEO);
StartSession();
StopSession();
EXPECT_EQ(video_host_->GetVideoCaptureParams().SuggestConstraints(),
expected_constraints);
}

// TODO(crbug.com/1363512): Remove support for sender side letterboxing.
TEST_F(OpenscreenSessionHostTest, AnswerWithConstraintsLetterboxEnabled) {
TEST_F(OpenscreenSessionHostTest, AnswerWithConstraintsLetterboxDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(features::kCastDisableLetterboxing);
feature_list.InitAndEnableFeature(features::kCastDisableLetterboxing);
SetAnswer(std::make_unique<openscreen::cast::Answer>(kAnswerWithConstraints));
media::VideoCaptureParams::SuggestedConstraints expected_constraints = {
.min_frame_size = gfx::Size(320, 180),
.min_frame_size = gfx::Size(2, 2),
.max_frame_size = gfx::Size(1920, 1080),
.fixed_aspect_ratio = true};
.fixed_aspect_ratio = false};
CreateSession(SessionType::AUDIO_AND_VIDEO);
StartSession();
StopSession();
Expand Down
31 changes: 27 additions & 4 deletions components/mirroring/service/session_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ class SessionTest : public mojom::ResourceProvider,
OnConnectToRemotingSource();
}

void ForceLetterboxing() { force_letterboxing_ = true; }

void SendAnswer() {
ASSERT_TRUE(session_);
std::vector<FrameSenderConfig> audio_configs;
Expand Down Expand Up @@ -266,6 +268,9 @@ class SessionTest : public mojom::ResourceProvider,
session_params->target_playout_delay =
base::Milliseconds(target_playout_delay_ms_);
}
if (force_letterboxing_) {
session_params->force_letterboxing = true;
}
session_params->is_remote_playback = is_remote_playback_;
cast_mode_ = "mirroring";
mojo::PendingRemote<mojom::ResourceProvider> resource_provider_remote;
Expand Down Expand Up @@ -488,6 +493,7 @@ class SessionTest : public mojom::ResourceProvider,
int32_t offer_sequence_number_ = -1;
int32_t capability_sequence_number_ = -1;
int32_t target_playout_delay_ms_ = kDefaultPlayoutDelay;
bool force_letterboxing_{false};

std::unique_ptr<Session> session_;
std::unique_ptr<MockNetworkContext> network_context_;
Expand Down Expand Up @@ -515,7 +521,25 @@ TEST_F(SessionTest, AudioAndVideoMirroring) {
StopSession();
}

TEST_F(SessionTest, AnswerWithConstraints) {
// TODO(crbug.com/1363512): Remove support for sender side letterboxing.
TEST_F(SessionTest, AnswerWithConstraintsLetterboxEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(features::kCastDisableLetterboxing);
SetAnswer(std::make_unique<openscreen::cast::Answer>(kAnswerWithConstraints));
media::VideoCaptureParams::SuggestedConstraints expected_constraints = {
.min_frame_size = gfx::Size(320, 180),
.max_frame_size = gfx::Size(1280, 720),
.fixed_aspect_ratio = true};
CreateSession(SessionType::AUDIO_AND_VIDEO);
StartSession();
StopSession();
EXPECT_EQ(video_host_->GetVideoCaptureParams().SuggestConstraints(),
expected_constraints);
}

TEST_F(SessionTest, AnswerWithConstraintsLetterboxDisabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kCastDisableLetterboxing);
SetAnswer(std::make_unique<openscreen::cast::Answer>(kAnswerWithConstraints));
media::VideoCaptureParams::SuggestedConstraints expected_constraints = {
.min_frame_size = gfx::Size(2, 2),
Expand All @@ -529,9 +553,8 @@ TEST_F(SessionTest, AnswerWithConstraints) {
}

// TODO(crbug.com/1363512): Remove support for sender side letterboxing.
TEST_F(SessionTest, AnswerWithConstraintsLetterboxEnabled) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndDisableFeature(features::kCastDisableLetterboxing);
TEST_F(SessionTest, AnswerWithConstraintsLetterboxForced) {
ForceLetterboxing();
SetAnswer(std::make_unique<openscreen::cast::Answer>(kAnswerWithConstraints));
media::VideoCaptureParams::SuggestedConstraints expected_constraints = {
.min_frame_size = gfx::Size(320, 180),
Expand Down
16 changes: 16 additions & 0 deletions media/capture/video_capture_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

#include "media/capture/video_capture_types.h"

#include <ostream>

#include "base/check.h"
#include "base/ranges/algorithm.h"
#include "base/strings/strcat.h"
#include "base/strings/stringprintf.h"
#include "media/base/limits.h"

Expand Down Expand Up @@ -71,6 +74,13 @@ bool VideoCaptureParams::IsValid() const {
power_line_frequency <= PowerLineFrequency::FREQUENCY_MAX;
}

std::string VideoCaptureParams::SuggestedConstraints::ToString() const {
return base::StrCat(
{"min = ", min_frame_size.ToString(),
", max = ", max_frame_size.ToString(),
", fixed_aspect_ratio = ", fixed_aspect_ratio ? "true" : "false"});
}

VideoCaptureParams::SuggestedConstraints
VideoCaptureParams::SuggestConstraints() const {
// The requested frame size is always the maximum frame size. Ensure that it
Expand Down Expand Up @@ -121,4 +131,10 @@ VideoCaptureParams::SuggestConstraints() const {
resolution_change_policy == ResolutionChangePolicy::FIXED_ASPECT_RATIO};
}

std::ostream& operator<<(
std::ostream& os,
const VideoCaptureParams::SuggestedConstraints& constraints) {
return os << constraints.ToString();
}

} // namespace media
6 changes: 6 additions & 0 deletions media/capture/video_capture_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ struct CAPTURE_EXPORT VideoCaptureParams {
gfx::Size max_frame_size;
bool fixed_aspect_ratio;

std::string ToString() const;

bool operator==(const SuggestedConstraints& other) const {
return min_frame_size == other.min_frame_size &&
max_frame_size == other.max_frame_size &&
Expand Down Expand Up @@ -346,6 +348,10 @@ struct CAPTURE_EXPORT VideoCaptureParams {
bool enable_face_detection;
};

CAPTURE_EXPORT std::ostream& operator<<(
std::ostream& os,
const VideoCaptureParams::SuggestedConstraints& constraints);

} // namespace media

#endif // MEDIA_CAPTURE_VIDEO_CAPTURE_TYPES_H_

0 comments on commit 031a1c9

Please sign in to comment.