Revert of Storing raw audio sink for default audio track. (patchset #7 id:120001 of https://codereview.chromium.org/1551813002/ )
Reason for revert:
tommi pointed out that using a refptr for the sink may cause issues. Will reland with a slightly different approach.
Original issue's description:
> Storing raw audio sink for default audio track.
>
> BUG=webrtc:5250
>
> Committed: https://crrev.com/e591f9377f33f3f725a30faecd1bef1a71fa6b99
> Cr-Commit-Position: refs/heads/master@{#11230}
TBR=pthatcher@webrtc.org,solenberg@webrtc.org,pbos@webrtc.org,tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5250
Review URL: https://codereview.webrtc.org/1588693002
Cr-Commit-Position: refs/heads/master@{#11241}
diff --git a/talk/app/webrtc/mediastreamprovider.h b/talk/app/webrtc/mediastreamprovider.h
index 2a82656..585d51b 100644
--- a/talk/app/webrtc/mediastreamprovider.h
+++ b/talk/app/webrtc/mediastreamprovider.h
@@ -29,7 +29,7 @@
#define TALK_APP_WEBRTC_MEDIASTREAMPROVIDER_H_
#include "webrtc/base/basictypes.h"
-#include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/base/scoped_ptr.h"
namespace cricket {
@@ -75,7 +75,7 @@
// passed to the provider.
virtual void SetRawAudioSink(
uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) = 0;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) = 0;
protected:
virtual ~AudioProviderInterface() {}
diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc
index 205ab5c..c3789b7 100644
--- a/talk/app/webrtc/peerconnectioninterface_unittest.cc
+++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc
@@ -1989,12 +1989,8 @@
ASSERT_EQ(1u, observer_.remote_streams()->count());
ASSERT_EQ(1u, remote_stream->GetAudioTracks().size());
EXPECT_EQ("defaulta0", remote_stream->GetAudioTracks()[0]->id());
- EXPECT_EQ(MediaStreamTrackInterface::kLive,
- remote_stream->GetAudioTracks()[0]->state());
ASSERT_EQ(1u, remote_stream->GetVideoTracks().size());
EXPECT_EQ("defaultv0", remote_stream->GetVideoTracks()[0]->id());
- EXPECT_EQ(MediaStreamTrackInterface::kLive,
- remote_stream->GetVideoTracks()[0]->state());
}
// This tests that a default MediaStream is created if a remote session
diff --git a/talk/app/webrtc/remoteaudiosource.cc b/talk/app/webrtc/remoteaudiosource.cc
index 45e2e9e..e904dd9 100644
--- a/talk/app/webrtc/remoteaudiosource.cc
+++ b/talk/app/webrtc/remoteaudiosource.cc
@@ -96,7 +96,8 @@
// To make sure we always get notified when the provider goes out of scope,
// we register for callbacks here and not on demand in AddSink.
if (provider) { // May be null in tests.
- provider->SetRawAudioSink(ssrc, new rtc::RefCountedObject<Sink>(this));
+ provider->SetRawAudioSink(
+ ssrc, rtc::scoped_ptr<AudioSinkInterface>(new Sink(this)));
}
}
diff --git a/talk/app/webrtc/rtpsenderreceiver_unittest.cc b/talk/app/webrtc/rtpsenderreceiver_unittest.cc
index eceed19..a590e1d 100644
--- a/talk/app/webrtc/rtpsenderreceiver_unittest.cc
+++ b/talk/app/webrtc/rtpsenderreceiver_unittest.cc
@@ -70,14 +70,13 @@
cricket::AudioRenderer* renderer));
MOCK_METHOD2(SetAudioPlayoutVolume, void(uint32_t ssrc, double volume));
- void SetRawAudioSink(
- uint32_t,
- const rtc::scoped_refptr<AudioSinkInterface>& sink) override {
- sink_ = sink;
+ void SetRawAudioSink(uint32_t,
+ rtc::scoped_ptr<AudioSinkInterface> sink) override {
+ sink_ = std::move(sink);
}
private:
- rtc::scoped_refptr<AudioSinkInterface> sink_;
+ rtc::scoped_ptr<AudioSinkInterface> sink_;
};
// Helper class to test RtpSender/RtpReceiver.
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 41e22da..d8f7637 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -1328,14 +1328,13 @@
}
}
-void WebRtcSession::SetRawAudioSink(
- uint32_t ssrc,
- const rtc::scoped_refptr<AudioSinkInterface>& sink) {
+void WebRtcSession::SetRawAudioSink(uint32_t ssrc,
+ rtc::scoped_ptr<AudioSinkInterface> sink) {
ASSERT(signaling_thread()->IsCurrent());
if (!voice_channel_)
return;
- voice_channel_->SetRawAudioSink(ssrc, sink);
+ voice_channel_->SetRawAudioSink(ssrc, std::move(sink));
}
bool WebRtcSession::SetCaptureDevice(uint32_t ssrc,
diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h
index efb280f..b79e0ec 100644
--- a/talk/app/webrtc/webrtcsession.h
+++ b/talk/app/webrtc/webrtcsession.h
@@ -254,9 +254,8 @@
const cricket::AudioOptions& options,
cricket::AudioRenderer* renderer) override;
void SetAudioPlayoutVolume(uint32_t ssrc, double volume) override;
- void SetRawAudioSink(
- uint32_t ssrc,
- const rtc::scoped_refptr<AudioSinkInterface>& sink) override;
+ void SetRawAudioSink(uint32_t ssrc,
+ rtc::scoped_ptr<AudioSinkInterface> sink) override;
// Implements VideoMediaProviderInterface.
bool SetCaptureDevice(uint32_t ssrc, cricket::VideoCapturer* camera) override;
diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h
index 583d5f1..149704f 100644
--- a/talk/media/base/fakemediaengine.h
+++ b/talk/media/base/fakemediaengine.h
@@ -349,8 +349,8 @@
virtual void SetRawAudioSink(
uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) {
- sink_ = sink;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
+ sink_ = std::move(sink);
}
private:
@@ -425,7 +425,7 @@
int time_since_last_typing_;
AudioOptions options_;
std::map<uint32_t, VoiceChannelAudioSink*> local_renderers_;
- rtc::scoped_refptr<webrtc::AudioSinkInterface> sink_;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink_;
};
// A helper function to compare the FakeVoiceMediaChannel::DtmfInfo.
diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h
index 130801b..f6fb77d 100644
--- a/talk/media/base/mediachannel.h
+++ b/talk/media/base/mediachannel.h
@@ -1037,7 +1037,7 @@
virtual void SetRawAudioSink(
uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) = 0;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) = 0;
};
struct VideoSendParameters : RtpSendParameters<VideoCodec, VideoOptions> {
diff --git a/talk/media/webrtc/fakewebrtccall.cc b/talk/media/webrtc/fakewebrtccall.cc
index 990ac51..d50a53c 100644
--- a/talk/media/webrtc/fakewebrtccall.cc
+++ b/talk/media/webrtc/fakewebrtccall.cc
@@ -93,8 +93,8 @@
}
void FakeAudioReceiveStream::SetSink(
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) {
- sink_ = sink;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
+ sink_ = std::move(sink);
}
FakeVideoSendStream::FakeVideoSendStream(
diff --git a/talk/media/webrtc/fakewebrtccall.h b/talk/media/webrtc/fakewebrtccall.h
index ca08c8c..3528c7a 100644
--- a/talk/media/webrtc/fakewebrtccall.h
+++ b/talk/media/webrtc/fakewebrtccall.h
@@ -89,9 +89,6 @@
void SetStats(const webrtc::AudioReceiveStream::Stats& stats);
int received_packets() const { return received_packets_; }
void IncrementReceivedPackets();
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink() const {
- return sink_;
- }
private:
// webrtc::ReceiveStream implementation.
@@ -109,13 +106,12 @@
// webrtc::AudioReceiveStream implementation.
webrtc::AudioReceiveStream::Stats GetStats() const override;
- void SetSink(
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) override;
+ void SetSink(rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) override;
webrtc::AudioReceiveStream::Config config_;
webrtc::AudioReceiveStream::Stats stats_;
int received_packets_;
- rtc::scoped_refptr<webrtc::AudioSinkInterface> sink_;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink_;
};
class FakeVideoSendStream final : public webrtc::VideoSendStream,
diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc
index 962cb54..9742564 100644
--- a/talk/media/webrtc/webrtcvoiceengine.cc
+++ b/talk/media/webrtc/webrtcvoiceengine.cc
@@ -1244,10 +1244,9 @@
return config_.voe_channel_id;
}
- void SetRawAudioSink(
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) {
+ void SetRawAudioSink(rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
- stream_->SetSink(sink);
+ stream_->SetSink(std::move(sink));
}
private:
@@ -2187,7 +2186,6 @@
}
default_recv_ssrc_ = ssrc;
SetOutputVolume(default_recv_ssrc_, default_recv_volume_);
- SetRawAudioSink(default_recv_ssrc_, default_sink_);
}
// Forward packet to Call. If the SSRC is unknown we'll return after this.
@@ -2414,22 +2412,15 @@
void WebRtcVoiceMediaChannel::SetRawAudioSink(
uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) {
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::SetRawAudioSink";
- if (ssrc == 0) {
- default_sink_ = sink;
- if (default_recv_ssrc_ == -1) {
- return;
- }
- ssrc = static_cast<uint32_t>(default_recv_ssrc_);
- }
const auto it = recv_streams_.find(ssrc);
if (it == recv_streams_.end()) {
LOG(LS_WARNING) << "SetRawAudioSink: no recv stream" << ssrc;
return;
}
- it->second->SetRawAudioSink(sink);
+ it->second->SetRawAudioSink(std::move(sink));
}
int WebRtcVoiceMediaChannel::GetOutputLevel(int channel) {
diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h
index cd010f1..0f2f59e 100644
--- a/talk/media/webrtc/webrtcvoiceengine.h
+++ b/talk/media/webrtc/webrtcvoiceengine.h
@@ -198,7 +198,7 @@
void SetRawAudioSink(
uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) override;
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) override;
// implements Transport interface
bool SendRtp(const uint8_t* data,
@@ -269,8 +269,6 @@
int64_t default_recv_ssrc_ = -1;
// Volume for unsignalled stream, which may be set before the stream exists.
double default_recv_volume_ = 1.0;
- // Sink for unsignalled stream, which may be set before the stream exists.
- rtc::scoped_refptr<webrtc::AudioSinkInterface> default_sink_;
// Default SSRC to use for RTCP receiver reports in case of no signaled
// send streams. See: https://code.google.com/p/webrtc/issues/detail?id=4740
// and https://code.google.com/p/chromium/issues/detail?id=547661
diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
index 07894f6..a62bcb2 100644
--- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc
+++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
@@ -72,11 +72,6 @@
};
} // namespace
-class FakeAudioSink : public rtc::RefCountedObject<webrtc::AudioSinkInterface> {
- public:
- void OnData(const Data& audio) override {}
-};
-
class WebRtcVoiceEngineTestFake : public testing::Test {
public:
WebRtcVoiceEngineTestFake()
@@ -130,12 +125,6 @@
return *send_stream;
}
- const cricket::FakeAudioReceiveStream& GetRecvStream(uint32_t ssrc) {
- const auto* recv_stream = call_.GetAudioReceiveStream(ssrc);
- EXPECT_TRUE(recv_stream);
- return *recv_stream;
- }
-
const webrtc::AudioSendStream::Config& GetSendStreamConfig(uint32_t ssrc) {
const auto* send_stream = call_.GetAudioSendStream(ssrc);
EXPECT_TRUE(send_stream);
@@ -3116,57 +3105,6 @@
EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), -1);
}
-TEST_F(WebRtcVoiceEngineTestFake, SetRawAudioSink) {
- EXPECT_TRUE(SetupEngine());
- rtc::scoped_refptr<FakeAudioSink> fake_sink = new FakeAudioSink();
-
- // This should do nothing, since there's no recv stream yet.
- channel_->SetRawAudioSink(kSsrc1, fake_sink);
- // Ensure the ref count wasn't incremented.
- EXPECT_TRUE(fake_sink->HasOneRef());
-
- EXPECT_TRUE(
- channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(kSsrc1)));
- // Now, the channel should latch on to the sink.
- channel_->SetRawAudioSink(kSsrc1, fake_sink);
- EXPECT_FALSE(fake_sink->HasOneRef());
- EXPECT_EQ(fake_sink.get(), GetRecvStream(kSsrc1).sink().get());
-
- // Setting a nullptr should release the reference.
- channel_->SetRawAudioSink(kSsrc1, nullptr);
- EXPECT_TRUE(fake_sink->HasOneRef());
-}
-
-TEST_F(WebRtcVoiceEngineTestFake, SetRawAudioSinkDefaultRecvStream) {
- EXPECT_TRUE(SetupEngine());
- rtc::scoped_refptr<FakeAudioSink> fake_sink_1 = new FakeAudioSink();
- rtc::scoped_refptr<FakeAudioSink> fake_sink_2 = new FakeAudioSink();
-
- // Should be able to set a default sink even when no stream exists.
- channel_->SetRawAudioSink(0, fake_sink_1);
- EXPECT_FALSE(fake_sink_1->HasOneRef());
-
- // Create default channel.
- DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
- EXPECT_EQ(fake_sink_1.get(), GetRecvStream(0x01).sink().get());
-
- // Should be able to set the default sink after a stream exists.
- channel_->SetRawAudioSink(0, fake_sink_2);
- EXPECT_TRUE(fake_sink_1->HasOneRef());
- EXPECT_FALSE(fake_sink_2->HasOneRef());
- EXPECT_EQ(fake_sink_2.get(), GetRecvStream(0x01).sink().get());
-
- // If we remove and add a default stream, it should get the same sink.
- EXPECT_TRUE(channel_->RemoveRecvStream(0x01));
- DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
- EXPECT_FALSE(fake_sink_2->HasOneRef());
- EXPECT_EQ(fake_sink_2.get(), GetRecvStream(0x01).sink().get());
-
- // Finally, try resetting the default sink.
- channel_->SetRawAudioSink(0, nullptr);
- EXPECT_TRUE(fake_sink_2->HasOneRef());
-}
-
// Tests that the library initializes and shuts down properly.
TEST(WebRtcVoiceEngineTest, StartupShutdown) {
cricket::WebRtcVoiceEngine engine;
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index fa1165a..a59c3f8 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -45,6 +45,16 @@
namespace cricket {
using rtc::Bind;
+namespace {
+// See comment below for why we need to use a pointer to a scoped_ptr.
+bool SetRawAudioSink_w(VoiceMediaChannel* channel,
+ uint32_t ssrc,
+ rtc::scoped_ptr<webrtc::AudioSinkInterface>* sink) {
+ channel->SetRawAudioSink(ssrc, std::move(*sink));
+ return true;
+}
+} // namespace
+
enum {
MSG_EARLYMEDIATIMEOUT = 1,
MSG_SCREENCASTWINDOWEVENT,
@@ -1389,9 +1399,11 @@
void VoiceChannel::SetRawAudioSink(
uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) {
- worker_thread()->Invoke<void>(
- Bind(&VoiceMediaChannel::SetRawAudioSink, media_channel(), ssrc, sink));
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
+ // We need to work around Bind's lack of support for scoped_ptr and ownership
+ // passing. So we invoke to our own little routine that gets a pointer to
+ // our local variable. This is OK since we're synchronously invoking.
+ InvokeOnWorker(Bind(&SetRawAudioSink_w, media_channel(), ssrc, &sink));
}
bool VoiceChannel::GetStats(VoiceMediaInfo* stats) {
diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h
index 4cfefd8..d8fde67 100644
--- a/talk/session/media/channel.h
+++ b/talk/session/media/channel.h
@@ -375,9 +375,8 @@
// event 0-9, *, #, A-D.
bool InsertDtmf(uint32_t ssrc, int event_code, int duration);
bool SetOutputVolume(uint32_t ssrc, double volume);
- void SetRawAudioSink(
- uint32_t ssrc,
- const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink);
+ void SetRawAudioSink(uint32_t ssrc,
+ rtc::scoped_ptr<webrtc::AudioSinkInterface> sink);
// Get statistics about the current media session.
bool GetStats(VoiceMediaInfo* stats);
