Adapt media/cast technologies into Chromoting host |
||||
Issue descriptionWe are using very similar implementations in both cast and chromoting. So adapting media/cast implementations is a workable solution to reduce the complexity / maintenance cost of chromoting and also improve the quality. After some initial investigations, two components can be used by chromoting. 1. VP8 encoder wrapper. - VP8 encoder is a very low-level implementation, it needs various tunes to work well. Currently we have implemented our own wrapper called WebrtcVideoEncoderVpx, which is less optimized than media::cast::Vp8Encoder. We can at least share the implementation between cast and chromoting. 2. congestion control - The inaccuracy of bandwidth estimates from webrtc is a long term headache of chromoting, and it also significantly impacts the overall performance and end-user experience. media/cast implements a standalone component (AdaptiveCongestionControl) to measure and estimate the bandwidth, which requires very few inputs. We can use it to help us estimate the bandwidth without fully relying on webrtc.
,
Nov 13 2017
Thank you for the comments Sergey. AdaptiveCongestionControl is the name of the component, it does not really "control" the "congestion". Indeed it's a simple component to calculate the bitrate according to the statistic of frames. So, - Yes, it may not work well on public network, but we can have a try. - Lack of per-frame ACKs is a problem. It looks like we can record frame size and use OnTargetBitrateChanged() to simulate the behavior. - AdaptiveCongestionControl impacts only the bitrate of the encoder, it does not control the data pipeline. This is an experimental feature I would like to try. I will update this bug once we have some analysis results.
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e2abda24be0e7d9d0290fab20ee2bed5d9b00070 commit e2abda24be0e7d9d0290fab20ee2bed5d9b00070 Author: Zijie He <zijiehe@chromium.org> Date: Fri Nov 17 02:57:21 2017 [Chromoting] Implement CastSoftwareVideoEncoderAdapter to use cast encoders in Chromoting CastSoftwareVideoEncoderAdapter implements WebrtcVideoEncoder by using a media::cast::SoftwareVideoEncoder. This implementation is now only buildable, we still need several changes to make it work. E.g. converting the DesktopFrame data from ARGB to I420. I am working on a WebRTC change for it. Currently this kind of conversions have been implemented several times across Chromium. Bug: chromium:781432 Change-Id: Ia259bc0478957d2cef492cbdb418202bfda72a4b Reviewed-on: https://chromium-review.googlesource.com/754060 Reviewed-by: Xiangjun Zhang <xjz@chromium.org> Reviewed-by: Yuri Wiitala <miu@chromium.org> Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org> Commit-Queue: Zijie He <zijiehe@chromium.org> Cr-Commit-Position: refs/heads/master@{#517261} [modify] https://crrev.com/e2abda24be0e7d9d0290fab20ee2bed5d9b00070/remoting/codec/BUILD.gn [modify] https://crrev.com/e2abda24be0e7d9d0290fab20ee2bed5d9b00070/remoting/codec/DEPS [add] https://crrev.com/e2abda24be0e7d9d0290fab20ee2bed5d9b00070/remoting/codec/cast_software_video_encoder_adapter.cc [add] https://crrev.com/e2abda24be0e7d9d0290fab20ee2bed5d9b00070/remoting/codec/cast_software_video_encoder_adapter.h
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/44ad510c97f91178e7528c74e1373312d143f5aa commit 44ad510c97f91178e7528c74e1373312d143f5aa Author: Zijie He <zijiehe@chromium.org> Date: Fri Nov 17 20:23:57 2017 [Chromoting] Split BandwidthEstimator logic out of WebrtcFrameSchedulerSimpler This change adds a BandwidthEstimator interface and use it in WebrtcFrameSchedulerSimpler, so we can use different implementations to evaluate the bandwidth without fully relying on WebRTC. Currently the implementation is still WebrtcBandwidthEstimator; this change should have no logic impact. Bug: chromium:781432 Change-Id: I9f910e2d5e5c441d7f324c549e77e1786e5ed046 Reviewed-on: https://chromium-review.googlesource.com/776009 Commit-Queue: Zijie He <zijiehe@chromium.org> Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org> Cr-Commit-Position: refs/heads/master@{#517536} [modify] https://crrev.com/44ad510c97f91178e7528c74e1373312d143f5aa/remoting/protocol/BUILD.gn [add] https://crrev.com/44ad510c97f91178e7528c74e1373312d143f5aa/remoting/protocol/bandwidth_estimator.h [add] https://crrev.com/44ad510c97f91178e7528c74e1373312d143f5aa/remoting/protocol/webrtc_bandwidth_estimator.cc [add] https://crrev.com/44ad510c97f91178e7528c74e1373312d143f5aa/remoting/protocol/webrtc_bandwidth_estimator.h [modify] https://crrev.com/44ad510c97f91178e7528c74e1373312d143f5aa/remoting/protocol/webrtc_frame_scheduler_simple.cc [modify] https://crrev.com/44ad510c97f91178e7528c74e1373312d143f5aa/remoting/protocol/webrtc_frame_scheduler_simple.h
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c18adab8242a77388f9c6e934d3aec8fde0dd8f8 commit c18adab8242a77388f9c6e934d3aec8fde0dd8f8 Author: Zijie He <zijiehe@chromium.org> Date: Fri Nov 17 21:08:47 2017 [Chromoting] Rename HostSessionOptions to SessionOptions HostSessionOptions is under host folder, it cannot be referred by components within protocol / codec folders. Bug: chromium:781432 Change-Id: I23fe466b8ee60f1761797507d2ae9131a9752c56 Reviewed-on: https://chromium-review.googlesource.com/776197 Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org> Commit-Queue: Zijie He <zijiehe@chromium.org> Cr-Commit-Position: refs/heads/master@{#517551} [modify] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/base/BUILD.gn [rename] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/base/session_options.cc [rename] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/base/session_options.h [rename] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/base/session_options_unittest.cc [modify] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/host/BUILD.gn [modify] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/host/client_session.cc [modify] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/host/desktop_environment_options.cc [modify] https://crrev.com/c18adab8242a77388f9c6e934d3aec8fde0dd8f8/remoting/host/desktop_environment_options.h
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba571517b953fced57a37407845fa50cbcafdfd6 commit ba571517b953fced57a37407845fa50cbcafdfd6 Author: Zijie He <zijiehe@chromium.org> Date: Mon Nov 20 20:22:23 2017 [Chromoting] Use SessionOptions instead of SetPreferredVideoCodec After cl 776197, SessionOptions can be referred by protocol/ and codec/, so we can use it to forward the session options instead of using pure string for video codec selection. This change should have no behavior impact. Bug: chromium:781432 Change-Id: Id7b87d460a4789dead75806f6f33d731cf8ddada Reviewed-on: https://chromium-review.googlesource.com/777776 Commit-Queue: Zijie He <zijiehe@chromium.org> Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org> Cr-Commit-Position: refs/heads/master@{#517908} [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/base/session_options.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/base/session_options.h [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/host/client_session.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/connection_to_client.h [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_connection_to_client.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_connection_to_client.h [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_frame_scheduler_simple.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_frame_scheduler_simple.h [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_frame_scheduler_unittest.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_transport.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_transport.h [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_video_stream.cc [modify] https://crrev.com/ba571517b953fced57a37407845fa50cbcafdfd6/remoting/protocol/webrtc_video_stream.h
,
Dec 11 2017
Some work has been done. But we need change https://webrtc-review.googlesource.com/c/src/+/21103 to use CastSoftwareVideoEncoderAdapter (https://cs.chromium.org/chromium/src/remoting/codec/cast_software_video_encoder_adapter.h?type=cs&q=castsoftware&l=30). It's definitely worthy to use CastSoftwareVideoEncoderAdapter as well as VpxImage to avoid 800+ lines of duplication. Using AdaptiveConjestionControl is way more complex, WebRTC uses NACK instead of ACK.
,
Dec 11 2017
,
Dec 14 2017
I don't think this is important enough to prioritize it right now.
,
Dec 14 2017
The reason I opened this bug is because the performance gap of encoding between cast and chromoting that Mark mentioned in another mail thread. Mark believes that HD@60fps can be achieved by only one core in cast. But according to my experience, this is definitely unachievable in Chromoting. The problem may be because of the configurations we are using in VP8 encoder wrapper. But controlling VP8 is more complex than setting several configurations, so adapting VP8 encoder wrapper looks like a simple and effective solution. |
||||
►
Sign in to add a comment |
||||
Comment 1 by sergeyu@google.com
, Nov 11 2017