Use RtcpPacket to send APP in RtcpSender

Use RtcpPacket to send APP in RtcpSender

BUG=webrtc:2450
R=asapersson@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/521875a9a48256ffa865f7c4a635f260358fa4a7

[sprang_webrtc, 2015-08-21 15:20:00]

sprang@webrtc.org changed reviewers:
+ asapersson@webrtc.org

[åsapersson, 2015-08-25 16:10:37]

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

https://codereview.webrtc.org/1311453002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:785: // sanity
I think it is possible to send an app packet without app_data.

https://codereview.webrtc.org/1311453002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:797: if (ctx->position + 12 +
app_length_ >= IP_PACKET_SIZE) {
checked within the rtcp packet class

[sprang_webrtc, 2015-08-31 12:52:38]

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

https://codereview.webrtc.org/1311453002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:785: // sanity
On 2015/08/25 16:10:36, åsapersson wrote:
> I think it is possible to send an app packet without app_data.

OK. Removed check and added test case.

https://codereview.webrtc.org/1311453002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:797: if (ctx->position + 12 +
app_length_ >= IP_PACKET_SIZE) {
On 2015/08/25 16:10:36, åsapersson wrote:
> checked within the rtcp packet class

Done.

[åsapersson, 2015-08-31 14:32:52]

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

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:788: app.WithName(app_name_);
Note that there is an assert in WithData.

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

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc:394: const uint32_t name
= 0x6E616D65;
kName

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc:403: EXPECT_EQ(name,
parser()->app()->Name());
maybe check that there is no app_item

[sprang_webrtc, 2015-08-31 15:29:29]

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

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:788: app.WithName(app_name_);
On 2015/08/31 14:32:52, åsapersson wrote:
> Note that there is an assert in WithData.

That's ok, in RTCPSender::SetApplicationSpecificData() with lenght = 0, the
buffer will be reset to a zero-length array, so the assert won't trigger:
    app_data_.reset(new uint8_t[length]);

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

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc:394: const uint32_t name
= 0x6E616D65;
On 2015/08/31 14:32:52, åsapersson wrote:
> kName

Done.

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc:403: EXPECT_EQ(name,
parser()->app()->Name());
On 2015/08/31 14:32:52, åsapersson wrote:
> maybe check that there is no app_item

Done.

[åsapersson, 2015-08-31 16:48:06]

lgtm

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

https://codereview.webrtc.org/1311453002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_sender.cc:788: app.WithName(app_name_);
On 2015/08/31 15:29:28, språng wrote:
> On 2015/08/31 14:32:52, åsapersson wrote:
> > Note that there is an assert in WithData.
> 
> That's ok, in RTCPSender::SetApplicationSpecificData() with lenght = 0, the
> buffer will be reset to a zero-length array, so the assert won't trigger:
>     app_data_.reset(new uint8_t[length]);

Acknowledged.

[sprang_webrtc, 2015-08-31 18:50:10]

The CQ bit was checked by sprang@webrtc.org

[commit-bot: I haz the power, 2015-08-31 18:50:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1311453002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1311453002/40001

[commit-bot: I haz the power, 2015-08-31 20:50:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-31 20:50:38]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[sprang_webrtc, 2015-09-01 08:11:42]

Committed patchset #3 (id:40001) manually as
521875a9a48256ffa865f7c4a635f260358fa4a7 (presubmit successful).