Bug 985254: Cleaup H264 packetization and jitter buffer r=pkerr

media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc

@@ -277,12 +277,23 @@ int32_t RTPReceiverVideo::ReceiveH264Codec(WebRtcRTPHeader* rtp_header,
     h264_header->single_nalu        = true;
 
     // WebRtcRTPHeader
-    if (nal_type == RtpFormatH264::kH264NALU_SPS ||
-        nal_type == RtpFormatH264::kH264NALU_PPS ||
-        nal_type == RtpFormatH264::kH264NALU_IDR) {
-      rtp_header->frameType = kVideoFrameKey; // not really....
-    } else {
-      rtp_header->frameType = kVideoFrameDelta;
+    switch (nal_type) {
+      // TODO(jesup): Evil hack.  The jitter buffer *really* doesn't like
+      // "frames" to have the same timestamps.  NOTE: this only works
+      // for SPS/PPS/IDR, not for PPS/SPS/IDR.  Keep this until all issues
+      // are resolved in the jitter buffer
+      case RtpFormatH264::kH264NALU_SPS:
+        rtp_header->header.timestamp -= 10;
+        // fall through
+      case RtpFormatH264::kH264NALU_PPS:
+        rtp_header->header.timestamp -= 10;
+        // fall through
+      case RtpFormatH264::kH264NALU_IDR:
+        rtp_header->frameType = kVideoFrameKey;
+        break;
+      default:
+        rtp_header->frameType = kVideoFrameDelta;
+        break;
     }
   }
 

media/webrtc/trunk/webrtc/modules/video_coding/main/source/jitter_buffer.cc

@@ -47,8 +47,45 @@ void FrameList::InsertFrame(VCMFrameBuffer* frame) {
   insert(rbegin().base(), FrameListPair(frame->TimeStamp(), frame));
 }
 
-VCMFrameBuffer* FrameList::FindFrame(uint32_t timestamp) const {
+// Find a Frame which (may) include seq_num
+// Note: if we don't have an end for the frame yet AND there are multiple Frames
+// with the same timestamp being input, in theory you can get packets
+// for a later Frame mixed with an earlier one where there's a reordering.
+// e.g. for <frame 1: 1 2 3> <frame 2: 4 5 6> and we receive
+//          1 2 4 3 5 6
+// or       4 1 2 3 5 6
+// we'll return <frame 1> for packet 4, and at some point it needs to move to
+// <frame 2>.  You can't key off isFirstPacket or kNaluStart because the OOO packet
+// may be 5.
+
+// This can be done by re-characterizing 4 when <frame 1> becomes complete
+// and we find it doesn't include 4.  Perhaps a better abstraction would be
+// to keep the packets in a single sorted list (per timestamp or not,
+// doesn't really matter), and then on insertion look to see if it's in a
+// complete unit (kNaluComplete or kNaluStart ... kNaluEnd sequence), and
+// remove the set *then*.
+//
+// If we instead limit multiple frames with the same timestamp to
+// kNaluComplete (single-packet) frames, it's simpler.  You do need to be
+// careful to pull off Frames only if they're contiguous in sequence number
+// to the previous frame, but that's normal since you can get 4 5 6 1 2 3
+// Note that you have to be careful reordering still:
+// <frame 1: 1> <frame 2: 2 3 4>
+// and arrival 2 1 3 4
+// means you must not match the frame created for 2 when 1 comes in
+
+VCMFrameBuffer* FrameList::FindFrame(uint16_t seq_num, uint32_t timestamp) const {
   FrameList::const_iterator it = find(timestamp);
+  // TODO(jesup): use seq_num to do the fancier version above, or
+  // rearchitect per above to keep a single list and pull out Frames as they
+  // become complete (or decodable).
+
+  // Simple version: Skip already-complete frames
+  // Note: higher level must deal with the 2 1 3 4 case above by not calling
+  // this for single-nal packets
+  while (it != end() && it->second->GetState() == kStateComplete) {
+    it++;
+  }
   if (it == end())
     return NULL;
   return it->second;
@@ -597,12 +634,21 @@ VCMFrameBufferEnum VCMJitterBuffer::GetFrame(const VCMPacket& packet,
   }
   num_consecutive_old_packets_ = 0;
 
-  *frame = incomplete_frames_.FindFrame(packet.timestamp);
-  if (*frame)
-    return kNoError;
-  *frame = decodable_frames_.FindFrame(packet.timestamp);
-  if (*frame)
-    return kNoError;
+  // Handle the 2 1 3 4 case (where 2 3 4 are frame 2 with the timestamp)
+  // from above, for complete nalu's (single-nalus) only.
+
+  // TODO(jesup) To handle a sequence of fragmented nalus which all are
+  // slices of the same lower-case frame (timestamp), the more complete
+  // solution for FindFrame that uses the seqNum and can move packets
+  // between sessions would be needed.
+  if (packet.completeNALU != kNaluComplete) {
+    *frame = incomplete_frames_.FindFrame(packet.seqNum, packet.timestamp);
+    if (*frame)
+      return kNoError;
+    *frame = decodable_frames_.FindFrame(packet.seqNum, packet.timestamp);
+    if (*frame && (*frame)->GetState() != kStateComplete)
+      return kNoError;
+  }
 
   // No match, return empty frame.
   *frame = GetEmptyFrame();

media/webrtc/trunk/webrtc/modules/video_coding/main/source/jitter_buffer.h

@@ -63,7 +63,7 @@ class FrameList
     : public std::map<uint32_t, VCMFrameBuffer*, TimestampLessThan> {
  public:
   void InsertFrame(VCMFrameBuffer* frame);
-  VCMFrameBuffer* FindFrame(uint32_t timestamp) const;
+  VCMFrameBuffer* FindFrame(uint16_t seq_num, uint32_t timestamp) const;
   VCMFrameBuffer* PopFrame(uint32_t timestamp);
   VCMFrameBuffer* Front() const;
   VCMFrameBuffer* Back() const;

media/webrtc/trunk/webrtc/modules/video_coding/main/source/packet.cc

@@ -111,17 +111,13 @@ void VCMPacket::CopyCodecSpecifics(const RTPVideoHeader& videoHeader) {
     }
     case kRtpVideoH264: {
       uint8_t nal_type = videoHeader.codecHeader.H264.nalu_header & RtpFormatH264::kH264NAL_TypeMask;
-      if (videoHeader.codecHeader.H264.single_nalu) {
-        isFirstPacket   = true;
-        markerBit       = true;
-      }
+      isFirstPacket = videoHeader.isFirstPacket;
       if (isFirstPacket) {
         insertStartCode = true;
       }
-
-      if (isFirstPacket && markerBit)
+      if (videoHeader.codecHeader.H264.single_nalu) {
          completeNALU = kNaluComplete;
-      else if (isFirstPacket)
+      } else if (isFirstPacket)
          completeNALU = kNaluStart;
       else if (markerBit)
          completeNALU = kNaluEnd;

media/webrtc/trunk/webrtc/modules/video_coding/main/source/session_info.cc

@@ -432,8 +432,14 @@ int VCMSessionInfo::InsertPacket(const VCMPacket& packet,
       }
     }
 
-    // Track the marker bit, should only be set for one packet per session.
-    if (packet.markerBit && last_packet_seq_num_ == -1) {
+    // TODO(jesup) Handle STAP-A's here, since they must share a timestamp.  Break
+    // into individual packets at this point, then handle like kNaluCompletes
+
+    // Ignore Marker bit for reassembly, since it's not 100% guaranteed to be correct
+    // Look at kNaluComplete (single_nal), or an unbroken sequence of
+    // isFirstPacket/kNaluStart (FU-A with S bit), FU-A's, FU-A with E bit (kNaluEnd)
+    if ((packet.completeNALU == kNaluComplete || packet.completeNALU == kNaluEnd) &&
+        last_packet_seq_num_ == -1) {
       last_packet_seq_num_ = static_cast<int>(packet.seqNum);
     } else if (last_packet_seq_num_ != -1 &&
       IsNewerSequenceNumber(packet.seqNum, last_packet_seq_num_)) {
@@ -442,17 +448,8 @@ int VCMSessionInfo::InsertPacket(const VCMPacket& packet,
       return -3;
     }
 
-    if (first_packet_seq_num_ == packet.seqNum &&
-        packet.completeNALU != kNaluComplete) {
-      VCMPacket& npacket = const_cast<VCMPacket&> (packet);
-      npacket.isFirstPacket = true;
-      npacket.completeNALU = kNaluStart;
-      // The insert operation invalidates the iterator |rit|.
-      packet_list_it = packets_.insert(rit.base(), npacket);
-    } else {
-      // The insert operation invalidates the iterator |rit|.
-      packet_list_it = packets_.insert(rit.base(), packet);
-    }
+    // The insert operation invalidates the iterator |rit|.
+    packet_list_it = packets_.insert(rit.base(), packet);
   } else {
     // Only insert media packets between first and last packets (when available).
     // Placing check here, as to properly account for duplicate packets.