Reject XR TargetBitrate items with unsupported layer indices

Reject XR TargetBitrate items with unsupported layer indices

Specifically, reject any bitrate allocated for a layer not representable
by the BitrateAllocation struct.

BUG=chromium:671312
Committed: https://crrev.com/6d314c7a88f9e7e666fb89bbadc6dda22811329d
Cr-Commit-Position: refs/heads/master@{#15447}

[sprang_webrtc, 2016-12-06 11:06:18]

sprang@webrtc.org changed reviewers:
+ danilchap@webrtc.org, stefan@webrtc.org

[sprang_webrtc, 2016-12-06 11:06:19]

danil, ptal
+stefan as owner for common_types

[danilchap, 2016-12-06 13:12:56]

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc (right):

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:740: if (item.spatial_layer >=
kMaxSpatialLayers ||
may be #include "webrtc/common_types.h" directly for
kMaxSpatialLayers/kMaxTemporalStreams.

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc (right):

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1306:
bitrate.AddTargetBitrate(0, kMaxTemporalStreams - 1, 20000);
rtcp::TargetBitrate has different limits for temporal/spatial layers (16 for
each instead of 4/5)
you can set them directly, no need to temper with raw packet.

[sprang_webrtc, 2016-12-06 13:22:41]

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc (right):

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:740: if (item.spatial_layer >=
kMaxSpatialLayers ||
On 2016/12/06 13:12:56, danilchap wrote:
> may be #include "webrtc/common_types.h" directly for
> kMaxSpatialLayers/kMaxTemporalStreams.

Done.

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc (right):

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1306:
bitrate.AddTargetBitrate(0, kMaxTemporalStreams - 1, 20000);
On 2016/12/06 13:12:56, danilchap wrote:
> rtcp::TargetBitrate has different limits for temporal/spatial layers (16 for
> each instead of 4/5)
> you can set them directly, no need to temper with raw packet.

You're right. Done.

[sprang_webrtc, 2016-12-06 13:22:41]

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc (right):

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc:740: if (item.spatial_layer >=
kMaxSpatialLayers ||
On 2016/12/06 13:12:56, danilchap wrote:
> may be #include "webrtc/common_types.h" directly for
> kMaxSpatialLayers/kMaxTemporalStreams.

Done.

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc (right):

https://codereview.webrtc.org/2549233005/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1306:
bitrate.AddTargetBitrate(0, kMaxTemporalStreams - 1, 20000);
On 2016/12/06 13:12:56, danilchap wrote:
> rtcp::TargetBitrate has different limits for temporal/spatial layers (16 for
> each instead of 4/5)
> you can set them directly, no need to temper with raw packet.

You're right. Done.

[danilchap, 2016-12-06 13:28:50]

lgtm

[stefan-webrtc, 2016-12-06 13:51:38]

lgtm

[sprang_webrtc, 2016-12-06 13:52:34]

The CQ bit was checked by sprang@webrtc.org

[commit-bot: I haz the power, 2016-12-06 13:52:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-12-06 14:08:53]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481032354437280,
"parent_rev": "d823f25f68c827f55586779aca2c20c58f492277", "commit_rev":
"0b312711bebdc703ebf1cb0776caef973d5205d7"}

[commit-bot: I haz the power, 2016-12-06 14:08:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-06 14:09:03]

Description was changed from

==========
Reject XR TargetBitrate items with unsupported layer indices

Specifically, reject any bitrate allocated for a layer not representable
by the BitrateAllocation struct.

BUG=chromium:671312
==========

to

==========
Reject XR TargetBitrate items with unsupported layer indices

Specifically, reject any bitrate allocated for a layer not representable
by the BitrateAllocation struct.

BUG=chromium:671312

Committed: https://crrev.com/6d314c7a88f9e7e666fb89bbadc6dda22811329d
Cr-Commit-Position: refs/heads/master@{#15447}
==========

[commit-bot: I haz the power, 2016-12-06 14:09:03]

Patchset 2 (id:??) landed as
https://crrev.com/6d314c7a88f9e7e666fb89bbadc6dda22811329d
Cr-Commit-Position: refs/heads/master@{#15447}