Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(292)

Unified Diff: webrtc/call/call.cc

Issue 2553863003: Parse FlexFEC RTP headers in Call and add integration with BWE. (Closed)
Patch Set: Work in progress. Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/call/call.cc
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index 32234375316f57e7f168bf20178e58d11ce6bcdb..1ccf24bbdd1dcd43eb254d382ed1a14867f3f541 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -38,6 +38,9 @@
#include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_header_extension.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h"
+#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
#include "webrtc/modules/utility/include/process_thread.h"
#include "webrtc/system_wrappers/include/clock.h"
#include "webrtc/system_wrappers/include/cpu_info.h"
@@ -153,6 +156,12 @@ class Call : public webrtc::Call,
return nullptr;
}
+ RtpPacketReceived ParseRtpPacket(
+ const uint8_t* packet,
+ size_t length,
+ const PacketTime& packet_time,
+ const RtpHeaderExtensionMap* rtp_header_extensions);
+
void UpdateSendHistograms() EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_);
void UpdateReceiveHistograms();
void UpdateHistograms();
@@ -191,6 +200,14 @@ class Call : public webrtc::Call,
std::map<std::string, AudioReceiveStream*> sync_stream_mapping_
GUARDED_BY(receive_crit_);
+ // Registered RTP header extensions for each stream.
+ // Note that RTP header extensions are negotiated per track ("m= line") in the
+ // SDP, but we have no notion of tracks at the Call level. We therefore store
+ // the RTP header extensions per SSRC instead, which may lead to some
+ // overhead.
+ std::map<uint32_t, RtpHeaderExtensionMap> received_rtp_header_extensions_
+ GUARDED_BY(receive_crit_);
+
std::unique_ptr<RWLockWrapper> send_crit_;
// Audio and Video send streams are owned by the client that creates them.
std::map<uint32_t, AudioSendStream*> audio_send_ssrcs_ GUARDED_BY(send_crit_);
@@ -252,6 +269,31 @@ Call* Call::Create(const Call::Config& config) {
return new internal::Call(config);
}
+namespace {
+
+RtpHeaderExtensionMap CreateRtpHeaderExtensionMap(
danilchap 2016/12/06 15:04:30 this function implemented as a constructor of the
brandtr 2016/12/12 13:51:07 Great!
+ const std::vector<RtpExtension>& rtp_header_extensions) {
+ RtpHeaderExtensionMap map;
+ for (const auto& extension : rtp_header_extensions) {
+ if (extension.uri == AbsoluteSendTime::kUri) {
+ map.Register<AbsoluteSendTime>(extension.id);
+ } else if (extension.uri == AudioLevel::kUri) {
+ map.Register<AudioLevel>(extension.id);
+ } else if (extension.uri == TransmissionOffset::kUri) {
+ map.Register<TransmissionOffset>(extension.id);
+ } else if (extension.uri == TransportSequenceNumber::kUri) {
+ map.Register<TransportSequenceNumber>(extension.id);
+ } else if (extension.uri == VideoOrientation::kUri) {
+ map.Register<VideoOrientation>(extension.id);
+ } else if (extension.uri == PlayoutDelayLimits::kUri) {
+ map.Register<PlayoutDelayLimits>(extension.id);
+ }
+ }
+ return map;
+}
+
+} // namespace
+
namespace internal {
Call::Call(const Call::Config& config)
@@ -344,6 +386,23 @@ Call::~Call() {
Trace::ReturnTrace();
}
+RtpPacketReceived Call::ParseRtpPacket(
+ const uint8_t* packet,
+ size_t length,
+ const PacketTime& packet_time,
+ const RtpHeaderExtensionMap* rtp_header_extensions) {
+ RtpPacketReceived parsed_packet(nullptr);
+ parsed_packet.Parse(packet, length);
danilchap 2016/12/06 15:04:30 what if Parse fails?
brandtr 2016/12/12 13:51:07 Fixed by using optional.
+ int64_t arrival_time_ms;
+ if (packet_time.timestamp != -1) {
+ arrival_time_ms = (packet_time.timestamp + 500) / 1000;
+ } else {
+ arrival_time_ms = clock_->TimeInMilliseconds();
+ }
+ parsed_packet.set_arrival_time_ms(arrival_time_ms);
+ return parsed_packet;
+}
+
void Call::UpdateHistograms() {
RTC_HISTOGRAM_COUNTS_100000(
"WebRTC.Call.LifetimeInSeconds",
@@ -481,9 +540,11 @@ webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream(
event_log_->LogAudioReceiveStreamConfig(config);
AudioReceiveStream* receive_stream = new AudioReceiveStream(
&packet_router_,
- // TODO(nisse): Used only when UseSendSideBwe(config) is true.
- congestion_controller_->GetRemoteBitrateEstimator(true), config,
- config_.audio_state, event_log_);
+ congestion_controller_->GetRemoteBitrateEstimator(
+ CongestionController::UseSendSideBwe(
+ config.rtp.transport_cc,
+ CreateRtpHeaderExtensionMap(config.rtp.extensions))),
+ config, config_.audio_state, event_log_);
{
WriteLockScoped write_lock(*receive_crit_);
RTC_DCHECK(audio_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
@@ -658,18 +719,39 @@ webrtc::FlexfecReceiveStream* Call::CreateFlexfecReceiveStream(
const webrtc::FlexfecReceiveStream::Config& config) {
TRACE_EVENT0("webrtc", "Call::CreateFlexfecReceiveStream");
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
- FlexfecReceiveStream* receive_stream = new FlexfecReceiveStream(config, this);
+
+ RtpHeaderExtensionMap rtp_header_extensions =
+ CreateRtpHeaderExtensionMap(config.rtp_header_extensions);
+ RecoveredPacketReceiver* recovered_packet_receiver = this;
+ RemoteBitrateEstimator* remote_bitrate_estimator =
+ congestion_controller_->GetRemoteBitrateEstimator(
+ CongestionController::UseSendSideBwe(config.transport_cc,
+ rtp_header_extensions));
+ FlexfecReceiveStream* receive_stream = new FlexfecReceiveStream(
+ config, recovered_packet_receiver, remote_bitrate_estimator);
{
WriteLockScoped write_lock(*receive_crit_);
+
+ RTC_DCHECK(flexfec_receive_streams_.find(receive_stream) ==
+ flexfec_receive_streams_.end());
+ flexfec_receive_streams_.insert(receive_stream);
+
for (auto ssrc : config.protected_media_ssrcs)
flexfec_receive_ssrcs_media_.insert(std::make_pair(ssrc, receive_stream));
+
RTC_DCHECK(flexfec_receive_ssrcs_protection_.find(config.remote_ssrc) ==
flexfec_receive_ssrcs_protection_.end());
flexfec_receive_ssrcs_protection_[config.remote_ssrc] = receive_stream;
- flexfec_receive_streams_.insert(receive_stream);
+
+ RTC_DCHECK(received_rtp_header_extensions_.find(config.remote_ssrc) ==
+ received_rtp_header_extensions_.end());
+ received_rtp_header_extensions_[config.remote_ssrc] =
+ std::move(rtp_header_extensions);
danilchap 2016/12/06 15:04:30 do not move: RtpHeaderExtensionMap doesn't have a
brandtr 2016/12/12 13:51:07 Done.
}
+
// TODO(brandtr): Store config in RtcEventLog here.
+
return receive_stream;
}
@@ -677,21 +759,20 @@ void Call::DestroyFlexfecReceiveStream(
webrtc::FlexfecReceiveStream* receive_stream) {
TRACE_EVENT0("webrtc", "Call::DestroyFlexfecReceiveStream");
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
RTC_DCHECK(receive_stream != nullptr);
+
// There exist no other derived classes of webrtc::FlexfecReceiveStream,
// so this downcast is safe.
FlexfecReceiveStream* receive_stream_impl =
static_cast<FlexfecReceiveStream*>(receive_stream);
+ uint32_t ssrc = receive_stream_impl->config().remote_ssrc;
+
{
WriteLockScoped write_lock(*receive_crit_);
- // Remove all SSRCs pointing to the FlexfecReceiveStream to be destroyed.
- auto media_it = flexfec_receive_ssrcs_media_.begin();
- while (media_it != flexfec_receive_ssrcs_media_.end()) {
- if (media_it->second == receive_stream_impl)
- media_it = flexfec_receive_ssrcs_media_.erase(media_it);
- else
- ++media_it;
- }
+
+ received_rtp_header_extensions_.erase(ssrc);
+
auto prot_it = flexfec_receive_ssrcs_protection_.begin();
while (prot_it != flexfec_receive_ssrcs_protection_.end()) {
if (prot_it->second == receive_stream_impl)
@@ -699,8 +780,18 @@ void Call::DestroyFlexfecReceiveStream(
else
++prot_it;
}
+
+ auto media_it = flexfec_receive_ssrcs_media_.begin();
+ while (media_it != flexfec_receive_ssrcs_media_.end()) {
+ if (media_it->second == receive_stream_impl)
+ media_it = flexfec_receive_ssrcs_media_.erase(media_it);
+ else
+ ++media_it;
+ }
+
flexfec_receive_streams_.erase(receive_stream_impl);
}
+
delete receive_stream_impl;
}
@@ -1077,10 +1168,14 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
auto status = it->second->DeliverRtp(packet, length, packet_time)
? DELIVERY_OK
: DELIVERY_PACKET_ERROR;
- // Deliver media packets to FlexFEC subsystem.
+ // Deliver media packets to FlexFEC subsystem. RTP header extensions need
+ // not be parsed, as FlexFEC is oblivious to the semantic meaning of the
danilchap 2016/12/06 15:04:30 what about BWE header extensions?
brandtr 2016/12/12 13:51:07 Extended comment to why this is not relevant for m
+ // payload.
+ RtpPacketReceived parsed_packet =
+ ParseRtpPacket(packet, length, packet_time, nullptr);
auto it_bounds = flexfec_receive_ssrcs_media_.equal_range(ssrc);
for (auto it = it_bounds.first; it != it_bounds.second; ++it)
- it->second->AddAndProcessReceivedPacket(packet, length);
+ it->second->AddAndProcessReceivedPacket(parsed_packet);
if (status == DELIVERY_OK)
event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
return status;
@@ -1089,9 +1184,14 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
if (media_type == MediaType::ANY || media_type == MediaType::VIDEO) {
auto it = flexfec_receive_ssrcs_protection_.find(ssrc);
if (it != flexfec_receive_ssrcs_protection_.end()) {
- auto status = it->second->AddAndProcessReceivedPacket(packet, length)
- ? DELIVERY_OK
- : DELIVERY_PACKET_ERROR;
+ const RtpHeaderExtensionMap& extensions =
+ received_rtp_header_extensions_[ssrc];
+ RtpPacketReceived parsed_packet =
+ ParseRtpPacket(packet, length, packet_time, &extensions);
+ auto status =
+ it->second->AddAndProcessReceivedPacket(std::move(parsed_packet))
+ ? DELIVERY_OK
+ : DELIVERY_PACKET_ERROR;
if (status == DELIVERY_OK)
event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
return status;

Powered by Google App Engine
This is Rietveld 408576698