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}
This commit is contained in:
deadbeef 2016-01-13 12:00:26 -08:00 committed by Commit bot
parent 8432e1f4b8
commit 2d110be77f
23 changed files with 57 additions and 134 deletions

View File

@ -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 @@ class AudioProviderInterface {
// 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() {}

View File

@ -1989,12 +1989,8 @@ TEST_F(PeerConnectionInterfaceTest, SdpWithoutMsidCreatesDefaultStream) {
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

View File

@ -96,7 +96,8 @@ void RemoteAudioSource::Initialize(uint32_t ssrc,
// 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)));
}
}

View File

@ -70,14 +70,13 @@ class MockAudioProvider : public AudioProviderInterface {
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.

View File

@ -1328,14 +1328,13 @@ void WebRtcSession::SetAudioPlayoutVolume(uint32_t ssrc, double volume) {
}
}
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,

View File

@ -254,9 +254,8 @@ class WebRtcSession : public AudioProviderInterface,
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;

View File

@ -349,8 +349,8 @@ class FakeVoiceMediaChannel : public RtpHelper<VoiceMediaChannel> {
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 @@ class FakeVoiceMediaChannel : public RtpHelper<VoiceMediaChannel> {
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.

View File

@ -1037,7 +1037,7 @@ class VoiceMediaChannel : public MediaChannel {
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> {

View File

@ -93,8 +93,8 @@ webrtc::AudioReceiveStream::Stats FakeAudioReceiveStream::GetStats() const {
}
void FakeAudioReceiveStream::SetSink(
const rtc::scoped_refptr<webrtc::AudioSinkInterface>& sink) {
sink_ = sink;
rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
sink_ = std::move(sink);
}
FakeVideoSendStream::FakeVideoSendStream(

View File

@ -89,9 +89,6 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream {
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 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream {
// 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,

View File

@ -1244,10 +1244,9 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
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 @@ void WebRtcVoiceMediaChannel::OnPacketReceived(
}
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 @@ bool WebRtcVoiceMediaChannel::GetStats(VoiceMediaInfo* info) {
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) {

View File

@ -198,7 +198,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
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 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
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

View File

@ -72,11 +72,6 @@ class FakeVoEWrapper : public cricket::VoEWrapper {
};
} // 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 @@ class WebRtcVoiceEngineTestFake : public testing::Test {
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 @@ TEST_F(WebRtcVoiceEngineTestFake, AssociateChannelResetUponDeleteChannnel) {
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;

View File

@ -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 @@ bool VoiceChannel::SetOutputVolume(uint32_t ssrc, double volume) {
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) {

View File

@ -375,9 +375,8 @@ class VoiceChannel : public BaseChannel {
// 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);

View File

@ -235,10 +235,9 @@ webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats() const {
return stats;
}
void AudioReceiveStream::SetSink(
const rtc::scoped_refptr<AudioSinkInterface>& sink) {
void AudioReceiveStream::SetSink(rtc::scoped_ptr<AudioSinkInterface> sink) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
channel_proxy_->SetSink(sink);
channel_proxy_->SetSink(std::move(sink));
}
const webrtc::AudioReceiveStream::Config& AudioReceiveStream::config() const {

View File

@ -45,7 +45,7 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream {
// webrtc::AudioReceiveStream implementation.
webrtc::AudioReceiveStream::Stats GetStats() const override;
void SetSink(const rtc::scoped_refptr<AudioSinkInterface>& sink) override;
void SetSink(rtc::scoped_ptr<AudioSinkInterface> sink) override;
const webrtc::AudioReceiveStream::Config& config() const;

View File

@ -19,12 +19,10 @@
#include <inttypes.h>
#include <stddef.h>
#include "webrtc/base/refcount.h"
namespace webrtc {
// Represents a simple push audio sink.
class AudioSinkInterface : public rtc::RefCountInterface {
class AudioSinkInterface {
public:
virtual ~AudioSinkInterface() {}

View File

@ -15,7 +15,7 @@
#include <string>
#include <vector>
#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/config.h"
#include "webrtc/stream.h"
#include "webrtc/transport.h"
@ -112,12 +112,12 @@ class AudioReceiveStream : public ReceiveStream {
// Sets an audio sink that receives unmixed audio from the receive stream.
// Ownership of the sink is passed to the stream and can be used by the
// caller to do lifetime management (i.e. when the sink's dtor is called).
// Only one sink can be set and passing a null sink clears an existing one.
// Only one sink can be set and passing a null sink, clears an existing one.
// NOTE: Audio must still somehow be pulled through AudioTransport for audio
// to stream through this sink. In practice, this happens if mixed audio
// is being pulled+rendered and/or if audio is being pulled for the purposes
// of feeding to the AEC.
virtual void SetSink(const rtc::scoped_refptr<AudioSinkInterface>& sink) = 0;
virtual void SetSink(rtc::scoped_ptr<AudioSinkInterface> sink) = 0;
};
} // namespace webrtc

View File

@ -1185,9 +1185,9 @@ Channel::UpdateLocalTimeStamp()
return 0;
}
void Channel::SetSink(const rtc::scoped_refptr<AudioSinkInterface>& sink) {
void Channel::SetSink(rtc::scoped_ptr<AudioSinkInterface> sink) {
CriticalSectionScoped cs(&_callbackCritSect);
audio_sink_ = sink;
audio_sink_ = std::move(sink);
}
int32_t

View File

@ -14,7 +14,6 @@
#include "webrtc/audio/audio_sink.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/common_audio/resampler/include/push_resampler.h"
#include "webrtc/common_types.h"
#include "webrtc/modules/audio_coding/include/audio_coding_module.h"
@ -194,7 +193,7 @@ public:
CriticalSectionWrapper* callbackCritSect);
int32_t UpdateLocalTimeStamp();
void SetSink(const rtc::scoped_refptr<AudioSinkInterface>& sink);
void SetSink(rtc::scoped_ptr<AudioSinkInterface> sink);
// API methods
@ -512,7 +511,7 @@ private:
TelephoneEventHandler* telephone_event_handler_;
rtc::scoped_ptr<RtpRtcp> _rtpRtcpModule;
rtc::scoped_ptr<AudioCodingModule> audio_coding_;
rtc::scoped_refptr<AudioSinkInterface> audio_sink_;
rtc::scoped_ptr<AudioSinkInterface> audio_sink_;
AudioLevel _outputAudioLevel;
bool _externalTransport;
AudioFrame _audioFrame;

View File

@ -139,9 +139,9 @@ bool ChannelProxy::SendTelephoneEventOutband(uint8_t event,
channel()->SendTelephoneEventOutband(event, duration_ms, 10, false) == 0;
}
void ChannelProxy::SetSink(const rtc::scoped_refptr<AudioSinkInterface>& sink) {
void ChannelProxy::SetSink(rtc::scoped_ptr<AudioSinkInterface> sink) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
channel()->SetSink(sink);
channel()->SetSink(std::move(sink));
}
Channel* ChannelProxy::channel() const {

View File

@ -11,7 +11,6 @@
#ifndef WEBRTC_VOICE_ENGINE_CHANNEL_PROXY_H_
#define WEBRTC_VOICE_ENGINE_CHANNEL_PROXY_H_
#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/voice_engine/channel_manager.h"
#include "webrtc/voice_engine/include/voe_rtp_rtcp.h"
@ -66,7 +65,7 @@ class ChannelProxy {
virtual bool SetSendTelephoneEventPayloadType(int payload_type);
virtual bool SendTelephoneEventOutband(uint8_t event, uint32_t duration_ms);
virtual void SetSink(const rtc::scoped_refptr<AudioSinkInterface>& sink);
virtual void SetSink(rtc::scoped_ptr<AudioSinkInterface> sink);
private:
Channel* channel() const;