From a881ac0ec9e79874e6b2e97cac5f66ca24958076 Mon Sep 17 00:00:00 2001 From: Seth Hampson Date: Mon, 12 Feb 2018 14:14:39 -0800 Subject: [PATCH] Updated comments for RtpEncodingParameters. Currently with the RtpEncodingParameters the active field is supported per simulcast layer, but max_bitrate_bps and bitrate_priority are just supoorted per rtp sender. Updated the comments to make this more clear and added TODOs with bugs. Bug: webrtc:8819 Change-Id: I130f6dda0896079b5082af58a2693b898d6e22f0 Reviewed-on: https://webrtc-review.googlesource.com/52141 Reviewed-by: Taylor Brandstetter Commit-Queue: Seth Hampson Cr-Commit-Position: refs/heads/master@{#22007} --- api/rtpparameters.h | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/api/rtpparameters.h b/api/rtpparameters.h index 00ae094ffe..d9ac1b6238 100644 --- a/api/rtpparameters.h +++ b/api/rtpparameters.h @@ -362,8 +362,12 @@ struct RtpEncodingParameters { rtc::Optional dtx; // The relative bitrate priority of this encoding. Currently this is - // implemented on the sender level (using the first RtpEncodingParameters - // of the rtp parameters). + // implemented for the entire rtp sender by using the value of the first + // encoding parameter. + // TODO(webrtc.bugs.org/8630): Implement this per encoding parameter. + // Currently there is logic for how bitrate is distributed per simulcast layer + // in the VideoBitrateAllocator. This must be updated to incorporate relative + // bitrate priority. double bitrate_priority = kDefaultBitratePriority; // Indicates the preferred duration of media represented by a packet in @@ -376,7 +380,16 @@ struct RtpEncodingParameters { // If set, this represents the Transport Independent Application Specific // maximum bandwidth defined in RFC3890. If unset, there is no maximum - // bitrate. + // bitrate. Currently this is implemented for the entire rtp sender by using + // the value of the first encoding parameter. + // + // TODO(webrtc.bugs.org/8655): Implement this per encoding parameter. + // Current implementation for a sender: + // The max bitrate is decided by taking the minimum of the first encoding + // parameter's max_bitrate_bps and the max bitrate specified by the sdp with + // the b=AS attribute. In the case of simulcast video, default values are used + // for each simulcast layer, and if there is some bitrate left over from the + // sender's max bitrate then it will roll over into the highest quality layer. // // Just called "maxBitrate" in ORTC spec. // @@ -397,10 +410,12 @@ struct RtpEncodingParameters { // TODO(deadbeef): Not implemented. double scale_framerate_down_by = 1.0; - // For an RtpSender, set to true to cause this encoding to be sent, and false - // for it not to be sent. - // TODO(bugs.webrtc.org/8653): Currently this is implemented per sender. - // Implement per-encoding. + // For an RtpSender, set to true to cause this encoding to be encoded and + // sent, and false for it not to be encoded and sent. This allows control + // across multiple encodings of a sender for turning simulcast layers on and + // off. + // TODO(webrtc.bugs.org/8807): Updating this parameter will trigger an encoder + // reset, but this isn't necessarily required. bool active = true; // Value to use for RID RTP header extension.