admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of Add RtcpRttStats to AudioStream (patchset #1 id:1 of https://codereview.webrtc.org/2402333002/ )

Browse Source 
Reason for revert:
Speculative revert.
Intermittent memory access errors suspected to be caused by this cl.

See for instance https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Light/builds/8018

UNADDRESSABLE ACCESS of freed memory: reading 0x0331d330-0x0331d334 4 byte(s)
# 0 webrtc::voe::RtcpRttStatsProxy::LastProcessedRtt
# 1 webrtc::ModuleRtpRtcpImpl::Process

Original issue's description:
> Add RtcpRttStats to AudioStream
>
> BUG=webrtc:6508
>
> Committed: https://crrev.com/e0729c56d35acfaf9738fdb32c6508cd78eaf089
> Cr-Commit-Position: refs/heads/master@{#14595}

TBR=stefan@webrtc.org,minyue@webrtc.org,solenberg@webrtc.org,michaelt@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6508

Review-Url: https://codereview.webrtc.org/2415943002
Cr-Commit-Position: refs/heads/master@{#14631}
This commit is contained in:
sprang 2016-10-13 06:23:11 -07:00 committed by Commit bot
parent b593bc06ad
commit 982bf89444
10 changed files with 8 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
webrtc/audio/audio_send_stream.cc
View File

@ -64,8 +64,7 @@ AudioSendStream::AudioSendStream(
rtc::TaskQueue* worker_queue,
CongestionController* congestion_controller,
BitrateAllocator* bitrate_allocator,
RtcEventLog* event_log,
RtcpRttStats* rtcp_rtt_stats)
RtcEventLog* event_log)
: worker_queue_(worker_queue),
config_(config),
audio_state_(audio_state),
@ -78,7 +77,6 @@ AudioSendStream::AudioSendStream(
VoiceEngineImpl* voe_impl = static_cast<VoiceEngineImpl*>(voice_engine());
channel_proxy_ = voe_impl->GetChannelProxy(config_.voe_channel_id);
channel_proxy_->SetRtcEventLog(event_log);
channel_proxy_->SetRtcpRttStats(rtcp_rtt_stats);
channel_proxy_->RegisterSenderCongestionControlObjects(
congestion_controller->pacer(),
congestion_controller->GetTransportFeedbackObserver(),

4
webrtc/audio/audio_send_stream.h
View File

@ -23,7 +23,6 @@ namespace webrtc {
class CongestionController;
class VoiceEngine;
class RtcEventLog;
class RtcpRttStats;
namespace voe {
class ChannelProxy;
@ -38,8 +37,7 @@ class AudioSendStream final : public webrtc::AudioSendStream,
rtc::TaskQueue* worker_queue,
CongestionController* congestion_controller,
BitrateAllocator* bitrate_allocator,
RtcEventLog* event_log,
RtcpRttStats* rtcp_rtt_stats);
RtcEventLog* event_log);
~AudioSendStream() override;
// webrtc::AudioSendStream implementation.

15
webrtc/audio/audio_send_stream_unittest.cc
View File

@ -20,7 +20,6 @@
#include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h"
#include "webrtc/modules/pacing/paced_sender.h"
#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h"
#include "webrtc/modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h"
#include "webrtc/test/gtest.h"
#include "webrtc/test/mock_voe_channel_proxy.h"
#include "webrtc/test/mock_voice_engine.h"
@ -110,8 +109,6 @@ struct ConfigHelper {
.Times(1);
EXPECT_CALL(*channel_proxy_, SetRtcEventLog(testing::IsNull()))
.Times(1); // Destructor resets the event log
EXPECT_CALL(*channel_proxy_, SetRtcpRttStats(&rtcp_rtt_stats_))
.Times(1);
return channel_proxy_;
}));
stream_config_.voe_channel_id = kChannelId;
@ -135,7 +132,6 @@ struct ConfigHelper {
BitrateAllocator* bitrate_allocator() { return &bitrate_allocator_; }
rtc::TaskQueue* worker_queue() { return &worker_queue_; }
RtcEventLog* event_log() { return &event_log_; }
RtcpRttStats* rtcp_rtt_stats() { return &rtcp_rtt_stats_; }
void SetupMockForSendTelephoneEvent() {
EXPECT_TRUE(channel_proxy_);
@ -190,7 +186,6 @@ struct ConfigHelper {
testing::NiceMock<MockRemoteBitrateObserver> remote_bitrate_observer_;
CongestionController congestion_controller_;
MockRtcEventLog event_log_;
MockRtcpRttStats rtcp_rtt_stats_;
testing::NiceMock<MockLimitObserver> limit_observer_;
BitrateAllocator bitrate_allocator_;
// |worker_queue| is defined last to ensure all pending tasks are cancelled
@ -220,7 +215,7 @@ TEST(AudioSendStreamTest, ConstructDestruct) {
internal::AudioSendStream send_stream(
helper.config(), helper.audio_state(), helper.worker_queue(),
helper.congestion_controller(), helper.bitrate_allocator(),
helper.event_log(), helper.rtcp_rtt_stats());
helper.event_log());
}
TEST(AudioSendStreamTest, SendTelephoneEvent) {
@ -228,7 +223,7 @@ TEST(AudioSendStreamTest, SendTelephoneEvent) {
internal::AudioSendStream send_stream(
helper.config(), helper.audio_state(), helper.worker_queue(),
helper.congestion_controller(), helper.bitrate_allocator(),
helper.event_log(), helper.rtcp_rtt_stats());
helper.event_log());
helper.SetupMockForSendTelephoneEvent();
EXPECT_TRUE(send_stream.SendTelephoneEvent(kTelephoneEventPayloadType,
kTelephoneEventCode, kTelephoneEventDuration));
@ -239,7 +234,7 @@ TEST(AudioSendStreamTest, SetMuted) {
internal::AudioSendStream send_stream(
helper.config(), helper.audio_state(), helper.worker_queue(),
helper.congestion_controller(), helper.bitrate_allocator(),
helper.event_log(), helper.rtcp_rtt_stats());
helper.event_log());
EXPECT_CALL(*helper.channel_proxy(), SetInputMute(true));
send_stream.SetMuted(true);
}
@ -249,7 +244,7 @@ TEST(AudioSendStreamTest, GetStats) {
internal::AudioSendStream send_stream(
helper.config(), helper.audio_state(), helper.worker_queue(),
helper.congestion_controller(), helper.bitrate_allocator(),
helper.event_log(), helper.rtcp_rtt_stats());
helper.event_log());
helper.SetupMockForGetStats();
AudioSendStream::Stats stats = send_stream.GetStats();
EXPECT_EQ(kSsrc, stats.local_ssrc);
@ -279,7 +274,7 @@ TEST(AudioSendStreamTest, GetStatsTypingNoiseDetected) {
internal::AudioSendStream send_stream(
helper.config(), helper.audio_state(), helper.worker_queue(),
helper.congestion_controller(), helper.bitrate_allocator(),
helper.event_log(), helper.rtcp_rtt_stats());
helper.event_log());
helper.SetupMockForGetStats();
EXPECT_FALSE(send_stream.GetStats().typing_noise_detected);

2
webrtc/call/call.cc
View File

@ -373,7 +373,7 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream(
event_log_->LogAudioSendStreamConfig(config);
AudioSendStream* send_stream = new AudioSendStream(
config, config_.audio_state, &worker_queue_, congestion_controller_.get(),
bitrate_allocator_.get(), event_log_, call_stats_->rtcp_rtt_stats());
bitrate_allocator_.get(), event_log_);
{
WriteLockScoped write_lock(*send_crit_);
RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) ==

26
webrtc/modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h
View File

@ -1,26 +0,0 @@
/*
* Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/
#ifndef WEBRTC_MODULES_RTP_RTCP_MOCKS_MOCK_RTCP_RTT_STATS_H_
#define WEBRTC_MODULES_RTP_RTCP_MOCKS_MOCK_RTCP_RTT_STATS_H_
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/test/gmock.h"
namespace webrtc {
class MockRtcpRttStats : public RtcpRttStats {
public:
MOCK_METHOD1(OnRttUpdate, void(int64_t rtt));
MOCK_CONST_METHOD0(LastProcessedRtt, int64_t());
};
} // namespace webrtc
#endif // WEBRTC_MODULES_RTP_RTCP_MOCKS_MOCK_RTCP_RTT_STATS_H_

1
webrtc/test/mock_voe_channel_proxy.h
View File

@ -59,7 +59,6 @@ class MockVoEChannelProxy : public voe::ChannelProxy {
const rtc::scoped_refptr<AudioDecoderFactory>&());
MOCK_METHOD1(SetChannelOutputVolumeScaling, void(float scaling));
MOCK_METHOD1(SetRtcEventLog, void(RtcEventLog* event_log));
MOCK_METHOD1(SetRtcpRttStats, void(RtcpRttStats* rtcp_rtt_stats));
MOCK_METHOD1(SetBitrate, void(int bitrate_bps));
};
} // namespace test

34
webrtc/voice_engine/channel.cc
View File

@ -157,34 +157,6 @@ class RtcEventLogProxy final : public webrtc::RtcEventLog {
RTC_DISALLOW_COPY_AND_ASSIGN(RtcEventLogProxy);
};
class RtcpRttStatsProxy final : public RtcpRttStats {
public:
RtcpRttStatsProxy() : rtcp_rtt_stats_(nullptr) {}
void OnRttUpdate(int64_t rtt) override {
rtc::CritScope lock(&crit_);
if (rtcp_rtt_stats_)
rtcp_rtt_stats_->OnRttUpdate(rtt);
}
int64_t LastProcessedRtt() const override {
rtc::CritScope lock(&crit_);
if (rtcp_rtt_stats_ == nullptr)
return 0;
return rtcp_rtt_stats_->LastProcessedRtt();
}
void SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats) {
rtc::CritScope lock(&crit_);
rtcp_rtt_stats_ = rtcp_rtt_stats;
}
private:
rtc::CriticalSection crit_;
RtcpRttStats* rtcp_rtt_stats_ GUARDED_BY(crit_);
RTC_DISALLOW_COPY_AND_ASSIGN(RtcpRttStatsProxy);
};
class TransportFeedbackProxy : public TransportFeedbackObserver {
public:
TransportFeedbackProxy() : feedback_observer_(nullptr) {
@ -843,7 +815,6 @@ Channel::Channel(int32_t channelId,
: _instanceId(instanceId),
_channelId(channelId),
event_log_proxy_(new RtcEventLogProxy()),
rtcp_rtt_stats_proxy_(new RtcpRttStatsProxy()),
rtp_header_parser_(RtpHeaderParser::Create()),
rtp_payload_registry_(
new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))),
@ -928,7 +899,6 @@ Channel::Channel(int32_t channelId,
configuration.transport_feedback_callback = feedback_observer_proxy_.get();
}
configuration.event_log = &(*event_log_proxy_);
configuration.rtt_stats = &(*rtcp_rtt_stats_proxy_);
configuration.retransmission_rate_limiter =
retransmission_rate_limiter_.get();
@ -2873,10 +2843,6 @@ void Channel::SetRtcEventLog(RtcEventLog* event_log) {
event_log_proxy_->SetEventLog(event_log);
}
void Channel::SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats) {
rtcp_rtt_stats_proxy_->SetRtcpRttStats(rtcp_rtt_stats);
}
int Channel::RegisterExternalMediaProcessing(ProcessingTypes type,
VoEMediaProcess& processObject) {
WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),

4
webrtc/voice_engine/channel.h
View File

@ -66,7 +66,6 @@ namespace voe {
class OutputMixer;
class RtcEventLogProxy;
class RtcpRttStatsProxy;
class RtpPacketSenderProxy;
class Statistics;
class StatisticsProxy;
@ -421,8 +420,6 @@ class Channel
// Set a RtcEventLog logging object.
void SetRtcEventLog(RtcEventLog* event_log);
void SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats);
protected:
void OnIncomingFractionLoss(int fraction_lost);
@ -458,7 +455,6 @@ class Channel
ChannelState channel_state_;
std::unique_ptr<voe::RtcEventLogProxy> event_log_proxy_;
std::unique_ptr<voe::RtcpRttStatsProxy> rtcp_rtt_stats_proxy_;
std::unique_ptr<RtpHeaderParser> rtp_header_parser_;
std::unique_ptr<RTPPayloadRegistry> rtp_payload_registry_;

5
webrtc/voice_engine/channel_proxy.cc
View File

@ -214,11 +214,6 @@ void ChannelProxy::SetRtcEventLog(RtcEventLog* event_log) {
channel()->SetRtcEventLog(event_log);
}
void ChannelProxy::SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
channel()->SetRtcpRttStats(rtcp_rtt_stats);
}
Channel* ChannelProxy::channel() const {
RTC_DCHECK(channel_owner_.channel());
return channel_owner_.channel();

3
webrtc/voice_engine/channel_proxy.h
View File

@ -25,7 +25,6 @@ namespace webrtc {
class AudioSinkInterface;
class PacketRouter;
class RtcEventLog;
class RtcpRttStats;
class RtpPacketSender;
class Transport;
class TransportFeedbackObserver;
@ -92,8 +91,6 @@ class ChannelProxy {
virtual void SetRtcEventLog(RtcEventLog* event_log);
virtual void SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats);
private:
Channel* channel() const;