Commit ae177cd5 authored by Lambros Lambrou's avatar Lambros Lambrou Committed by Commit Bot

[remoting host] Remove VideoControl lossless-color implementation.

This CL prevents WebRTC clients from changing the VP9 lossless-color
property via a VideoControl message. This is because WebRTC handles
that setting via the SDP codec parameter "profile-id=1", which means
an SDP offer/answer exchange is needed to properly control this
setting. Non-WebRTC clients are not affected and they can still toggle
lossless-color via a VideoControl message.

This CL also adds logging to the VP9 encoder so we can verify what
settings are being applied at the codec level.

This also fixes a bug where the SDP parameter for lossless-color was
not being respected.

Bug: 1113499
Change-Id: I15ca83b0fcc157a3b63d3a3ca3509039ed47b24f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342126
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: default avatarJamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795973}
parent 8e2d31cb
...@@ -416,6 +416,13 @@ void WebrtcVideoEncoderVpx::Configure(const webrtc::DesktopSize& size) { ...@@ -416,6 +416,13 @@ void WebrtcVideoEncoderVpx::Configure(const webrtc::DesktopSize& size) {
DCHECK(use_vp9_ || !lossless_color_); DCHECK(use_vp9_ || !lossless_color_);
DCHECK(use_vp9_ || !lossless_encode_); DCHECK(use_vp9_ || !lossless_encode_);
if (use_vp9_) {
VLOG(0) << "Configuring VP9 encoder with lossless-color="
<< (lossless_color_ ? "true" : "false")
<< ", lossless-encode=" << (lossless_encode_ ? "true" : "false")
<< ".";
}
// Tear down |image_| if it no longer matches the size and color settings. // Tear down |image_| if it no longer matches the size and color settings.
// PrepareImage() will then create a new buffer of the required dimensions if // PrepareImage() will then create a new buffer of the required dimensions if
// |image_| is not allocated. // |image_| is not allocated.
......
...@@ -190,10 +190,8 @@ void WebrtcVideoStream::SetLosslessEncode(bool want_lossless) { ...@@ -190,10 +190,8 @@ void WebrtcVideoStream::SetLosslessEncode(bool want_lossless) {
} }
void WebrtcVideoStream::SetLosslessColor(bool want_lossless) { void WebrtcVideoStream::SetLosslessColor(bool want_lossless) {
lossless_color_ = want_lossless; NOTIMPLEMENTED() << "Changing lossless-color for VP9 requires SDP "
if (encoder_) { "offer/answer exchange.";
encoder_->SetLosslessColor(want_lossless);
}
} }
void WebrtcVideoStream::SetObserver(Observer* observer) { void WebrtcVideoStream::SetObserver(Observer* observer) {
...@@ -233,7 +231,6 @@ void WebrtcVideoStream::OnCaptureResult( ...@@ -233,7 +231,6 @@ void WebrtcVideoStream::OnCaptureResult(
encoder_selector_.SetDesktopFrame(*frame); encoder_selector_.SetDesktopFrame(*frame);
encoder_ = encoder_selector_.CreateEncoder(); encoder_ = encoder_selector_.CreateEncoder();
encoder_->SetLosslessEncode(lossless_encode_); encoder_->SetLosslessEncode(lossless_encode_);
encoder_->SetLosslessColor(lossless_color_);
// TODO(zijiehe): Permanently stop the video stream if we cannot create an // TODO(zijiehe): Permanently stop the video stream if we cannot create an
// encoder for the |frame|. // encoder for the |frame|.
......
...@@ -113,8 +113,11 @@ class WebrtcVideoStream : public VideoStream, ...@@ -113,8 +113,11 @@ class WebrtcVideoStream : public VideoStream,
// Settings that are received from video-control messages. These are stored // Settings that are received from video-control messages. These are stored
// here in case a message is received before the encoder is created. // here in case a message is received before the encoder is created.
// Lossless-color is not stored here because, for VP9 encoder, WebRTC handles
// it via an SDP codec parameter, "profile-id=1". Changing it requires a
// new SDP offer/answer exchange, and therefore it cannot be changed directly
// via video-control message.
bool lossless_encode_ = false; bool lossless_encode_ = false;
bool lossless_color_ = false;
base::WeakPtrFactory<WebrtcVideoStream> weak_factory_{this}; base::WeakPtrFactory<WebrtcVideoStream> weak_factory_{this};
......
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