zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Reland "Use non-proxied source object in VideoTrack.""

Browse Source 
This reverts commit 1158bff15f33c467543928dd6a49cb6ad04da1ba.

Reason for revert: Downstream issues unresolved (2nd of two reverts)

Original change's description:
> Reland "Use non-proxied source object in VideoTrack."
>
> This is a reland of 3eb29c12358930a60134f185cd849e0d12aa9166
>
> This reland doesn't contain the AudioTrack changes (see original
> description) that got triggered in some cases and needs to be
> addressed separately.
>
> Another change in this re-land is that instead of the `state` property
> of the VideoTrack be marshalled to the signaling thread, it's readable
> from the calling thread. Previously this was marshalled to the worker
> and the original changed that to the signaling thread (same as for
> AudioTrack) - but in case that's causing downstream problems this reland
> uses BYPASS_PROXY_CONSTMETHOD0 for the `state()` accessor of the
> VideoTrack proxy.
>
> Original change's description:
> > Use non-proxied source object in VideoTrack.
> >
> > Use the internal representation of the video source object from the
> > track. Before there were implicit thread hops due to use of the proxy.
> >
> > Also, override AudioTrack's enabled methods to enforce thread
> > expectations.
> >
> > Bug: webrtc:13540
> > Change-Id: I4bc7aca96d6fc24f31ade45e47f52599f1cc2f97
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250180
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#35911}
>
> Bug: webrtc:13540
> Change-Id: Icb3e165f07240ae10730a316d3a8a3b2b9167d82
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251387
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35979}

# Not skipping CQ checks because original CL landed > 1 day ago.

Using "No-Try" to not have to wait for the win chromium bot to unblock
(currently takes hours).

No-Try: true
Bug: webrtc:13540
Change-Id: I8f34536bf472a6d069344e84d889864f195c93f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251686
Reviewed-by: Christoffer Jansson <jansson@google.com>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35993}
This commit is contained in:
Tomas Gunnarsson
2022-02-14 11:03:29 +00:00
committed by WebRTC LUCI CQ
parent 43fb16921b
commit e7b4c137cc
6 changed files with 30 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
pc/BUILD.gn
View File

@ -1242,7 +1242,6 @@ rtc_library("video_track") {
"video_track.h",
]
deps = [
":rtc_pc_base",
"../api:media_stream_interface",
"../api:scoped_refptr",
"../api:sequence_checker",

4
pc/audio_track.h
View File

@ -20,10 +20,6 @@
namespace webrtc {
// TODO(tommi): Instead of inheriting from `MediaStreamTrack<>`, implement the
// properties directly in this class. `MediaStreamTrack` doesn't guard against
// conflicting access, so we'd need to override those methods anyway in this
// class in order to make sure things are correctly checked.
class AudioTrack : public MediaStreamTrack<AudioTrackInterface>,
public ObserverInterface {
protected:

6
pc/media_stream_track_proxy.h
View File

@ -29,12 +29,12 @@ BYPASS_PROXY_CONSTMETHOD0(std::string, kind)
BYPASS_PROXY_CONSTMETHOD0(std::string, id)
PROXY_CONSTMETHOD0(TrackState, state)
PROXY_CONSTMETHOD0(bool, enabled)
PROXY_METHOD1(bool, set_enabled, bool)
BYPASS_PROXY_CONSTMETHOD0(AudioSourceInterface*, GetSource)
PROXY_METHOD1(void, AddSink, AudioTrackSinkInterface*)
PROXY_METHOD1(void, RemoveSink, AudioTrackSinkInterface*)
PROXY_METHOD1(bool, GetSignalLevel, int*)
PROXY_METHOD0(rtc::scoped_refptr<AudioProcessorInterface>, GetAudioProcessor)
PROXY_METHOD1(bool, set_enabled, bool)
PROXY_METHOD1(void, RegisterObserver, ObserverInterface*)
PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*)
END_PROXY_MAP(AudioTrack)
@ -43,9 +43,7 @@ BEGIN_PROXY_MAP(VideoTrack)
PROXY_PRIMARY_THREAD_DESTRUCTOR()
BYPASS_PROXY_CONSTMETHOD0(std::string, kind)
BYPASS_PROXY_CONSTMETHOD0(std::string, id)
// TODO(bugs.webrtc.org/13540): This should probably be PROXY_CONSTMETHOD0, but
// leaving as is due to downstream incompatibility.
BYPASS_PROXY_CONSTMETHOD0(TrackState, state)
PROXY_SECONDARY_CONSTMETHOD0(TrackState, state)
PROXY_SECONDARY_CONSTMETHOD0(bool, enabled)
PROXY_SECONDARY_METHOD1(bool, set_enabled, bool)
PROXY_SECONDARY_CONSTMETHOD0(ContentHint, content_hint)

6
pc/video_rtp_receiver.cc
View File

@ -41,7 +41,11 @@ VideoRtpReceiver::VideoRtpReceiver(
track_(VideoTrackProxyWithInternal<VideoTrack>::Create(
rtc::Thread::Current(),
worker_thread,
VideoTrack::Create(receiver_id, source_, worker_thread))),
VideoTrack::Create(receiver_id,
CreateVideoTrackSourceProxy(rtc::Thread::Current(),
worker_thread,
source_),
worker_thread))),
attachment_id_(GenerateUniqueId()) {
RTC_DCHECK(worker_thread_);
SetStreams(streams);

43
pc/video_track.cc
View File

@ -22,14 +22,12 @@
namespace webrtc {
VideoTrack::VideoTrack(
const std::string& label,
rtc::scoped_refptr<
VideoTrackSourceProxyWithInternal<VideoTrackSourceInterface>> source,
rtc::Thread* worker_thread)
VideoTrack::VideoTrack(const std::string& label,
VideoTrackSourceInterface* video_source,
rtc::Thread* worker_thread)
: MediaStreamTrack<VideoTrackInterface>(label),
worker_thread_(worker_thread),
video_source_(std::move(source)),
video_source_(video_source),
content_hint_(ContentHint::kNone) {
RTC_DCHECK_RUN_ON(&signaling_thread_);
// Detach the thread checker for VideoSourceBaseGuarded since we'll make calls
@ -56,18 +54,18 @@ void VideoTrack::AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
VideoSourceBaseGuarded::AddOrUpdateSink(sink, wants);
rtc::VideoSinkWants modified_wants = wants;
modified_wants.black_frames = !enabled();
video_source_->internal()->AddOrUpdateSink(sink, modified_wants);
video_source_->AddOrUpdateSink(sink, modified_wants);
}
void VideoTrack::RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
RTC_DCHECK_RUN_ON(worker_thread_);
VideoSourceBaseGuarded::RemoveSink(sink);
video_source_->internal()->RemoveSink(sink);
video_source_->RemoveSink(sink);
}
void VideoTrack::RequestRefreshFrame() {
RTC_DCHECK_RUN_ON(worker_thread_);
video_source_->internal()->RequestRefreshFrame();
video_source_->RequestRefreshFrame();
}
VideoTrackSourceInterface* VideoTrack::GetSource() const {
@ -90,11 +88,10 @@ void VideoTrack::set_content_hint(ContentHint hint) {
bool VideoTrack::set_enabled(bool enable) {
RTC_DCHECK_RUN_ON(worker_thread_);
auto* source = video_source_->internal();
for (auto& sink_pair : sink_pairs()) {
rtc::VideoSinkWants modified_wants = sink_pair.wants;
modified_wants.black_frames = !enable;
source->AddOrUpdateSink(sink_pair.sink, modified_wants);
video_source_->AddOrUpdateSink(sink_pair.sink, modified_wants);
}
return MediaStreamTrack<VideoTrackInterface>::set_enabled(enable);
}
@ -105,30 +102,28 @@ bool VideoTrack::enabled() const {
}
MediaStreamTrackInterface::TrackState VideoTrack::state() const {
// TODO(bugs.webrtc.org/13540): Re-enable this DCHECK and update the proxy.
// RTC_DCHECK_RUN_ON(&signaling_thread_);
RTC_DCHECK_RUN_ON(worker_thread_);
return MediaStreamTrack<VideoTrackInterface>::state();
}
void VideoTrack::OnChanged() {
RTC_DCHECK_RUN_ON(&signaling_thread_);
rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
auto* source = video_source_->internal();
auto state = source->state();
set_state(state == MediaSourceInterface::kEnded ? kEnded : kLive);
worker_thread_->Invoke<void>(
RTC_FROM_HERE, [this, state = video_source_->state()]() {
// TODO(tommi): Calling set_state() this way isn't ideal since we're
// currently blocking the signaling thread and set_state() may
// internally fire notifications via `FireOnChanged()` which may further
// amplify the blocking effect on the signaling thread.
rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
set_state(state == MediaSourceInterface::kEnded ? kEnded : kLive);
});
}
rtc::scoped_refptr<VideoTrack> VideoTrack::Create(
const std::string& id,
VideoTrackSourceInterface* source,
rtc::Thread* worker_thread) {
rtc::scoped_refptr<
VideoTrackSourceProxyWithInternal<VideoTrackSourceInterface>>
source_proxy = VideoTrackSourceProxy::Create(rtc::Thread::Current(),
worker_thread, source);
return rtc::make_ref_counted<VideoTrack>(id, std::move(source_proxy),
worker_thread);
return rtc::make_ref_counted<VideoTrack>(id, source, worker_thread);
}
} // namespace webrtc

18
pc/video_track.h
View File

@ -21,16 +21,11 @@
#include "api/video/video_sink_interface.h"
#include "api/video/video_source_interface.h"
#include "media/base/video_source_base.h"
#include "pc/video_track_source_proxy.h"
#include "rtc_base/thread.h"
#include "rtc_base/thread_annotations.h"
namespace webrtc {
// TODO(tommi): Instead of inheriting from `MediaStreamTrack<>`, implement the
// properties directly in this class. `MediaStreamTrack` doesn't guard against
// conflicting access, so we'd need to override those methods anyway in this
// class in order to make sure things are correctly checked.
class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
public rtc::VideoSourceBaseGuarded,
public ObserverInterface {
@ -54,11 +49,9 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
std::string kind() const override;
protected:
VideoTrack(
const std::string& id,
rtc::scoped_refptr<
VideoTrackSourceProxyWithInternal<VideoTrackSourceInterface>> source,
rtc::Thread* worker_thread);
VideoTrack(const std::string& id,
VideoTrackSourceInterface* video_source,
rtc::Thread* worker_thread);
~VideoTrack();
private:
@ -67,10 +60,7 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker signaling_thread_;
rtc::Thread* const worker_thread_;
const rtc::scoped_refptr<
VideoTrackSourceProxyWithInternal<VideoTrackSourceInterface>>
video_source_;
const rtc::scoped_refptr<VideoTrackSourceInterface> video_source_;
ContentHint content_hint_ RTC_GUARDED_BY(worker_thread_);
};