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

Reland "Remove stopped_ from AudioRtpReceiver and VideoRtpReceiver."

Browse Source 
This is a reland of 3ed36c0521546881656c73984456485dcab16205

Original change's description:
> Reland "Remove `stopped_` from AudioRtpReceiver and VideoRtpReceiver."
>
> This is a reland of bb57e2d7aa9b36843233d1394422f03d12d9c31f
>
> The difference from the original CL is that a check for
> `state_ == kLive` inside of RemoteAudioSource::AddSink has been removed.
> This caused a side effect that registering the sink while the source
> was in an "initializing" state, failed. The last remaining state
> however, is `kEnded` - but since there's no logic in the class around
> the expected value of the states, the check inside of AddSink()
> doesn't provide an additional value - it's rather a surprise for
> developers if it doesn't succeed. So, now removed.
>
> Original change's description:
> > Remove `stopped_` from AudioRtpReceiver and VideoRtpReceiver.
> >
> > This simplifies the logic in these classes a bit, which makes upcoming
> > change easier. The `stopped_` flag in these classes was essentially
> > the same thing as `media_channel_ == nullptr`, which is what's
> > consistently used now for the same checks.
> >
> > Bug: webrtc:13540
> > Change-Id: Ib60bfad9f28d5ddee8a8d5170c3f2a7ef017a5ca
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250163
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#35907}
>
> Bug: webrtc:13540
> Change-Id: I3e5b3046fae11cb56b50c38c5f08972a6f283dd5
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251326
> Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35958}

Bug: webrtc:13540
Change-Id: I6d7d67fddb1ddfc69a302f0f69a9b815f2fd82f7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251386
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35967}
This commit is contained in:
Tommi
2022-02-08 21:12:15 +01:00
committed by WebRTC LUCI CQ
parent 981c572eab
commit 20d8d9150c
10 changed files with 44 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
pc/BUILD.gn
View File

@ -1416,6 +1416,7 @@ rtc_library("video_track_source") {
"../media:rtc_media_base",
"../rtc_base:checks",
"../rtc_base:rtc_base_approved",
"../rtc_base/system:no_unique_address",
"../rtc_base/system:rtc_export",
]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]

48
pc/audio_rtp_receiver.cc
View File

@ -61,7 +61,6 @@ AudioRtpReceiver::AudioRtpReceiver(
AudioRtpReceiver::~AudioRtpReceiver() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
RTC_DCHECK(stopped_);
RTC_DCHECK(!media_channel_);
track_->GetSource()->UnregisterAudioObserver(this);
@ -85,6 +84,10 @@ void AudioRtpReceiver::OnChanged() {
void AudioRtpReceiver::SetOutputVolume_w(double volume) {
RTC_DCHECK_GE(volume, 0.0);
RTC_DCHECK_LE(volume, 10.0);
if (!media_channel_)
return;
ssrc_ ? media_channel_->SetOutputVolume(*ssrc_, volume)
: media_channel_->SetDefaultOutputVolume(volume);
}
@ -94,13 +97,10 @@ void AudioRtpReceiver::OnSetVolume(double volume) {
RTC_DCHECK_GE(volume, 0);
RTC_DCHECK_LE(volume, 10);
// Update the cached_volume_ even when stopped_, to allow clients to set the
// Update the cached_volume_ even when stopped, to allow clients to set the
// volume before starting/restarting, eg see crbug.com/1272566.
cached_volume_ = volume;
if (stopped_)
return;
// When the track is disabled, the volume of the source, which is the
// corresponding WebRtc Voice Engine channel will be 0. So we do not allow
// setting the volume to the source when the track is disabled.
@ -160,10 +160,7 @@ AudioRtpReceiver::GetFrameDecryptor() const {
void AudioRtpReceiver::Stop() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
// TODO(deadbeef): Need to do more here to fully stop receiving packets.
if (!stopped_) {
source_->SetState(MediaSourceInterface::kEnded);
stopped_ = true;
}
source_->SetState(MediaSourceInterface::kEnded);
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
RTC_DCHECK_RUN_ON(worker_thread_);
@ -183,22 +180,17 @@ void AudioRtpReceiver::StopAndEndTrack() {
void AudioRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
bool ok = worker_thread_->Invoke<bool>(
RTC_FROM_HERE, [&, enabled = cached_track_enabled_,
volume = cached_volume_, was_stopped = stopped_]() {
MediaSourceInterface::SourceState state = source_->state();
worker_thread_->Invoke<void>(
RTC_FROM_HERE,
[&, enabled = cached_track_enabled_, volume = cached_volume_]() {
RTC_DCHECK_RUN_ON(worker_thread_);
if (!media_channel_) {
RTC_DCHECK(was_stopped);
return false; // Can't restart.
}
if (!media_channel_)
return; // Can't restart.
if (!was_stopped && ssrc_ == ssrc) {
// Already running with that ssrc.
RTC_DCHECK(worker_thread_safety_->alive());
return true;
}
if (!was_stopped) {
if (state != MediaSourceInterface::kInitializing) {
if (ssrc_ == ssrc)
return;
source_->Stop(media_channel_, ssrc_);
}
@ -209,13 +201,8 @@ void AudioRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
}
Reconfigure(enabled, volume);
return true;
});
if (!ok)
return;
stopped_ = false;
source_->SetState(MediaSourceInterface::kLive);
}
void AudioRtpReceiver::SetupMediaChannel(uint32_t ssrc) {
@ -335,9 +322,6 @@ void AudioRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) {
RTC_DCHECK(media_channel == nullptr ||
media_channel->media_type() == media_type());
if (stopped_ && !media_channel)
return;
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread_);
SetMediaChannel_w(media_channel);

1
pc/audio_rtp_receiver.h
View File

@ -133,7 +133,6 @@ class AudioRtpReceiver : public ObserverInterface,
RTC_GUARDED_BY(&signaling_thread_checker_);
bool cached_track_enabled_ RTC_GUARDED_BY(&signaling_thread_checker_);
double cached_volume_ RTC_GUARDED_BY(&signaling_thread_checker_) = 1.0;
bool stopped_ RTC_GUARDED_BY(&signaling_thread_checker_) = true;
RtpReceiverObserverInterface* observer_
RTC_GUARDED_BY(&signaling_thread_checker_) = nullptr;
bool received_first_packet_ RTC_GUARDED_BY(&signaling_thread_checker_) =

1
pc/audio_rtp_receiver_unittest.cc
View File

@ -63,6 +63,7 @@ TEST_F(AudioRtpReceiverTest, SetOutputVolumeIsCalled) {
receiver_->track();
receiver_->track()->set_enabled(true);
receiver_->SetMediaChannel(&media_channel_);
EXPECT_CALL(media_channel_, SetDefaultRawAudioSink(_)).Times(0);
receiver_->SetupMediaChannel(kSsrc);
EXPECT_CALL(media_channel_, SetOutputVolume(kSsrc, kVolume))

7
pc/remote_audio_source.cc
View File

@ -55,7 +55,7 @@ RemoteAudioSource::RemoteAudioSource(
: main_thread_(rtc::Thread::Current()),
worker_thread_(worker_thread),
on_audio_channel_gone_action_(on_audio_channel_gone_action),
state_(MediaSourceInterface::kLive) {
state_(MediaSourceInterface::kInitializing) {
RTC_DCHECK(main_thread_);
RTC_DCHECK(worker_thread_);
}
@ -134,11 +134,6 @@ void RemoteAudioSource::AddSink(AudioTrackSinkInterface* sink) {
RTC_DCHECK_RUN_ON(main_thread_);
RTC_DCHECK(sink);
if (state_ != MediaSourceInterface::kLive) {
RTC_LOG(LS_ERROR) << "Can't register sink as the source isn't live.";
return;
}
MutexLock lock(&sink_lock_);
RTC_DCHECK(!absl::c_linear_search(sinks_, sink));
sinks_.push_back(sink);

47
pc/video_rtp_receiver.cc
View File

@ -49,12 +49,11 @@ VideoRtpReceiver::VideoRtpReceiver(
attachment_id_(GenerateUniqueId()) {
RTC_DCHECK(worker_thread_);
SetStreams(streams);
RTC_DCHECK_EQ(source_->state(), MediaSourceInterface::kLive);
RTC_DCHECK_EQ(source_->state(), MediaSourceInterface::kInitializing);
}
VideoRtpReceiver::~VideoRtpReceiver() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
RTC_DCHECK(stopped_);
RTC_DCHECK(!media_channel_);
}
@ -116,10 +115,7 @@ void VideoRtpReceiver::Stop() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
// TODO(deadbeef): Need to do more here to fully stop receiving packets.
if (!stopped_) {
source_->SetState(MediaSourceInterface::kEnded);
stopped_ = true;
}
source_->SetState(MediaSourceInterface::kEnded);
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread_);
@ -140,33 +136,29 @@ void VideoRtpReceiver::StopAndEndTrack() {
void VideoRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
// `stopped_` will be `true` on construction. RestartMediaChannel
// can in this case function like "ensure started" and flip `stopped_`
// to false.
MediaSourceInterface::SourceState state = source_->state();
// TODO(tommi): Can we restart the media channel without blocking?
bool ok = worker_thread_->Invoke<bool>(RTC_FROM_HERE, [&, was_stopped =
stopped_] {
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread_);
if (!media_channel_) {
// Ignore further negotiations if we've already been stopped and don't
// have an associated media channel.
RTC_DCHECK(was_stopped);
return false; // Can't restart.
return; // Can't restart.
}
if (!was_stopped && ssrc_ == ssrc) {
// Already running with that ssrc.
return true;
}
const bool encoded_sink_enabled = saved_encoded_sink_enabled_;
// Disconnect from the previous ssrc.
if (!was_stopped) {
if (state != MediaSourceInterface::kInitializing) {
if (ssrc == ssrc_)
return;
// Disconnect from a previous ssrc.
SetSink(nullptr);
}
bool encoded_sink_enabled = saved_encoded_sink_enabled_;
SetEncodedSinkEnabled(false);
if (encoded_sink_enabled)
SetEncodedSinkEnabled(false);
}
// Set up the new ssrc.
ssrc_ = std::move(ssrc);
@ -187,14 +179,8 @@ void VideoRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
media_channel_->SetBaseMinimumPlayoutDelayMs(*ssrc_, delay_.GetMs());
}
return true;
});
if (!ok)
return;
stopped_ = false;
source_->SetState(MediaSourceInterface::kLive);
}
// RTC_RUN_ON(worker_thread_)
@ -288,9 +274,6 @@ void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) {
RTC_DCHECK(media_channel == nullptr ||
media_channel->media_type() == media_type());
if (stopped_ && !media_channel)
return;
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread_);
SetMediaChannel_w(media_channel);

11
pc/video_rtp_receiver.h
View File

@ -141,8 +141,6 @@ class VideoRtpReceiver : public RtpReceiverInternal {
rtc::Thread* const worker_thread_;
const std::string id_;
// See documentation for `stopped_` below for when a valid media channel
// has been assigned and when this pointer will be null.
cricket::VideoMediaChannel* media_channel_ RTC_GUARDED_BY(worker_thread_) =
nullptr;
absl::optional<uint32_t> ssrc_ RTC_GUARDED_BY(worker_thread_);
@ -152,15 +150,6 @@ class VideoRtpReceiver : public RtpReceiverInternal {
const rtc::scoped_refptr<VideoTrackProxyWithInternal<VideoTrack>> track_;
std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams_
RTC_GUARDED_BY(&signaling_thread_checker_);
// `stopped` is state that's used on the signaling thread to indicate whether
// a valid `media_channel_` has been assigned and configured. When an instance
// of VideoRtpReceiver is initially created, `stopped_` is true and will
// remain true until either `SetupMediaChannel` or
// `SetupUnsignaledMediaChannel` is called after assigning a media channel.
// After that, `stopped_` will remain false until `Stop()` is called.
// Note, for checking the state of the class on the worker thread,
// check `media_channel_` instead, as that's the main worker thread state.
bool stopped_ RTC_GUARDED_BY(&signaling_thread_checker_) = true;
RtpReceiverObserverInterface* observer_
RTC_GUARDED_BY(&signaling_thread_checker_) = nullptr;
bool received_first_packet_ RTC_GUARDED_BY(&signaling_thread_checker_) =

1
pc/video_rtp_receiver_unittest.cc
View File

@ -169,7 +169,6 @@ TEST_F(VideoRtpReceiverTest, BroadcastsEncodedFramesWhenEnabled) {
TEST_F(VideoRtpReceiverTest, EnablesEncodedOutputOnChannelRestart) {
InSequence s;
EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback(0));
MockVideoSink sink;
Source()->AddEncodedSink(&sink);
EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback(4711, _));

3
pc/video_track_source.cc
View File

@ -15,11 +15,12 @@
namespace webrtc {
VideoTrackSource::VideoTrackSource(bool remote)
: state_(kLive), remote_(remote) {
: state_(kInitializing), remote_(remote) {
worker_thread_checker_.Detach();
}
void VideoTrackSource::SetState(SourceState new_state) {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
if (state_ != new_state) {
state_ = new_state;
FireOnChanged();

11
pc/video_track_source.h
View File

@ -20,6 +20,7 @@
#include "api/video/video_sink_interface.h"
#include "api/video/video_source_interface.h"
#include "media/base/media_channel.h"
#include "rtc_base/system/no_unique_address.h"
#include "rtc_base/system/rtc_export.h"
namespace webrtc {
@ -31,7 +32,10 @@ class RTC_EXPORT VideoTrackSource : public Notifier<VideoTrackSourceInterface> {
explicit VideoTrackSource(bool remote);
void SetState(SourceState new_state);
SourceState state() const override { return state_; }
SourceState state() const override {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
return state_;
}
bool remote() const override { return remote_; }
bool is_screencast() const override { return false; }
@ -56,8 +60,9 @@ class RTC_EXPORT VideoTrackSource : public Notifier<VideoTrackSourceInterface> {
virtual rtc::VideoSourceInterface<VideoFrame>* source() = 0;
private:
SequenceChecker worker_thread_checker_;
SourceState state_;
RTC_NO_UNIQUE_ADDRESS SequenceChecker worker_thread_checker_;
RTC_NO_UNIQUE_ADDRESS SequenceChecker signaling_thread_checker_;
SourceState state_ RTC_GUARDED_BY(&signaling_thread_checker_);
const bool remote_;
};