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

[remoting host] Use correct encoder after SDP is restarted.

This CL fixes the host process so that the client can successfully
select a different codec after SDP offer/answer exchange is restarted
during an active connection. This comes in 2 parts:

1. The current encoder in WebrtcVideoStream is reset, so that
captured frames are routed to the new encoder.

2. The WebrtcVideoEncoderSelector logic is slightly tweaked so that a
call to SetPreferredCodec() resets the cycle of encoders returned by
repeated calls to CreateEncoder(). This is done so that the correct
encoder is returned after WebrtcVideoStream changes the preferred codec.
This tweak does not break any existing unittests.

This CL fixes crashes which occur when the host tries to use a different
encoder than what WebRTC created via the WebrtcDummyVideoEncoderFactory
class. These crashes can only be triggered when SDP (or ICE) is
restarted via the recently-implemented control messages, which are not
supported in older versions of the host.

Bug: 1113499
Change-Id: Idd1e388252c4359e1edc62ac4a51d71be0ae77cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353672
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@{#797705}
parent b330be01
...@@ -57,6 +57,16 @@ void WebrtcVideoEncoderSelector::SetPreferredCodec(int codec) { ...@@ -57,6 +57,16 @@ void WebrtcVideoEncoderSelector::SetPreferredCodec(int codec) {
DCHECK_GE(codec, 0); DCHECK_GE(codec, 0);
DCHECK_LT(codec, static_cast<int>(encoders_.size())); DCHECK_LT(codec, static_cast<int>(encoders_.size()));
preferred_codec_ = codec; preferred_codec_ = codec;
// Reset so that the next call to CreateEncoder() creates an encoder which
// matches the one negotiated over SDP. Otherwise, repeated calls to
// CreateEncoder() would cycle through every codec that could encode at the
// current resolution, even when they do not match the codec "created" by
// WebRTC via the DummyVideoEncoderFactory.
// TODO(crbug.com/1115789): Review the CreateEncoder() logic to ensure it only
// creates encoders that are compatible with the SDP-selected codec.
last_codec_ = -1;
} }
int WebrtcVideoEncoderSelector::RegisterEncoder( int WebrtcVideoEncoderSelector::RegisterEncoder(
......
...@@ -35,6 +35,8 @@ namespace remoting { ...@@ -35,6 +35,8 @@ namespace remoting {
// 6. If the encoder failed, go back to 3 with the failed DesktopFrame. // 6. If the encoder failed, go back to 3 with the failed DesktopFrame.
// //
// Selector will return nullptr if there is no more codec supporting |profile_|. // Selector will return nullptr if there is no more codec supporting |profile_|.
// Calling SetPreferredCodec() again will reset the cycle, so that the next call
// to CreateEncoder() will start with the codec from SDP.
class WebrtcVideoEncoderSelector final { class WebrtcVideoEncoderSelector final {
public: public:
struct Profile { struct Profile {
......
...@@ -351,6 +351,16 @@ void WebrtcVideoStream::OnEncoderCreated( ...@@ -351,6 +351,16 @@ void WebrtcVideoStream::OnEncoderCreated(
webrtc::VideoCodecType codec_type, webrtc::VideoCodecType codec_type,
const webrtc::SdpVideoFormat::Parameters& parameters) { const webrtc::SdpVideoFormat::Parameters& parameters) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
// Reset the encoder in case a previous one was being used and SDP
// re-negotiation selected a different one. The proper codec will be
// created after the next frame is captured.
// An optimization would be to reset only if the new encoder is different
// from the current one. However, SDP renegotiation is expected to occur
// infrequently (only when the user changes a setting), and should typically
// not cause the same codec to be repeatedly selected.
encoder_.reset();
// The preferred codec id depends on the order of // The preferred codec id depends on the order of
// |encoder_selector_|.RegisterEncoder(). // |encoder_selector_|.RegisterEncoder().
if (codec_type == webrtc::kVideoCodecVP8) { if (codec_type == webrtc::kVideoCodecVP8) {
......
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