Refactor Cluster::AddFrame member function

Change-Id: Ibff460918243647eb76785508bb50490bc1d8561

mkvmuxer.cpp

@@ -12,6 +12,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <ctime>
+#include <limits>
 #include <new>
 
 #include "mkvmuxerutil.hpp"
@@ -1024,28 +1025,14 @@ bool Cluster::Init(IMkvWriter* ptr_writer) {
 bool Cluster::AddFrame(const uint8* frame,
                        uint64 length,
                        uint64 track_number,
-                       int16 timecode,
+                       uint64 abs_timecode,
                        bool is_key) {
-  if (finalized_)
+  return DoWriteBlock(frame,
-    return false;
+                      length,
-
+                      track_number,
-  if (!header_written_)
+                      abs_timecode,
-    if (!WriteClusterHeader())
+                      is_key ? 1 : 0,
-      return false;
+                      &WriteSimpleBlock);
-
-  const uint64 element_size = WriteSimpleBlock(writer_,
-                                               frame,
-                                               length,
-                                               static_cast<char>(track_number),
-                                               timecode,
-                                               is_key);
-  if (!element_size)
-    return false;
-
-  AddPayloadSize(element_size);
-  blocks_added_++;
-
-  return true;
 }
 
 void Cluster::AddPayloadSize(uint64 size) {
@@ -1081,6 +1068,59 @@ uint64 Cluster::Size() const {
   return element_size;
 }
 
+bool Cluster::DoWriteBlock(
+    const uint8* frame,
+    uint64 length,
+    uint64 track_number,
+    uint64 abs_timecode,
+    uint64 generic_arg,
+    WriteBlock write_block) {
+  if (frame == NULL || length == 0)
+    return false;
+
+  // To simplify things, we require that there be fewer than 127
+  // tracks -- this allows us to serialize the track number value for
+  // a stream using a single byte, per the Matroska encoding.
+
+  if (track_number == 0 || track_number > 0x7E)
+    return false;
+
+  const int64 cluster_timecode = this->Cluster::timecode();
+  const int64 rel_timecode =
+      static_cast<int64>(abs_timecode) - cluster_timecode;
+
+  if (rel_timecode < 0)
+    return false;
+
+  if (rel_timecode > std::numeric_limits<int16>::max())
+    return false;
+
+  if (write_block == NULL)
+    return false;
+
+  if (finalized_)
+    return false;
+
+  if (!header_written_)
+    if (!WriteClusterHeader())
+      return false;
+
+  const uint64 element_size = (*write_block)(writer_,
+                                             frame,
+                                             length,
+                                             track_number,
+                                             rel_timecode,
+                                             generic_arg);
+
+  if (element_size == 0)
+    return false;
+
+  AddPayloadSize(element_size);
+  blocks_added_++;
+
+  return true;
+}
+
 bool Cluster::WriteClusterHeader() {
   if (finalized_)
     return false;
@@ -1710,10 +1750,14 @@ bool Segment::AddFrame(const uint8* frame,
   if (!cluster)
     return false;
 
-  int64 block_timecode = timestamp / segment_info_.timecode_scale();
+  const uint64 timecode_scale = segment_info_.timecode_scale();
-  block_timecode -= static_cast<int64>(cluster->timecode());
+  const uint64 abs_timecode = timestamp / timecode_scale;
 
-  if (block_timecode < 0)
+  if (!cluster->AddFrame(frame,
+                         length,
+                         track_number,
+                         abs_timecode,
+                         is_key))
     return false;
 
   if (new_cuepoint_ && cues_track_ == track_number) {
@@ -1721,13 +1765,6 @@ bool Segment::AddFrame(const uint8* frame,
       return false;
   }
 
-  if (!cluster->AddFrame(frame,
-                         length,
-                         track_number,
-                         static_cast<int16>(block_timecode),
-                         is_key))
-    return false;
-
   if (timestamp > last_timestamp_)
     last_timestamp_ = timestamp;
 
@@ -2027,32 +2064,30 @@ bool Segment::WriteFramesAll() {
     if (!cluster)
       return false;
 
+    const uint64 timecode_scale = segment_info_.timecode_scale();
+
     for (int32 i = 0; i < frames_size_; ++i) {
-      Frame* const frame = frames_[i];
+      Frame*& frame = frames_[i];
-
+      const uint64 frame_timestamp = frame->timestamp();  // ns
-      int64 block_timecode =
+      const uint64 frame_timecode = frame_timestamp / timecode_scale;
-          frame->timestamp() / segment_info_.timecode_scale();
-      block_timecode -= static_cast<int64>(cluster->timecode());
-
-      if (block_timecode < 0)
-        return false;
-
-      if (new_cuepoint_ && cues_track_ == frame->track_number()) {
-        if (!AddCuePoint(frame->timestamp()))
-          return false;
-      }
 
       if (!cluster->AddFrame(frame->frame(),
                              frame->length(),
                              frame->track_number(),
-                             static_cast<int16>(block_timecode),
+                             frame_timecode,
                              frame->is_key()))
         return false;
 
-      if (frame->timestamp() > last_timestamp_)
+      if (new_cuepoint_ && cues_track_ == frame->track_number()) {
-        last_timestamp_ = frame->timestamp();
+        if (!AddCuePoint(frame_timestamp))
+          return false;
+      }
+
+      if (frame_timestamp > last_timestamp_)
+        last_timestamp_ = frame_timestamp;
 
       delete frame;
+      frame = NULL;
     }
 
     frames_size_ = 0;
@@ -2070,10 +2105,10 @@ bool Segment::WriteFramesLessThan(uint64 timestamp) {
       return false;
 
     Cluster* const cluster = cluster_list_[cluster_list_size_-1];
-
     if (!cluster)
       return false;
 
+    const uint64 timecode_scale = segment_info_.timecode_scale();
     int32 shift_left = 0;
 
     // TODO(fgalligan): Change this to use the durations of frames instead of
@@ -2085,29 +2120,24 @@ bool Segment::WriteFramesLessThan(uint64 timestamp) {
         break;
 
       const Frame* const frame_prev = frames_[i-1];
-
+      const uint64 frame_timestamp = frame_prev->timestamp();
-      int64 block_timecode =
+      const uint64 frame_timecode = frame_timestamp / timecode_scale;
-          frame_prev->timestamp() / segment_info_.timecode_scale();
-      block_timecode -= static_cast<int64>(cluster->timecode());
-
-      if (block_timecode < 0)
-        return false;
-
-      if (new_cuepoint_ && cues_track_ == frame_prev->track_number()) {
-        if (!AddCuePoint(frame_prev->timestamp()))
-          return false;
-      }
 
       if (!cluster->AddFrame(frame_prev->frame(),
                              frame_prev->length(),
                              frame_prev->track_number(),
-                             static_cast<int16>(block_timecode),
+                             frame_timecode,
                              frame_prev->is_key()))
         return false;
 
+      if (new_cuepoint_ && cues_track_ == frame_prev->track_number()) {
+        if (!AddCuePoint(frame_timestamp))
+          return false;
+      }
+
       ++shift_left;
-      if (frame_prev->timestamp() > last_timestamp_)
+      if (frame_timestamp > last_timestamp_)
-        last_timestamp_ = frame_prev->timestamp();
+        last_timestamp_ = frame_timestamp;
 
       delete frame_prev;
     }

mkvmuxer.hpp

@@ -489,13 +489,14 @@ class Cluster {
   //   frame: Pointer to the data
   //   length: Length of the data
   //   track_number: Track to add the data to. Value returned by Add track
-  //                 functions.
+  //                 functions.  The range of allowed values is [1, 126].
-  //   timestamp:    Timecode of the frame relative to the cluster timecode.
+  //   timecode:     Absolute (not relative to cluster) timestamp of the
+  //                 frame, expressed in timecode units.
   //   is_key:       Flag telling whether or not this frame is a key frame.
   bool AddFrame(const uint8* frame,
                 uint64 length,
                 uint64 track_number,
-                short timecode,
+                uint64 timecode,  // timecode units (absolute)
                 bool is_key);
 
   // Increments the size of the cluster's data in bytes.
@@ -514,6 +515,23 @@ class Cluster {
   uint64 timecode() const { return timecode_; }
 
  private:
+  //  Signature that matches either of WriteSimpleBlock or WriteMetadataBlock
+  //  in the muxer utilities package.
+  typedef uint64 (*WriteBlock)(IMkvWriter* writer,
+                               const uint8* data,
+                               uint64 length,
+                               uint64 track_number,
+                               int64 timecode,
+                               uint64 generic_arg);
+
+  //  Used to implement AddFrame.
+  bool DoWriteBlock(const uint8* frame,
+                    uint64 length,
+                    uint64 track_number,
+                    uint64 absolute_timecode,
+                    uint64 generic_arg,
+                    WriteBlock write_block);
+
   // Outputs the Cluster header to |writer_|. Returns true on success.
   bool WriteClusterHeader();
 
@@ -670,7 +688,7 @@ class Segment {
   bool AddFrame(const uint8* frame,
                 uint64 length,
                 uint64 track_number,
-                uint64 timestamp,
+                uint64 timestamp_ns,
                 bool is_key);
 
   // Adds a video track to the segment. Returns the number of the track on

mkvmuxerutil.cpp

@@ -309,7 +309,7 @@ uint64 WriteSimpleBlock(IMkvWriter* writer,
                         uint64 length,
                         uint64 track_number,
                         int64 timecode,
-                        bool is_key) {
+                        uint64 is_key) {
   if (!writer)
     return false;
 

mkvmuxerutil.hpp

@@ -62,13 +62,13 @@ bool WriteEbmlElement(IMkvWriter* writer,
 //                  permitted.
 //   timecode:     Relative timecode of the Block.  Only values in the
 //                  range [0, 2^15) are permitted.
-//   is_key:       Flag telling whether or not this frame is a key frame.
+//   is_key:       Non-zero value specifies that frame is a key frame.
 uint64 WriteSimpleBlock(IMkvWriter* writer,
                         const uint8* data,
                         uint64 length,
                         uint64 track_number,
                         int64 timecode,
-                        bool is_key);
+                        uint64 is_key);
 
 // Output a metadata keyframe, using a Block Group element.
 // Inputs: