Cast: Add capabity to send Receiver and Sender log messages over RTCP.

Cast: Add capabity to send Receiver and Sender log messages over RTCP.

This cl contains the generation of the RTCP packets containing the log
messages, no code (more than this unittest) trigger the send.
Additionally the receiver is not wired up either. If a receiver receives
a log message today it would be ignored.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238533

[pwestin, 2013-11-19 21:43:46]

Please review.

[pwestin, 2013-11-21 16:14:55]

+mikhal

[mikhal, 2013-11-21 16:43:20]

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
File media/cast/rtcp/rtcp_sender.cc (right):

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender.cc:49: NOTREACHED();
The receiver also includes jitter and packet loss calculation - that's already
sent to sender via a rtcp, but the same is true also for kAskSent.

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender.cc:70: remaining_space -= 12;
use consts, here and elsewhere.

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
File media/cast/rtcp/rtcp_sender_unittest.cc (right):

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender_unittest.cc:231: //TODO
what? who?

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_util...
File media/cast/rtcp/rtcp_utility.h (right):

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_util...
media/cast/rtcp/rtcp_utility.h:29: static const size_t kRtcpMaxCastLossFields =
100;
why 100?

[pwestin, 2013-11-22 18:50:47]

PTAL

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
File media/cast/rtcp/rtcp_sender.cc (right):

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender.cc:49: NOTREACHED();
On 2013/11/21 16:43:20, mikhal wrote:
> The receiver also includes jitter and packet loss calculation - that's already
> sent to sender via a rtcp, but the same is true also for kAskSent.

The difference is that this is intended for timing; hence when the ACK as sent.
When jitter or packet loss was calculated is not as interested.

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender.cc:70: remaining_space -= 12;
On 2013/11/21 16:43:20, mikhal wrote:
> use consts, here and elsewhere. 

Done.

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
File media/cast/rtcp/rtcp_sender_unittest.cc (right):

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender_unittest.cc:231: //TODO
On 2013/11/21 16:43:20, mikhal wrote:
> what? who?

ops

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_send...
media/cast/rtcp/rtcp_sender_unittest.cc:231: //TODO
On 2013/11/21 16:43:20, mikhal wrote:
> what? who?

Done.

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_util...
File media/cast/rtcp/rtcp_utility.h (right):

https://codereview.chromium.org/74613004/diff/30001/media/cast/rtcp/rtcp_util...
media/cast/rtcp/rtcp_utility.h:29: static const size_t kRtcpMaxCastLossFields =
100;
On 2013/11/21 16:43:20, mikhal wrote:
> why 100?

I just moved this to the h file to be able to re-use it. Did not change the
value. I guess that we did not want to NACK more than  100 packets at once.

[mikhal, 2013-11-22 21:39:33]

lgtm+ nit

https://chromiumcodereview.appspot.com/74613004/diff/110001/media/cast/rtcp/r...
File media/cast/rtcp/rtcp.cc (right):

https://chromiumcodereview.appspot.com/74613004/diff/110001/media/cast/rtcp/r...
media/cast/rtcp/rtcp.cc:56: : rtp_timestamp(timestamp) {}
just noticed: here too: rtp_timestamp_

[commit-bot: I haz the power, 2013-11-22 21:56:27]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[commit-bot: I haz the power, 2013-11-22 22:13:28]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[pwestin, 2013-11-22 22:54:53]

added mikhal @ chromium

[mikhal1, 2013-11-22 22:55:33]

lgtm

[commit-bot: I haz the power, 2013-11-22 22:56:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pwestin@google.com/74613004/250001

[commit-bot: I haz the power, 2013-11-22 22:56:42]

Failed to apply patch for media/cast/rtcp/rtcp.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file media/cast/rtcp/rtcp.cc
  Hunk #2 succeeded at 179 with fuzz 1 (offset 2 lines).
  Hunk #3 FAILED at 229.
  Hunk #4 succeeded at 253 (offset 7 lines).
  1 out of 4 hunks FAILED -- saving rejects to file media/cast/rtcp/rtcp.cc.rej

Patch:       media/cast/rtcp/rtcp.cc
Index: media/cast/rtcp/rtcp.cc
diff --git a/media/cast/rtcp/rtcp.cc b/media/cast/rtcp/rtcp.cc
index
e848f2bccbdacf4a42610ae481742c1a3b88f9ca..5fe691ad518f7bd7624d647ea5ada3aaae686724
100644
--- a/media/cast/rtcp/rtcp.cc
+++ b/media/cast/rtcp/rtcp.cc
@@ -52,7 +52,9 @@ RtcpNackMessage::~RtcpNackMessage() {}
 RtcpRembMessage::RtcpRembMessage() {}
 RtcpRembMessage::~RtcpRembMessage() {}
 
-RtcpReceiverFrameLogMessage::RtcpReceiverFrameLogMessage() {}
+RtcpReceiverFrameLogMessage::RtcpReceiverFrameLogMessage(uint32 timestamp)
+    : rtp_timestamp_(timestamp) {}
+
 RtcpReceiverFrameLogMessage::~RtcpReceiverFrameLogMessage() {}
 
 class LocalRtcpReceiverFeedback : public RtcpReceiverFeedback {
@@ -175,7 +177,7 @@ void Rtcp::IncomingRtcpPacket(const uint8* rtcp_buffer,
size_t length) {
 }
 
 void Rtcp::SendRtcpFromRtpReceiver(const RtcpCastMessage* cast_message,
-                                   const RtcpReceiverLogMessage* receiver_log)
{
+                                   RtcpReceiverLogMessage* receiver_log) {
   uint32 packet_type_flags = 0;
 
   base::TimeTicks now = clock_->NowTicks();
@@ -227,16 +229,15 @@ void Rtcp::SendRtcpFromRtpReceiver(const RtcpCastMessage*
cast_message,
                                         receiver_log);
 }
 
-void Rtcp::SendRtcpFromRtpSender(
-    const RtcpSenderLogMessage* sender_log_message) {
+void Rtcp::SendRtcpFromRtpSender(RtcpSenderLogMessage* sender_log_message) {
   uint32 packet_type_flags = RtcpSender::kRtcpSr;
   base::TimeTicks now = clock_->NowTicks();
 
-  RtcpSenderInfo sender_info;
-  RtcpDlrrReportBlock dlrr;
-
-  if (sender_log_message) packet_type_flags |= RtcpSender::kRtcpSenderLog;
+  if (sender_log_message) {
+    packet_type_flags |= RtcpSender::kRtcpSenderLog;
+  }
 
+  RtcpSenderInfo sender_info;
   if (rtp_sender_statistics_) {
     rtp_sender_statistics_->GetStatistics(now, &sender_info);
   } else {
@@ -244,6 +245,7 @@ void Rtcp::SendRtcpFromRtpSender(
   }
   SaveLastSentNtpTime(now, sender_info.ntp_seconds, sender_info.ntp_fraction);
 
+  RtcpDlrrReportBlock dlrr;
   if (!time_last_report_received_.is_null()) {
     packet_type_flags |= RtcpSender::kRtcpDlrr;
     dlrr.last_rr = last_report_received_;

[commit-bot: I haz the power, 2013-11-22 23:51:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pwestin@google.com/74613004/190003

[commit-bot: I haz the power, 2013-11-23 01:29:06]

Retried try job too often on win_x64_rel for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[commit-bot: I haz the power, 2013-11-23 03:00:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pwestin@google.com/74613004/190003

[commit-bot: I haz the power, 2013-11-23 03:00:26]

Failed to apply patch for media/cast/rtcp/rtcp.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file media/cast/rtcp/rtcp.cc
  Hunk #1 FAILED at 52.
  Hunk #2 succeeded at 179 (offset 2 lines).
  Hunk #3 succeeded at 237 (offset 2 lines).
  Hunk #4 succeeded at 253 (offset 2 lines).
  1 out of 4 hunks FAILED -- saving rejects to file media/cast/rtcp/rtcp.cc.rej

Patch:       media/cast/rtcp/rtcp.cc
Index: media/cast/rtcp/rtcp.cc
diff --git a/media/cast/rtcp/rtcp.cc b/media/cast/rtcp/rtcp.cc
index
a21bcb07ecd64a6c761e4f8228a9922c0e2706d2..3a36280394cf61806b087ad1423bb0b0c7cb2060
100644
--- a/media/cast/rtcp/rtcp.cc
+++ b/media/cast/rtcp/rtcp.cc
@@ -52,7 +52,9 @@ RtcpNackMessage::~RtcpNackMessage() {}
 RtcpRembMessage::RtcpRembMessage() {}
 RtcpRembMessage::~RtcpRembMessage() {}
 
-RtcpReceiverFrameLogMessage::RtcpReceiverFrameLogMessage() {}
+RtcpReceiverFrameLogMessage::RtcpReceiverFrameLogMessage(uint32 timestamp)
+    : rtp_timestamp_(timestamp) {}
+
 RtcpReceiverFrameLogMessage::~RtcpReceiverFrameLogMessage() {}
 
 class LocalRtcpReceiverFeedback : public RtcpReceiverFeedback {
@@ -177,7 +179,7 @@ void Rtcp::IncomingRtcpPacket(const uint8* rtcp_buffer,
size_t length) {
 }
 
 void Rtcp::SendRtcpFromRtpReceiver(const RtcpCastMessage* cast_message,
-                                   const RtcpReceiverLogMessage* receiver_log)
{
+                                   RtcpReceiverLogMessage* receiver_log) {
   uint32 packet_type_flags = 0;
 
   base::TimeTicks now = cast_environment_->Clock()->NowTicks();
@@ -235,15 +237,15 @@ void Rtcp::SendRtcpFromRtpReceiver(const RtcpCastMessage*
cast_message,
 }
 
 void Rtcp::SendRtcpFromRtpSender(
-    const RtcpSenderLogMessage* sender_log_message) {
+    RtcpSenderLogMessage* sender_log_message) {
   uint32 packet_type_flags = RtcpSender::kRtcpSr;
   base::TimeTicks now = cast_environment_->Clock()->NowTicks();
 
-  RtcpSenderInfo sender_info;
-  RtcpDlrrReportBlock dlrr;
-
-  if (sender_log_message) packet_type_flags |= RtcpSender::kRtcpSenderLog;
+  if (sender_log_message) {
+    packet_type_flags |= RtcpSender::kRtcpSenderLog;
+  }
 
+  RtcpSenderInfo sender_info;
   if (rtp_sender_statistics_) {
     rtp_sender_statistics_->GetStatistics(now, &sender_info);
   } else {
@@ -251,6 +253,7 @@ void Rtcp::SendRtcpFromRtpSender(
   }
   SaveLastSentNtpTime(now, sender_info.ntp_seconds, sender_info.ntp_fraction);
 
+  RtcpDlrrReportBlock dlrr;
   if (!time_last_report_received_.is_null()) {
     packet_type_flags |= RtcpSender::kRtcpDlrr;
     dlrr.last_rr = last_report_received_;

[commit-bot: I haz the power, 2013-12-03 19:25:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pwestin@google.com/74613004/670001

[commit-bot: I haz the power, 2013-12-03 20:47:38]

Retried try job too often on win_x64_rel for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[commit-bot: I haz the power, 2013-12-03 21:19:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pwestin@google.com/74613004/690001

[commit-bot: I haz the power, 2013-12-03 23:37:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pwestin@google.com/74613004/690001

[commit-bot: I haz the power, 2013-12-04 02:52:06]

Change committed as 238533