WebRTC Code Reviews
Help | Chromium Project | Sign in
(14)

Issue 46899004: Added RemoteVideoSource

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by hbos
Modified:
4 years, 2 months ago
Reviewers:
magjed, tommi
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, rwolff_gocast.it, yujie.mao, ajm, tterriberry, qiang.lu, Niklas
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Added RemoteVideoSource, a VideoSourceInterface implementation now used (instead of VideoSource) for remote video sources. Because this is a remote video source, it gets its frame through the cricket::VideoRenderer* FrameInput(), and only needs to pass the frame on to its sinks. Unlike VideoSource, this implementation does not rely on a (Remote or otherwise)VideoCapturer for frame delivery to sinks. However, a VideoCapturer is still necessary for further sending frames through peer connection. In order to support this, a RemoteVideoCapturer is still used, but is created lazily the first time RemoteVideoSource::GetVideoCapturer() is called. RemoteVideoSource will pass along FrameInput's frames to the capturer which in turn will signal that frames have been delivered. In Chrome, where the video capturer of a remote video source is never used, this should avoid creating and registering the RemoteVideoCapturer to the CaptureManager altogether. As such, there is no risk of reference counted objects keeping remote capturers alive. The referenced chromium bug is a crash that occurs at ~CaptureManager due to all capturers not having been unregistered, specifically RemoteVideoCapturers. This happens because blink still holds references to reference counted objects despite the peer connection closing and destructing. But in chromium, there should be no need for a RemoteVideoCapturer to exist in the first place. Hence this new video source. (See my post in chromium:320200 for details.) BUG=chromium:320200

Patch Set 1 : #

Total comments: 44

Patch Set 2 : #

Total comments: 1

Messages

Total messages: 6 (1 generated)
hbos
perkj: Here is RemoteVideoSource we talked about. It does pass all the tests, but I'm ...
4 years, 4 months ago (2015-04-14 09:41:41 UTC) #1
tommi
https://webrtc-codereview.appspot.com/46899004/diff/20001/talk/app/webrtc/remotevideosource.cc File talk/app/webrtc/remotevideosource.cc (right): https://webrtc-codereview.appspot.com/46899004/diff/20001/talk/app/webrtc/remotevideosource.cc#newcode3 talk/app/webrtc/remotevideosource.cc:3: * Copyright 2012 Google Inc. fix year here too ...
4 years, 3 months ago (2015-04-23 15:50:58 UTC) #2
tommi
+magjed -perkj
4 years, 3 months ago (2015-04-23 15:51:17 UTC) #4
hbos
PTAL https://webrtc-codereview.appspot.com/46899004/diff/20001/talk/app/webrtc/remotevideosource.cc File talk/app/webrtc/remotevideosource.cc (right): https://webrtc-codereview.appspot.com/46899004/diff/20001/talk/app/webrtc/remotevideosource.cc#newcode3 talk/app/webrtc/remotevideosource.cc:3: * Copyright 2012 Google Inc. On 2015/04/23 15:50:58, ...
4 years, 3 months ago (2015-05-18 14:32:20 UTC) #5
hbos
4 years, 2 months ago (2015-06-12 13:12:19 UTC) #6

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 245c2c2-tainted