Fix rtcp_sender to support sdes with 31 chunk

Fix rtcp_sender to support sdes with 31 chunk
Previosly it supported up to only 15 chunks which is a limit for csrcs in an rtp packet.

BUG=None

Review-Url: https://codereview.webrtc.org/2758533002
Cr-Commit-Position: refs/heads/master@{#17274}
Committed: https://chromium.googlesource.com/external/webrtc/+/74e8df8feb92ec64dda762254b1b2ba0b0445e48

[danilchap, 2017-03-16 13:15:54]

Description was changed from

==========
Fix Sdes to support up to 31 chunks
Previosly it supported up to only 15 chunks which is a limit for csrcs in an rtp
packet.

BUG=None
==========

to

==========
Fix rtcp_sender to support sdes with 31 chunk
Previosly it supported up to only 15 chunks which is a limit for csrcs in an rtp
packet.

BUG=None
==========

[danilchap, 2017-03-16 13:16:18]

danilchap@webrtc.org changed reviewers:
+ nisse@webrtc.org

[nisse-webrtc, 2017-03-16 13:42:44]

https://codereview.webrtc.org/2758533002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_sender.cc (right):

https://codereview.webrtc.org/2758533002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:331: if (csrc_cnames_.size() >=
rtcp::Sdes::kMaxNumberOfChunks - 1)
I'd need a bit more context to understand what's going on. 

I've had a quick look at RFC 3550. So kRtpCsrsSize == 15 is the maximum number
of csrs in an RTP header, i.e., a limit of the number of streams that can be
mixed together in a single RTP packet.

SDES is the source description RTCP message (RFC3550), and there can be at most
31 chunks, of which 30 can be CSRSs (and you can have even more, put into
additional SDES packets). Why would one have a larger number here? We have 50
sources to describe, even though at any point in time the mixer includes only a
few of them?

And what are we using the cname attribute for?

[danilchap, 2017-03-16 13:58:02]

https://codereview.webrtc.org/2758533002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_sender.cc (right):

https://codereview.webrtc.org/2758533002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:331: if (csrc_cnames_.size() >=
rtcp::Sdes::kMaxNumberOfChunks - 1)
On 2017/03/16 13:42:44, nisse-webrtc wrote:
> I'd need a bit more context to understand what's going on. 
> 
> I've had a quick look at RFC 3550. So kRtpCsrsSize == 15 is the maximum number
> of csrs in an RTP header, i.e., a limit of the number of streams that can be
> mixed together in a single RTP packet.
> 
> SDES is the source description RTCP message (RFC3550), and there can be at
most
> 31 chunks, of which 30 can be CSRSs (and you can have even more, put into
> additional SDES packets). Why would one have a larger number here? We have 50
> sources to describe, even though at any point in time the mixer includes only
a
> few of them?
> 
> And what are we using the cname attribute for?
> 
imagine 30 participant meeting with server-side audio mixing.
Sdes list all 30 participiant, and each rtp packet include csrcs of the
currently speaking people (thus it need place for only few csrcs). Using csrcs
client can show who was actually speaking.
cname is a way to identify the participant. same cname might be mapped to
several srcs (e.g. 1 audio, 3 video ssrcs).

[nisse-webrtc, 2017-03-16 14:35:34]

On 2017/03/16 13:58:02, danilchap wrote:
>
https://codereview.webrtc.org/2758533002/diff/20001/webrtc/modules/rtp_rtcp/s...
> File webrtc/modules/rtp_rtcp/source/rtcp_sender.cc (right):
> 
>
https://codereview.webrtc.org/2758533002/diff/20001/webrtc/modules/rtp_rtcp/s...
> webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:331: if (csrc_cnames_.size() >=
> rtcp::Sdes::kMaxNumberOfChunks - 1)
> On 2017/03/16 13:42:44, nisse-webrtc wrote:
> > I'd need a bit more context to understand what's going on. 
> > 
> > I've had a quick look at RFC 3550. So kRtpCsrsSize == 15 is the maximum
number
> > of csrs in an RTP header, i.e., a limit of the number of streams that can be
> > mixed together in a single RTP packet.
> > 
> > SDES is the source description RTCP message (RFC3550), and there can be at
> most
> > 31 chunks, of which 30 can be CSRSs (and you can have even more, put into
> > additional SDES packets). Why would one have a larger number here? We have
50
> > sources to describe, even though at any point in time the mixer includes
only
> a
> > few of them?
> > 
> > And what are we using the cname attribute for?
> > 
> imagine 30 participant meeting with server-side audio mixing.
> Sdes list all 30 participiant, and each rtp packet include csrcs of the
> currently speaking people (thus it need place for only few csrcs). Using csrcs
> client can show who was actually speaking.
> cname is a way to identify the participant. same cname might be mapped to
> several srcs (e.g. 1 audio, 3 video ssrcs).

Thanks for the explanation. lgtm.

[danilchap, 2017-03-16 14:39:38]

The CQ bit was checked by danilchap@webrtc.org

[commit-bot: I haz the power, 2017-03-16 14:39:44]

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, 2017-03-16 15:04:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489675178532570,
"parent_rev": "0937468ceea56dc511e27c39ebc68805317c25ac", "commit_rev":
"74e8df8feb92ec64dda762254b1b2ba0b0445e48"}

[commit-bot: I haz the power, 2017-03-16 15:04:11]

Description was changed from

==========
Fix rtcp_sender to support sdes with 31 chunk
Previosly it supported up to only 15 chunks which is a limit for csrcs in an rtp
packet.

BUG=None
==========

to

==========
Fix rtcp_sender to support sdes with 31 chunk
Previosly it supported up to only 15 chunks which is a limit for csrcs in an rtp
packet.

BUG=None

Review-Url: https://codereview.webrtc.org/2758533002
Cr-Commit-Position: refs/heads/master@{#17274}
Committed:
https://chromium.googlesource.com/external/webrtc/+/74e8df8feb92ec64dda762254...
==========

[commit-bot: I haz the power, 2017-03-16 15:04:12]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/74e8df8feb92ec64dda762254...