Make AudioSendStream to be OverheadObserver

In order to have correct overhead adjustments for ANA in AudioSendStream we need to know both transport and audio packetization overheads in AudioSendStream. This change makes AudioSendStream to be OverheadObserver instead of ChannelSend. This change is required for https://webrtc-review.googlesource.com/c/src/+/115082.

This change was also suggested earlier in TODO:
// TODO(solenberg): Make AudioSendStream an OverheadObserver instead.
https://cs.chromium.org/chromium/src/third_party/webrtc/audio/channel_send.cc?rcl=71b5a7df7794bbc4103296fcd8bd740acebdc901&l=1181

I think we should also consider moving TargetTransferRate observer to AudioSendStream. Since AudioSendStream owns encoder and configures ANA, it makes sense to consolidate all rate (and overhead) calculation there. Added TODO to clean it up in next chanelists.

Bug: webrtc:10150
Change-Id: I48791b998ea00ffde9ec75c6bca8b6dc83001b42
Reviewed-on: https://webrtc-review.googlesource.com/c/119121
Commit-Queue: Anton Sukhanov <sukhanov@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Minyue Li <minyue@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26540}
This commit is contained in:
Anton Sukhanov
2019-02-04 15:16:06 -08:00
committed by Commit Bot
parent e22498cc3e
commit 626015d7f8
5 changed files with 157 additions and 52 deletions

View File

@ -111,6 +111,7 @@ AudioSendStream::AudioSendStream(
voe::CreateChannelSend(worker_queue,
module_process_thread,
config.media_transport,
/*overhead_observer=*/this,
config.send_transport,
rtcp_rtt_stats,
event_log,
@ -152,7 +153,15 @@ AudioSendStream::AudioSendStream(
// should be no rtp_transport, and below check should be strengthened to XOR
// (either rtp_transport or media_transport but not both).
RTC_DCHECK(rtp_transport || config.media_transport);
if (config.media_transport) {
// TODO(sukhanov): Currently media transport audio overhead is considered
// constant, we will not get overhead_observer calls when using
// media_transport. In the future when we introduce RTP media transport we
// should make audio overhead interface consistent and work for both RTP and
// non-RTP implementations.
audio_overhead_per_packet_bytes_ =
config.media_transport->GetAudioPacketOverhead();
}
rtp_rtcp_module_ = channel_send_->GetRtpRtcp();
RTC_DCHECK(rtp_rtcp_module_);
@ -480,9 +489,38 @@ void AudioSendStream::OnPacketFeedbackVector(
}
}
void AudioSendStream::SetTransportOverhead(int transport_overhead_per_packet) {
void AudioSendStream::SetTransportOverhead(
int transport_overhead_per_packet_bytes) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
channel_send_->SetTransportOverhead(transport_overhead_per_packet);
rtc::CritScope cs(&overhead_per_packet_lock_);
transport_overhead_per_packet_bytes_ = transport_overhead_per_packet_bytes;
UpdateOverheadForEncoder();
}
void AudioSendStream::OnOverheadChanged(
size_t overhead_bytes_per_packet_bytes) {
rtc::CritScope cs(&overhead_per_packet_lock_);
audio_overhead_per_packet_bytes_ = overhead_bytes_per_packet_bytes;
UpdateOverheadForEncoder();
}
void AudioSendStream::UpdateOverheadForEncoder() {
const size_t overhead_per_packet_bytes = GetPerPacketOverheadBytes();
CallEncoder(channel_send_, [&](AudioEncoder* encoder) {
if (encoder) {
encoder->OnReceivedOverhead(overhead_per_packet_bytes);
}
});
}
size_t AudioSendStream::TestOnlyGetPerPacketOverheadBytes() const {
rtc::CritScope cs(&overhead_per_packet_lock_);
return GetPerPacketOverheadBytes();
}
size_t AudioSendStream::GetPerPacketOverheadBytes() const {
return transport_overhead_per_packet_bytes_ +
audio_overhead_per_packet_bytes_;
}
RtpState AudioSendStream::GetRtpState() const {
@ -568,10 +606,18 @@ bool AudioSendStream::SetupSendCodec(AudioSendStream* stream,
new_config.send_codec_spec->format.clockrate_hz);
}
// Set currently known overhead (used in ANA, opus only).
// If overhead changes later, it will be updated in UpdateOverheadForEncoder.
{
rtc::CritScope cs(&stream->overhead_per_packet_lock_);
encoder->OnReceivedOverhead(stream->GetPerPacketOverheadBytes());
}
stream->StoreEncoderProperties(encoder->SampleRateHz(),
encoder->NumChannels());
stream->channel_send_->SetEncoder(new_config.send_codec_spec->payload_type,
std::move(encoder));
return true;
}
@ -619,6 +665,12 @@ bool AudioSendStream::ReconfigureSendCodec(AudioSendStream* stream,
ReconfigureANA(stream, new_config);
ReconfigureCNG(stream, new_config);
// Set currently known overhead (used in ANA, opus only).
{
rtc::CritScope cs(&stream->overhead_per_packet_lock_);
stream->UpdateOverheadForEncoder();
}
return true;
}

View File

@ -36,7 +36,8 @@ class AudioState;
class AudioSendStream final : public webrtc::AudioSendStream,
public webrtc::BitrateAllocatorObserver,
public webrtc::PacketFeedbackObserver {
public webrtc::PacketFeedbackObserver,
public webrtc::OverheadObserver {
public:
AudioSendStream(const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
@ -85,11 +86,19 @@ class AudioSendStream final : public webrtc::AudioSendStream,
void OnPacketFeedbackVector(
const std::vector<PacketFeedback>& packet_feedback_vector) override;
void SetTransportOverhead(int transport_overhead_per_packet);
void SetTransportOverhead(int transport_overhead_per_packet_bytes);
// OverheadObserver override reports audio packetization overhead from
// RTP/RTCP module or Media Transport.
void OnOverheadChanged(size_t overhead_bytes_per_packet_bytes) override;
RtpState GetRtpState() const;
const voe::ChannelSendInterface* GetChannel() const;
// Returns combined per-packet overhead.
size_t TestOnlyGetPerPacketOverheadBytes() const
RTC_LOCKS_EXCLUDED(overhead_per_packet_lock_);
private:
class TimedTransport;
@ -116,6 +125,15 @@ class AudioSendStream final : public webrtc::AudioSendStream,
double bitrate_priority);
void RemoveBitrateObserver();
// Sets per-packet overhead on encoded (for ANA) based on current known values
// of transport and packetization overheads.
void UpdateOverheadForEncoder()
RTC_EXCLUSIVE_LOCKS_REQUIRED(overhead_per_packet_lock_);
// Returns combined per-packet overhead.
size_t GetPerPacketOverheadBytes() const
RTC_EXCLUSIVE_LOCKS_REQUIRED(overhead_per_packet_lock_);
void RegisterCngPayloadType(int payload_type, int clockrate_hz);
rtc::ThreadChecker worker_thread_checker_;
@ -156,6 +174,16 @@ class AudioSendStream final : public webrtc::AudioSendStream,
const std::vector<RtpExtension>& extensions);
static int TransportSeqNumId(const Config& config);
rtc::CriticalSection overhead_per_packet_lock_;
// Current transport overhead (ICE, TURN, etc.)
size_t transport_overhead_per_packet_bytes_
RTC_GUARDED_BY(overhead_per_packet_lock_) = 0;
// Current audio packetization overhead (RTP or Media Transport).
size_t audio_overhead_per_packet_bytes_
RTC_GUARDED_BY(overhead_per_packet_lock_) = 0;
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioSendStream);
};
} // namespace internal

View File

@ -520,9 +520,61 @@ TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) {
helper.transport(), Ne(nullptr)))
.Times(1);
}
// ModifyEncoder will be called to re-set overhead.
EXPECT_CALL(*helper.channel_send(), ModifyEncoder(testing::_)).Times(1);
send_stream->Reconfigure(new_config);
}
TEST(AudioSendStreamTest, OnTransportOverheadChanged) {
ConfigHelper helper(false, true);
auto send_stream = helper.CreateAudioSendStream();
auto new_config = helper.config();
// ModifyEncoder will be called on overhead change.
EXPECT_CALL(*helper.channel_send(), ModifyEncoder(testing::_)).Times(1);
const size_t transport_overhead_per_packet_bytes = 333;
send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes);
EXPECT_EQ(transport_overhead_per_packet_bytes,
send_stream->TestOnlyGetPerPacketOverheadBytes());
}
TEST(AudioSendStreamTest, OnAudioOverheadChanged) {
ConfigHelper helper(false, true);
auto send_stream = helper.CreateAudioSendStream();
auto new_config = helper.config();
// ModifyEncoder will be called on overhead change.
EXPECT_CALL(*helper.channel_send(), ModifyEncoder(testing::_)).Times(1);
const size_t audio_overhead_per_packet_bytes = 555;
send_stream->OnOverheadChanged(audio_overhead_per_packet_bytes);
EXPECT_EQ(audio_overhead_per_packet_bytes,
send_stream->TestOnlyGetPerPacketOverheadBytes());
}
TEST(AudioSendStreamTest, OnAudioAndTransportOverheadChanged) {
ConfigHelper helper(false, true);
auto send_stream = helper.CreateAudioSendStream();
auto new_config = helper.config();
// ModifyEncoder will be called when each of overhead changes.
EXPECT_CALL(*helper.channel_send(), ModifyEncoder(testing::_)).Times(2);
const size_t transport_overhead_per_packet_bytes = 333;
send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes);
const size_t audio_overhead_per_packet_bytes = 555;
send_stream->OnOverheadChanged(audio_overhead_per_packet_bytes);
EXPECT_EQ(
transport_overhead_per_packet_bytes + audio_overhead_per_packet_bytes,
send_stream->TestOnlyGetPerPacketOverheadBytes());
}
// Validates that reconfiguring the AudioSendStream with a Frame encryptor
// correctly reconfigures on the object without crashing.
TEST(AudioSendStreamTest, ReconfigureWithFrameEncryptor) {

View File

@ -78,7 +78,6 @@ class VoERtcpObserver;
class ChannelSend
: public ChannelSendInterface,
public OverheadObserver,
public AudioPacketizationCallback, // receive encoded packets from the
// ACM
public TargetTransferRateObserver {
@ -90,6 +89,7 @@ class ChannelSend
ChannelSend(rtc::TaskQueue* encoder_queue,
ProcessThread* module_process_thread,
MediaTransportInterface* media_transport,
OverheadObserver* overhead_observer,
Transport* rtp_transport,
RtcpRttStats* rtcp_rtt_stats,
RtcEventLog* rtc_event_log,
@ -160,8 +160,6 @@ class ChannelSend
// packet.
void ProcessAndEncodeAudio(std::unique_ptr<AudioFrame> audio_frame) override;
void SetTransportOverhead(size_t transport_overhead_per_packet) override;
// The existence of this function alongside OnUplinkPacketLossRate is
// a compromise. We want the encoder to be agnostic of the PLR source, but
// we also don't want it to receive conflicting information from TWCC and
@ -188,17 +186,11 @@ class ChannelSend
size_t payloadSize,
const RTPFragmentationHeader* fragmentation) override;
// From OverheadObserver in the RTP/RTCP module
void OnOverheadChanged(size_t overhead_bytes_per_packet) override;
void OnUplinkPacketLossRate(float packet_loss_rate);
bool InputMute() const;
int SetSendRtpHeaderExtension(bool enable, RTPExtensionType type, int id);
void UpdateOverheadForEncoder()
RTC_EXCLUSIVE_LOCKS_REQUIRED(overhead_per_packet_lock_);
int32_t SendRtpAudio(FrameType frameType,
uint8_t payloadType,
uint32_t timeStamp,
@ -254,10 +246,7 @@ class ChannelSend
// TODO(henrika): can today be accessed on the main thread and on the
// task queue; hence potential race.
bool _includeAudioLevelIndication;
size_t transport_overhead_per_packet_
RTC_GUARDED_BY(overhead_per_packet_lock_);
size_t rtp_overhead_per_packet_ RTC_GUARDED_BY(overhead_per_packet_lock_);
rtc::CriticalSection overhead_per_packet_lock_;
// RtcpBandwidthObserver
const std::unique_ptr<VoERtcpObserver> rtcp_observer_;
@ -636,6 +625,7 @@ int32_t ChannelSend::SendMediaTransportAudio(
ChannelSend::ChannelSend(rtc::TaskQueue* encoder_queue,
ProcessThread* module_process_thread,
MediaTransportInterface* media_transport,
OverheadObserver* overhead_observer,
Transport* rtp_transport,
RtcpRttStats* rtcp_rtt_stats,
RtcEventLog* rtc_event_log,
@ -650,8 +640,6 @@ ChannelSend::ChannelSend(rtc::TaskQueue* encoder_queue,
input_mute_(false),
previous_frame_muted_(false),
_includeAudioLevelIndication(false),
transport_overhead_per_packet_(0),
rtp_overhead_per_packet_(0),
rtcp_observer_(new VoERtcpObserver(this)),
feedback_observer_proxy_(new TransportFeedbackProxy()),
seq_num_allocator_proxy_(new TransportSequenceNumberProxy()),
@ -677,7 +665,12 @@ ChannelSend::ChannelSend(rtc::TaskQueue* encoder_queue,
// RTPMediaTransport. In this case it means that overhead and bandwidth
// observers should not be called when using media transport.
if (!media_transport_) {
configuration.overhead_observer = this;
// TODO(sukhanov): Overhead observer is only needed for RTP path, because in
// media transport audio overhead is currently considered constant (see
// getter MediaTransportInterface::GetAudioPacketOverhead). In the future
// when we introduce RTP media transport we should make audio overhead
// interface consistent and work for both RTP and non-RTP implementations.
configuration.overhead_observer = overhead_observer;
configuration.bandwidth_callback = rtcp_observer_.get();
configuration.transport_feedback_callback = feedback_observer_proxy_.get();
}
@ -704,7 +697,6 @@ ChannelSend::ChannelSend(rtc::TaskQueue* encoder_queue,
if (media_transport_) {
RTC_DLOG(LS_INFO) << "Setting media_transport_ rate observers.";
media_transport_->AddTargetTransferRateObserver(this);
OnOverheadChanged(media_transport_->GetAudioPacketOverhead());
} else {
RTC_DLOG(LS_INFO) << "Not setting media_transport_ rate observers.";
}
@ -731,7 +723,6 @@ ChannelSend::~ChannelSend() {
}
StopSend();
int error = audio_coding_->RegisterTransportCallback(NULL);
RTC_DCHECK_EQ(0, error);
@ -814,7 +805,9 @@ bool ChannelSend::SetEncoder(int payload_type,
void ChannelSend::ModifyEncoder(
rtc::FunctionView<void(std::unique_ptr<AudioEncoder>*)> modifier) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
// This method can be called on the worker thread, module process thread
// or network thread. Audio coding is thread safe, so we do not need to
// enforce the calling thread.
audio_coding_->ModifyEncoder(modifier);
}
@ -1145,30 +1138,6 @@ void ChannelSend::ProcessAndEncodeAudioOnTaskQueue(AudioFrame* audio_input) {
_timeStamp += static_cast<uint32_t>(audio_input->samples_per_channel_);
}
void ChannelSend::UpdateOverheadForEncoder() {
size_t overhead_per_packet =
transport_overhead_per_packet_ + rtp_overhead_per_packet_;
audio_coding_->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
if (*encoder) {
(*encoder)->OnReceivedOverhead(overhead_per_packet);
}
});
}
void ChannelSend::SetTransportOverhead(size_t transport_overhead_per_packet) {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
rtc::CritScope cs(&overhead_per_packet_lock_);
transport_overhead_per_packet_ = transport_overhead_per_packet;
UpdateOverheadForEncoder();
}
// TODO(solenberg): Make AudioSendStream an OverheadObserver instead.
void ChannelSend::OnOverheadChanged(size_t overhead_bytes_per_packet) {
rtc::CritScope cs(&overhead_per_packet_lock_);
rtp_overhead_per_packet_ = overhead_bytes_per_packet;
UpdateOverheadForEncoder();
}
ANAStats ChannelSend::GetANAStatistics() const {
RTC_DCHECK_RUN_ON(&worker_thread_checker_);
return audio_coding_->GetANAStats();
@ -1243,6 +1212,9 @@ void ChannelSend::SetFrameEncryptor(
}
}
// TODO(sukhanov): Consider moving TargetTransferRate observer to
// AudioSendStream. Since AudioSendStream owns encoder and configures ANA, it
// makes sense to consolidate all rate (and overhead) calculation there.
void ChannelSend::OnTargetTransferRate(TargetTransferRate rate) {
RTC_DCHECK(media_transport_);
OnReceivedRtt(rate.network_estimate.round_trip_time.ms());
@ -1264,6 +1236,7 @@ std::unique_ptr<ChannelSendInterface> CreateChannelSend(
rtc::TaskQueue* encoder_queue,
ProcessThread* module_process_thread,
MediaTransportInterface* media_transport,
OverheadObserver* overhead_observer,
Transport* rtp_transport,
RtcpRttStats* rtcp_rtt_stats,
RtcEventLog* rtc_event_log,
@ -1272,9 +1245,9 @@ std::unique_ptr<ChannelSendInterface> CreateChannelSend(
bool extmap_allow_mixed,
int rtcp_report_interval_ms) {
return absl::make_unique<ChannelSend>(
encoder_queue, module_process_thread, media_transport, rtp_transport,
rtcp_rtt_stats, rtc_event_log, frame_encryptor, crypto_options,
extmap_allow_mixed, rtcp_report_interval_ms);
encoder_queue, module_process_thread, media_transport, overhead_observer,
rtp_transport, rtcp_rtt_stats, rtc_event_log, frame_encryptor,
crypto_options, extmap_allow_mixed, rtcp_report_interval_ms);
}
} // namespace voe

View File

@ -89,7 +89,6 @@ class ChannelSendInterface {
virtual void ProcessAndEncodeAudio(
std::unique_ptr<AudioFrame> audio_frame) = 0;
virtual void SetTransportOverhead(size_t transport_overhead_per_packet) = 0;
virtual RtpRtcp* GetRtpRtcp() const = 0;
virtual void OnTwccBasedUplinkPacketLossRate(float packet_loss_rate) = 0;
@ -118,6 +117,7 @@ std::unique_ptr<ChannelSendInterface> CreateChannelSend(
rtc::TaskQueue* encoder_queue,
ProcessThread* module_process_thread,
MediaTransportInterface* media_transport,
OverheadObserver* overhead_observer,
Transport* rtp_transport,
RtcpRttStats* rtcp_rtt_stats,
RtcEventLog* rtc_event_log,