Implement FrameMarking header extension support

webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc

 /*
  *  Copyright (c) 2012 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 283 matching lines @@
                                const RTPFragmentationHeader* fragmentation,
                                const RTPVideoHeader* video_header) {
   if (payload_size == 0)
     return false;
 
   // Create header that will be reused in all packets.
   std::unique_ptr<RtpPacketToSend> rtp_header = rtp_sender_->AllocatePacket();
   rtp_header->SetPayloadType(payload_type);
   rtp_header->SetTimestamp(rtp_timestamp);
   rtp_header->set_capture_time_ms(capture_time_ms);
+
+  // Set Frame Marks

[danilchap, 2017/06/28 16:07:07]

Prefer to end comments with a '.'

[sergio.garcia.murillo, 2017/06/29 12:58:58]

Acknowledged.

+  FrameMarks frame_marks;
+  bool frame_marking_enabled = true;
+
+  // Common info
+  frame_marks.startOfFrame = true;
+  frame_marks.endOfFrame = false;
+  frame_marks.independent = (frame_type == kVideoFrameKey);
+
+  // Codec specific
+  switch (video_type) {
+    case kRtpVideoH264:
+      // Nothing to add
+      frame_marks.discardable = false;
+      frame_marks.temporalLayerId = 0;

[danilchap, 2017/06/28 16:07:07]

may be kNoTemporalIdx/kNoSpaitalIdx/kNoTl0PicIdx?

[sergio.garcia.murillo, 2017/06/29 12:58:58]

Done.

+      frame_marks.spatialLayerId = 0;
+      frame_marks.tl0PicIdx = 0;
+      break;
+    case kRtpVideoVp8:
+      frame_marks.discardable = video_header->codecHeader.VP8.nonReference;
+      frame_marks.baseLayerSync = video_header->codecHeader.VP8.layerSync;
+      frame_marks.temporalLayerId = video_header->codecHeader.VP8.temporalIdx;
+      frame_marks.spatialLayerId = 0;

[danilchap, 2017/06/28 16:07:07]

kNoSpatialIdx?

[sergio.garcia.murillo, 2017/06/29 12:58:58]

Done.

+      frame_marks.tl0PicIdx = video_header->codecHeader.VP8.tl0PicIdx;
+      break;
+    case kRtpVideoVp9:
+      frame_marks.discardable = false;
+      frame_marks.temporalLayerId = video_header->codecHeader.VP9.temporal_idx;
+      frame_marks.spatialLayerId = 0;
+      frame_marks.tl0PicIdx = video_header->codecHeader.VP9.tl0_pic_idx;
+      // TODO: This will need to be changed to support VP9 SVC, but videoheader

[danilchap, 2017/06/28 16:07:07]

add bugnumber or username to TODO()
https://google.github.io/styleguide/cppguide.html#TODO_Comments

+      // is set per-frame, not per packet, so we can't have access to this

[danilchap, 2017/06/28 16:07:06]

this is same for VP8, why is it a problem? SendVideo called per layer frame, not
per super frame for VP9.
same as with VP8 all fields except start_of_frame/end_of_frame can be set in
advance, those two fields need modification per-packet anyway.

[sergio.garcia.murillo, 2017/06/29 12:58:58]

No, because in VP8 there is no support for spatial scalability, so all packets
in the frame have same spatial, temporal and tl0picid. 

On VP9 the spatial layer id, changes between packets on the same frame.

[danilchap, 2017/06/29 13:47:26]

SendVideo takes layer frame, not super frame for VP9, i.e. all packets here use
same spatial layer id.

[sergio.garcia.murillo, 2017/06/30 10:09:23]

Perfect then.

Side note, wouldn't it still be a problem with:
     codecHeader.VP9.beginning_of_frame
and 
     codecHeader.VP9.end_of_frame values

They are per packet and not per layer frame (although not relevant for this CL)

[danilchap, 2017/06/30 12:53:25]

same struct is used for per-packet vp9 header (when parsed).
These two fields expected to be unused in this function.

+      // values.
+      // Also, small modifications to the extensions will be needed to not
+      // change size of the extension between sid:0 and sid:1
+//    frame_marks.startOfFrame =
+//        video_header->codecHeader.VP9.beginning_of_frame;
+//    frame_marks.endOfFrame = video_header->codecHeader.VP9.end_of_frame;
+//    frame_marks.spatialLayerId = video_header->codecHeader.VP9.spatial_idx;
+      break;
+    default:
+      // Do not use frame marking
+      frame_marking_enabled = false;
+  }
+  // Only add frame marking for known codecs
+  if (frame_marking_enabled)
+      // Add extension header for frame marking
+      rtp_header->SetExtension<FrameMarking>(frame_marks);
+
   auto last_packet = rtc::MakeUnique<RtpPacketToSend>(*rtp_header);
 
   size_t fec_packet_overhead;
   bool red_enabled;
   int32_t retransmission_settings;
   {
     rtc::CritScope cs(&crit_);
     // According to
     // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/
     // ts_126114v120700p.pdf Section 7.4.5:
@@ skipping 56 matching lines @@
   const RTPFragmentationHeader* frag =
       (video_type == kRtpVideoVp8) ? nullptr : fragmentation;
   size_t num_packets =
       packetizer->SetPayloadData(payload_data, payload_size, frag);
 
   if (num_packets == 0)
     return false;
 
   bool first_frame = first_frame_sent_();
   for (size_t i = 0; i < num_packets; ++i) {
+    bool first = (i == 0);
     bool last = (i + 1) == num_packets;
     auto packet = last ? std::move(last_packet)
                        : rtc::MakeUnique<RtpPacketToSend>(*rtp_header);
     if (!packetizer->NextPacket(packet.get()))
       return false;
     RTC_DCHECK_LE(packet->payload_size(),
                   last ? max_data_payload_length - last_packet_reduction_len
                        : max_data_payload_length);
+
+    // Update start and end marks

[danilchap, 2017/06/28 16:07:07]

may be put this block inside if (frame_marking_enabled) 

[sergio.garcia.murillo, 2017/06/29 12:58:58]

Done.

+    frame_marks.startOfFrame = first;
+    frame_marks.endOfFrame = last;
+
+    // Only add frame marking for known codecs
+    if (frame_marking_enabled)
+      // Update extension header for frame marking
+      packet->SetExtension<FrameMarking>(frame_marks);
+
     if (!rtp_sender_->AssignSequenceNumber(packet.get()))
       return false;
 
     const bool protect_packet =
         (packetizer->GetProtectionType() == kProtectedPacket);
     if (flexfec_enabled()) {
       // TODO(brandtr): Remove the FlexFEC code path when FlexfecSender
       // is wired up to PacedSender instead.
       SendVideoPacketWithFlexfec(std::move(packet), storage, protect_packet);
     } else if (red_enabled) {
@@ skipping 34 matching lines @@
   rtc::CritScope cs(&crit_);
   return retransmission_settings_;
 }
 
 void RTPSenderVideo::SetSelectiveRetransmissions(uint8_t settings) {
   rtc::CritScope cs(&crit_);
   retransmission_settings_ = settings;
 }
 
 }  // namespace webrtc