Add a flags field to video timing extension.

webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 230 matching lines @@
 
 bool VideoContentTypeExtension::Write(uint8_t* data,
                                       VideoContentType content_type) {
   data[0] = static_cast<uint8_t>(content_type);
   return true;
 }
 
 // Video Timing.
 // 6 timestamps in milliseconds counted from capture time stored in rtp header:
 // encode start/finish, packetization complete, pacer exit and reserved for
-// modification by the network modification.
+// modification by the network modification. |flags| is a bitmask and has the
+// following allowed values:
+// 0 = Valid data, but no flags available (backwards compatibility)
+// 1 = Frame marked as timing frame due to cyclic timer.
+// 2 = Frame marked as timing frame due to size being outside limit.
+// 255 = Invalid. The whole timing frame extension should be ignored.
+//
 //    0                   1                   2                   3
 //    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2
 //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//   |  ID   | len=11|  encode start ms delta          | encode finish |
+//   |  ID   | len=12|  encode start ms delta          | encode finish |
 //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 //   | ms delta      |  packetizer finish ms delta     | pacer exit    |
 //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 //   | ms delta      |  network timestamp ms delta     | network2 time-|
 //   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-//   | stamp ms delta|
-//   +-+-+-+-+-+-+-+-+
+//   | stamp ms delta|     flags     |

[danilchap, 2017/08/11 13:51:16]

did you consider putting flags first?
it would make this full comment nicer (times will be better aligned), will make
more logical adding more kind of timestamps later.

On minus side - will slightly complicate Parsing previous version.
(though that can be mitigated with an extra local variable:
const uint8* timestamps = data.data();
if (is_old_protocol) --timestamps;

old client will discard extension size != 11 anyway.

[sprang_webrtc, 2017/08/11 15:06:33]

I did, mainly abstained because of the Write method that takes a single field
and offset. But since we're always sending with the new format, let's go ahead
and change it.

Also, I think it's technically undefined behavior to move pointer outside your
memory area and then move it back again. But maybe we're ok since we actually
own the memory for the whole rtp header. Still, doing offset subtraction
instead.

+//   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 
 constexpr RTPExtensionType VideoTimingExtension::kId;
 constexpr uint8_t VideoTimingExtension::kValueSizeBytes;
 constexpr const char VideoTimingExtension::kUri[];
 
 bool VideoTimingExtension::Parse(rtc::ArrayView<const uint8_t> data,
                                  VideoSendTiming* timing) {
   RTC_DCHECK(timing);
-  if (data.size() != kValueSizeBytes)
+  // TODO(sprang): Deprecate support for old wire format.
+  const bool is_old_protocol = data.size() == (kValueSizeBytes - 1);
+
+  if (!(data.size() == kValueSizeBytes || is_old_protocol))
     return false;
-  timing->encode_start_delta_ms =
-      ByteReader<uint16_t>::ReadBigEndian(data.data());
+  timing->encode_start_delta_ms = ByteReader<uint16_t>::ReadBigEndian(
+      data.data() + VideoSendTiming::kEncodeStartDeltaOffset);
   timing->encode_finish_delta_ms = ByteReader<uint16_t>::ReadBigEndian(
-      data.data() + 2 * VideoSendTiming::kEncodeFinishDeltaIdx);
+      data.data() + VideoSendTiming::kEncodeFinishDeltaOffset);
   timing->packetization_finish_delta_ms = ByteReader<uint16_t>::ReadBigEndian(
-      data.data() + 2 * VideoSendTiming::kPacketizationFinishDeltaIdx);
+      data.data() + VideoSendTiming::kPacketizationFinishDeltaOffset);
   timing->pacer_exit_delta_ms = ByteReader<uint16_t>::ReadBigEndian(
-      data.data() + 2 * VideoSendTiming::kPacerExitDeltaIdx);
+      data.data() + VideoSendTiming::kPacerExitDeltaOffset);
   timing->network_timstamp_delta_ms = ByteReader<uint16_t>::ReadBigEndian(
-      data.data() + 2 * VideoSendTiming::kNetworkTimestampDeltaIdx);
+      data.data() + VideoSendTiming::kNetworkTimestampDeltaOffset);
   timing->network2_timstamp_delta_ms = ByteReader<uint16_t>::ReadBigEndian(
-      data.data() + 2 * VideoSendTiming::kNetwork2TimestampDeltaIdx);
-  timing->is_timing_frame = true;
+      data.data() + VideoSendTiming::kNetwork2TimestampDeltaOffset);
+  timing->flags = is_old_protocol
+                      ? 0
+                      : ByteReader<uint8_t>::ReadBigEndian(
+                            data.data() + VideoSendTiming::kFLagsOffset);
   return true;
 }
 
 bool VideoTimingExtension::Write(uint8_t* data, const VideoSendTiming& timing) {
-  ByteWriter<uint16_t>::WriteBigEndian(data, timing.encode_start_delta_ms);
   ByteWriter<uint16_t>::WriteBigEndian(
-      data + 2 * VideoSendTiming::kEncodeFinishDeltaIdx,
+      data + VideoSendTiming::kEncodeStartDeltaOffset,
+      timing.encode_start_delta_ms);
+  ByteWriter<uint16_t>::WriteBigEndian(
+      data + VideoSendTiming::kEncodeFinishDeltaOffset,
       timing.encode_finish_delta_ms);
   ByteWriter<uint16_t>::WriteBigEndian(
-      data + 2 * VideoSendTiming::kPacketizationFinishDeltaIdx,
+      data + VideoSendTiming::kPacketizationFinishDeltaOffset,
       timing.packetization_finish_delta_ms);
   ByteWriter<uint16_t>::WriteBigEndian(
-      data + 2 * VideoSendTiming::kPacerExitDeltaIdx,
+      data + VideoSendTiming::kPacerExitDeltaOffset,
       timing.pacer_exit_delta_ms);
   ByteWriter<uint16_t>::WriteBigEndian(
-      data + 2 * VideoSendTiming::kNetworkTimestampDeltaIdx, 0);  // reserved
+      data + VideoSendTiming::kNetworkTimestampDeltaOffset, 0);  // reserved
   ByteWriter<uint16_t>::WriteBigEndian(
-      data + 2 * VideoSendTiming::kNetwork2TimestampDeltaIdx, 0);  // reserved
+      data + VideoSendTiming::kNetwork2TimestampDeltaOffset, 0);  // reserved
+  ByteWriter<uint8_t>::WriteBigEndian(data + VideoSendTiming::kFLagsOffset,
+                                      timing.flags);
   return true;
 }
 
 bool VideoTimingExtension::Write(uint8_t* data,
                                  uint16_t time_delta_ms,
-                                 uint8_t idx) {
-  RTC_DCHECK_LT(idx, 6);
-  ByteWriter<uint16_t>::WriteBigEndian(data + 2 * idx, time_delta_ms);
+                                 uint8_t offset) {
+  RTC_DCHECK_LT(offset, kValueSizeBytes - sizeof(uint16_t));
+  ByteWriter<uint16_t>::WriteBigEndian(data + offset, time_delta_ms);
   return true;
 }
 
 bool BaseRtpStringExtension::Parse(rtc::ArrayView<const uint8_t> data,
                                    StringRtpHeaderExtension* str) {
   if (data.empty() || data[0] == 0)  // Valid string extension can't be empty.
     return false;
   str->Set(data);
   RTC_DCHECK(!str->empty());
   return true;
@@ skipping 31 matching lines @@
 constexpr RTPExtensionType RtpStreamId::kId;
 constexpr const char RtpStreamId::kUri[];
 
 constexpr RTPExtensionType RepairedRtpStreamId::kId;
 constexpr const char RepairedRtpStreamId::kUri[];
 
 constexpr RTPExtensionType RtpMid::kId;
 constexpr const char RtpMid::kUri[];
 
 }  // namespace webrtc