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

Unified Diff: webrtc/modules/rtp_rtcp/source/flexfec_sender.cc

Issue 2490523002: Wire up FlexfecSender in RTPSenderVideo. (Closed)
Patch Set: Feedback response 3. Created 4 years, 1 month 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
« no previous file with comments | « webrtc/modules/rtp_rtcp/include/flexfec_sender.h ('k') | webrtc/modules/rtp_rtcp/source/rtp_sender_video.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/modules/rtp_rtcp/source/flexfec_sender.cc
diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_sender.cc b/webrtc/modules/rtp_rtcp/source/flexfec_sender.cc
index 55e8e5266919bbb978b44e8d5fcd1fff356549c1..54ed1ab089468c7c5df720452c5e4b56205cf0e6 100644
--- a/webrtc/modules/rtp_rtcp/source/flexfec_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/flexfec_sender.cc
@@ -38,6 +38,26 @@ const int kMsToRtpTimestamp = kVideoPayloadTypeFrequency / 1000;
// How often to log the generated FEC packets to the text log.
constexpr int64_t kPacketLogIntervalMs = 10000;
+// Only activate BWE header extensions for FlexFEC.
+RtpHeaderExtensionMap FilterRtpHeaderExtensions(
+ const std::vector<RtpExtension>& rtp_header_extensions) {
+ RtpHeaderExtensionMap map;
+ for (const auto& extension : rtp_header_extensions) {
+ if (extension.uri == RtpExtension::kTransportSequenceNumberUri) {
+ map.Register(kRtpExtensionTransportSequenceNumber, extension.id);
danilchap 2016/11/10 11:29:23 I would like to remove this version of Register ev
brandtr 2016/11/10 12:00:41 Done.
+ } else if (extension.uri == RtpExtension::kAbsSendTimeUri) {
+ map.Register(kRtpExtensionAbsoluteSendTime, extension.id);
+ } else if (extension.uri == RtpExtension::kTimestampOffsetUri) {
+ map.Register(kRtpExtensionTransmissionTimeOffset, extension.id);
+ } else {
+ LOG(LS_WARNING) << "RTP header extension with id: " << extension.id
+ << ", uri: " << extension.uri
+ << ", is unsupported by FlexfecSender.";
+ }
+ }
+ return map;
+}
+
} // namespace
FlexfecSender::FlexfecSender(
@@ -57,7 +77,8 @@ FlexfecSender::FlexfecSender(
protected_media_ssrc_(protected_media_ssrc),
seq_num_(random_.Rand(1, kMaxInitRtpSeqNumber)),
ulpfec_generator_(ForwardErrorCorrection::CreateFlexfec()),
- rtp_header_extension_map_() {
+ rtp_header_extension_map_(
+ FilterRtpHeaderExtensions(rtp_header_extensions)) {
danilchap 2016/11/10 11:29:23 maybe call this function 'RegisterBweExtensions'
brandtr 2016/11/10 12:00:41 Done.
// This object should not have been instantiated if FlexFEC is disabled.
RTC_DCHECK_GE(payload_type, 0);
RTC_DCHECK_LE(payload_type, 127);
@@ -65,24 +86,6 @@ FlexfecSender::FlexfecSender(
// It's OK to create this object on a different thread/task queue than
// the one used during main operation.
sequence_checker_.Detach();
-
- // Register RTP header extensions for BWE.
- for (const auto& extension : rtp_header_extensions) {
- if (extension.uri == RtpExtension::kTransportSequenceNumberUri) {
- rtp_header_extension_map_.Register(kRtpExtensionTransportSequenceNumber,
- extension.id);
- } else if (extension.uri == RtpExtension::kAbsSendTimeUri) {
- rtp_header_extension_map_.Register(kRtpExtensionAbsoluteSendTime,
- extension.id);
- } else if (extension.uri == RtpExtension::kTimestampOffsetUri) {
- rtp_header_extension_map_.Register(kRtpExtensionTransmissionTimeOffset,
- extension.id);
- } else {
- LOG(LS_WARNING) << "RTP header extension with id: " << extension.id
- << ", uri: " << extension.uri
- << ", is unsupported by FlexfecSender.";
- }
- }
}
FlexfecSender::~FlexfecSender() = default;
@@ -158,8 +161,10 @@ FlexfecSender::GetFecPackets() {
return fec_packets_to_send;
}
+// For FlexFEC, the overhead is the FEC header, as well as the BWE header exts.
danilchap 2016/11/10 11:29:23 may be instead of shortering word remove redundant
brandtr 2016/11/10 12:00:41 Done.
size_t FlexfecSender::MaxPacketOverhead() const {
- return kFlexfecMaxHeaderSize;
+ return kFlexfecMaxHeaderSize +
+ rtp_header_extension_map_.GetTotalLengthInBytes();
danilchap 2016/11/10 11:29:23 really tiny nit: may be sum other way around since
brandtr 2016/11/10 12:00:41 I like that! These things make the code easier to
}
} // namespace webrtc
« no previous file with comments | « webrtc/modules/rtp_rtcp/include/flexfec_sender.h ('k') | webrtc/modules/rtp_rtcp/source/rtp_sender_video.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698