New issue
Advanced search Search tips

Issue 645656 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Chromoting: tune encoder parameters for WebRTC protocol

Project Member Reported by sergeyu@chromium.org, Sep 9 2016

Issue description

Currently when using WebRTC-based protocol image quality and performance is significantly worse in some scenarios when compared to the old protocol (e.g. big screen, low bandwidth). Tune encoder parameters and capture scheduler to improve quality.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 13 2016

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

commit 2becb601192f9f6912a982e39e04e3fa0a6a776a
Author: sergeyu <sergeyu@chromium.org>
Date: Tue Sep 13 18:41:32 2016

Add WebrtcVideoEncoder interface

Previous WebrtcVideoEncoderVpx was implementing VideoEncoder interface.
That interface depends on VideoPacket, which is not applicable for
WebRTC case. Added a separate interface that will be used for WebRTC
protocol. The new interface allows to set bandwidth and frame duration
for each frame individually.
Also renamed class previously called WebrtcVideoEncoder to
WebrtcDummyVideoEncoder.

BUG= 645656 

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

[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/client/software_video_renderer_unittest.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/codec_test.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/video_encoder.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/video_encoder_verbatim.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/video_encoder_verbatim.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/video_encoder_vpx.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/video_encoder_vpx.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/video_encoder_vpx_unittest.cc
[add] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/webrtc_video_encoder.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/webrtc_video_encoder_vpx.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/codec/webrtc_video_encoder_vpx.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/proto/video.proto
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/BUILD.gn
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/video_frame_pump.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/video_frame_pump_unittest.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_connection_to_client.cc
[rename] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_dummy_video_encoder.cc
[rename] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_dummy_video_encoder.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_transport.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_transport.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_video_stream.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/protocol/webrtc_video_stream.h
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/test/codec_perftest.cc
[modify] https://crrev.com/2becb601192f9f6912a982e39e04e3fa0a6a776a/remoting/test/test_video_renderer_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 14 2016

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

commit d5ce2225173ec41804750d3de0211408ba096bb9
Author: sergeyu <sergeyu@chromium.org>
Date: Wed Sep 14 01:18:19 2016

Measure bandwidth utilization in the ScrollPerformance test

Now ScrollPerformance also reports network bandwidth utilization.

Also removed two outdated tests: StreamFrameRate and IntermittentChanges.
These tests were intended mainly to evaluate performance of the PseudoTCP
layer and they were using verbatim video codec. They are not applicable
for WebRTC-based protocol. Also the protocol perf tests have been updated
to use SoftwareVideoRenderer which allows to get FrameStats when using
ICE protocol and this approach is not compatible with the old tests.

Bandwidth utilization numbers I get currently with 8Mbps bandwidth:
ICE: 93%
WebRTC: varies from 21 to 25%

The second number also goes to 32 when I change test length and warm-up
time to 5 seconds. This suggests that the problem is partly that the
bandwidth estimate in WebRTC is below the actual bandwidth in the test and
it takes a lot of time to ramp up.

BUG= 645656 

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

[modify] https://crrev.com/d5ce2225173ec41804750d3de0211408ba096bb9/remoting/test/protocol_perftest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20 2016

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

commit 0eaa5139dbab93ed41deefc56633d1ee19f0b832
Author: sergeyu <sergeyu@chromium.org>
Date: Tue Sep 20 01:39:34 2016

Add WebrtcFrameScheduler interface.

WebrtcFrameScheduler is used by WebrtcVideoStream to schedule video
frames. WebrtcFrameScheduler implementations now are also responsible
for top-off logic that previously resided in WebrtcVideoCapturerVpx.
This will allow to switch between different frame scheduling strategies
in runtime.

WebrtcFrameSchedulerSimple implements the old scheduling logic with
the following two exceptions:
 - VPX active map is cleaned only after top-off is finished (top-off
   wasn't always working properly due to this issue).
 - Removed min limit for FPS.

In the future WebrtcFrameSchedulerSimple will need to be replaced
with a new scheduler that allows processing multiple frames in parallel.

BUG= 645656 

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

[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/codec/webrtc_video_encoder.h
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/codec/webrtc_video_encoder_vpx.cc
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/codec/webrtc_video_encoder_vpx.h
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/BUILD.gn
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_dummy_video_encoder.cc
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_dummy_video_encoder.h
[add] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_frame_scheduler.h
[add] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_frame_scheduler_simple.cc
[add] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_frame_scheduler_simple.h
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_video_stream.cc
[modify] https://crrev.com/0eaa5139dbab93ed41deefc56633d1ee19f0b832/remoting/protocol/webrtc_video_stream.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 6 2016

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

commit af3ebce990837650253c9ef7b2b058e6fa7bc575
Author: sergeyu <sergeyu@chromium.org>
Date: Thu Oct 06 01:14:39 2016

Pace outgoing frames in frame scheduler to match target bitrate.

Previously WebrtcFrameSchedulerSimple was scheduling outgoing frames
according to the target bitrate, but it wasn't limiting that bandwidth
precisely, so it was possible for it to send more data than the
network can deliver, which increases bufferbloat. Also now frames are
rescheduled whenever target bitrate changes.

BUG= 645656 

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

[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/base/BUILD.gn
[add] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/base/leaky_bucket.cc
[add] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/base/leaky_bucket.h
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/protocol/webrtc_frame_scheduler_simple.h
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/test/BUILD.gn
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/test/fake_socket_factory.cc
[delete] https://crrev.com/05cbd5d7a01746aad60e6e2350d1d09ce09aa15c/remoting/test/leaky_bucket.cc
[delete] https://crrev.com/05cbd5d7a01746aad60e6e2350d1d09ce09aa15c/remoting/test/leaky_bucket.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 6 2016

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

commit 52f18b0a5d30a2259b79235dc208b1ebd8400ad0
Author: sergeyu <sergeyu@chromium.org>
Date: Thu Oct 06 23:38:52 2016

Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG= 645656 

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

[modify] https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0/remoting/protocol/webrtc_frame_scheduler_simple.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 7 2016

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

commit 2c410d363fdeeaa4551a4ed39526b8c065fea7d3
Author: sergeyu <sergeyu@chromium.org>
Date: Fri Oct 07 15:44:42 2016

Small fixes for WevRTC protocol in host

1. Updated max bitrate to 100Mbps
2. Fixed encoder to update active region for key frames to ensure proper
  top-off

BUG= 645656 

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

[modify] https://crrev.com/2c410d363fdeeaa4551a4ed39526b8c065fea7d3/remoting/codec/webrtc_video_encoder_vpx.cc
[modify] https://crrev.com/2c410d363fdeeaa4551a4ed39526b8c065fea7d3/remoting/protocol/webrtc_transport.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af3ebce990837650253c9ef7b2b058e6fa7bc575

commit af3ebce990837650253c9ef7b2b058e6fa7bc575
Author: sergeyu <sergeyu@chromium.org>
Date: Thu Oct 06 01:14:39 2016

Pace outgoing frames in frame scheduler to match target bitrate.

Previously WebrtcFrameSchedulerSimple was scheduling outgoing frames
according to the target bitrate, but it wasn't limiting that bandwidth
precisely, so it was possible for it to send more data than the
network can deliver, which increases bufferbloat. Also now frames are
rescheduled whenever target bitrate changes.

BUG= 645656 

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

[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/base/BUILD.gn
[add] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/base/leaky_bucket.cc
[add] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/base/leaky_bucket.h
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/protocol/webrtc_frame_scheduler_simple.h
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/test/BUILD.gn
[modify] https://crrev.com/af3ebce990837650253c9ef7b2b058e6fa7bc575/remoting/test/fake_socket_factory.cc
[delete] https://crrev.com/05cbd5d7a01746aad60e6e2350d1d09ce09aa15c/remoting/test/leaky_bucket.cc
[delete] https://crrev.com/05cbd5d7a01746aad60e6e2350d1d09ce09aa15c/remoting/test/leaky_bucket.h

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 27 2016

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

commit 52f18b0a5d30a2259b79235dc208b1ebd8400ad0
Author: sergeyu <sergeyu@chromium.org>
Date: Thu Oct 06 23:38:52 2016

Use high quantizer value for "big" frames after a sequence of "small" frames.

After a sequence of frames that don't saturate bandwidth libvpx always
chooses lowest allowed quntizer (highest quality). As result these frames
are quite big after being encoded, which results in poor response latency
for those frames. With this change the scheduler detects those frames and
sets min_quantizer to 60 to ensure that they are first encoded with low
quality and quality is topped off later.

This change reduces latency for "big" frames in
ProtocolPerfTest.TotalLatencyWebrtc from 500ms to 200ms in 8Mbps case,
even without proper BW estimation.

Also changed frame duration from 66 to 33 ms, to match actual frame
duration (this doesn't seem to have significant effect on quality).

BUG= 645656 

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

[modify] https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0/remoting/protocol/webrtc_frame_scheduler_simple.cc
[modify] https://crrev.com/52f18b0a5d30a2259b79235dc208b1ebd8400ad0/remoting/protocol/webrtc_frame_scheduler_simple.h

Comment 10 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment