Repositing Cues before Clusters

A whole new approach to repositioning Cues before Clusters. This
patchset adds a new function CopyAndMoveCuesBeforeClusters to the
Segment class. This function should be called after
Segment::Finalize() to obtain a copy of the same output file with
Cues positioned before the Clusters.

Removing everything else that was added to accomplish the same in
the previous few commits.

Also, adding std:: qualifier to one of the variables in
sample_muxer_metadata which was missed accidentally in the previous
commit.

Change-Id: I2810d06a6251325add2f5e54d32d1da6e2fe143f

Makefile

@@ -2,7 +2,7 @@ CXX       := g++
 CXXFLAGS  := -W -Wall -g
 LIBWEBMA  := libwebm.a
 LIBWEBMSO := libwebm.so
-WEBMOBJS  := mkvparser.o mkvreader.o mkvmuxer.o mkvmuxerutil.o mkvwriter.o mkvwriteeofreader.o
+WEBMOBJS  := mkvparser.o mkvreader.o mkvmuxer.o mkvmuxerutil.o mkvwriter.o
 OBJSA     := $(WEBMOBJS:.o=_a.o)
 OBJSSO    := $(WEBMOBJS:.o=_so.o)
 OBJECTS1  := sample.o

mkvmuxer.cpp

@@ -7,7 +7,6 @@
 // be found in the AUTHORS file in the root of the source tree.
 
 #include "mkvmuxer.hpp"
-#include "mkvwriteeofreader.hpp"
 
 #include <cstdio>
 #include <cstdlib>
@@ -16,6 +15,7 @@
 #include <new>
 
 #include "mkvmuxerutil.hpp"
+#include "mkvparser.hpp"
 #include "mkvwriter.hpp"
 #include "webmids.hpp"
 
@@ -91,7 +91,7 @@ bool WriteEbmlHeader(IMkvWriter* writer) {
   return true;
 }
 
-bool ChunkedCopy(mkvmuxer::IMkvWriteEOFReader* source,
+bool ChunkedCopy(mkvparser::IMkvReader* source,
                  mkvmuxer::IMkvWriter* dst,
                  mkvmuxer::int64 start, int64 size) {
   // TODO(vigneshv): Check if this is a reasonable value.
@@ -2078,25 +2078,36 @@ bool Segment::Init(IMkvWriter* ptr_writer) {
   if (!ptr_writer) {
     return false;
   }
-  writer_ = ptr_writer;
+  writer_cluster_ = ptr_writer;
-  if (cues_position_ == kAfterClusters) {
+  writer_cues_ = ptr_writer;
-    writer_cluster_ = writer_;
+  writer_header_ = ptr_writer;
-    writer_cues_ = writer_;
-    writer_header_ = writer_;
-  } else {
-    writer_cluster_ = writer_temp_;
-    writer_cues_ = writer_temp_;
-    writer_header_ = writer_temp_;
-  }
   return segment_info_.Init();
 }
 
-bool Segment::WriteCuesBeforeClusters(IMkvWriteEOFReader* ptr_writer) {
+bool Segment::CopyAndMoveCuesBeforeClusters(mkvparser::IMkvReader* reader,
-  if (!ptr_writer || writer_cluster_ || writer_cues_ || writer_header_) {
+                                            IMkvWriter* writer) {
+  const int64 cluster_offset = cluster_list_[0]->size_position() -
+                               GetUIntSize(kMkvCluster);
+
+  // Copy the headers.
+  if (!ChunkedCopy(reader, writer, 0, cluster_offset))
+    return false;
+
+  // Recompute cue positions and seek entries.
+  MoveCuesBeforeClusters();
+
+  // Write cues and seek entries.
+  // TODO(vigneshv): As of now, it's safe to call seek_head_.Finalize() for the
+  // second time with a different writer object. But the name Finalize() doesn't
+  // indicate something we want to call more than once. So consider renaming it
+  // to write() or some such.
+  if (!cues_.Write(writer) || !seek_head_.Finalize(writer))
+    return false;
+
+  // Copy the Clusters.
+  if (!ChunkedCopy(reader, writer, cluster_offset,
+                   cluster_end_offset_ - cluster_offset))
     return false;
-  }
-  writer_temp_ = ptr_writer;
-  cues_position_ = kBeforeClusters;
   return true;
 }
 
@@ -2142,16 +2153,7 @@ bool Segment::Finalize() {
         return false;
     }
 
-    const int64 current_offset = writer_cluster_->Position();
+    cluster_end_offset_ = writer_cluster_->Position();
-    const int64 cluster_offset = cluster_list_[0]->size_position() -
-                                 GetUIntSize(kMkvCluster);
-    if (cues_position_ == kBeforeClusters) {
-      writer_cluster_ = writer_;
-      writer_cues_ = writer_;
-      writer_header_ = writer_;
-      ChunkedCopy(writer_temp_, writer_cluster_, 0, cluster_offset);
-      MoveCuesBeforeClusters();
-    }
 
     // Write the seek headers and cues
     if (output_cues_)
@@ -2161,11 +2163,6 @@ bool Segment::Finalize() {
     if (!seek_head_.Finalize(writer_header_))
       return false;
 
-    if (cues_position_ == kBeforeClusters) {
-      ChunkedCopy(writer_temp_, writer_cluster_,
-                  cluster_offset, current_offset - cluster_offset);
-    }
-
     if (writer_header_->Seekable()) {
       if (size_position_ == -1)
         return false;

mkvmuxer.hpp

@@ -14,6 +14,10 @@
 // For a description of the WebM elements see
 // http://www.webmproject.org/code/specs/container/.
 
+namespace mkvparser {
+  class IMkvReader;
+}  // end namespace
+
 namespace mkvmuxer {
 
 class MkvWriter;
@@ -51,23 +55,12 @@ class IMkvWriter {
   LIBWEBM_DISALLOW_COPY_AND_ASSIGN(IMkvWriter);
 };
 
-// Classes implementing this interface should adhere to the following rule:
-// - There can be any number of calls to Write().
-// - Once the first call to Read() occurs, no more Write()'s are allowed.
-// - Write() should return error if there has been a call to Read() before.
-class IMkvWriteEOFReader : public IMkvWriter {
- public:
-  // Reads data of size |length| starting from the offset of |position| into
-  // |buffer|.
-  virtual int Read(int64 position, int32 length, uint8* buffer) = 0;
-};
-
 // Writes out the EBML header for a WebM file. This function must be called
 // before any other libwebm writing functions are called.
 bool WriteEbmlHeader(IMkvWriter* writer);
 
 // Copies in Chunk from source to destination between the given byte positions
-bool ChunkedCopy(IMkvWriteEOFReader* source, IMkvWriter* dst,
+bool ChunkedCopy(mkvparser::IMkvReader* source, IMkvWriter* dst,
                  int64 start, int64 size);
 
 ///////////////////////////////////////////////////////////////
@@ -946,12 +939,6 @@ class Segment {
   // |ptr_writer| is NULL.
   bool Init(IMkvWriter* ptr_writer);
 
-  // This function must be called before Init() if Cues are to be written before
-  // the Clusters. Input parameter is a IMkvWriteEOFReader object, which is to
-  // be used as the temporary storage, while moving the Cues before the
-  // Clusters.
-  bool WriteCuesBeforeClusters(IMkvWriteEOFReader* ptr_writer);
-
   // Adds a generic track to the segment.  Returns the newly-allocated
   // track object (which is owned by the segment) on success, NULL on
   // error. |number| is the number to use for the track.  |number|
@@ -1025,6 +1012,19 @@ class Segment {
   // the track number.
   uint64 AddVideoTrack(int32 width, int32 height, int32 number);
 
+  // This function must be called after Finalize() if you need a copy of the
+  // output with Cues written before the Clusters.
+  // Input parameters:
+  // reader - an IMkvReader object created with the same underlying file of the
+  //          current writer object. Make sure to close the existing writer
+  //          object before creating this so that all the data is properly
+  //          flushed and available for reading.
+  // writer - an IMkvWriter object pointing to a *different* file than the one
+  //          pointed by the current writer object. This file will contain the
+  //          Cues element before the Clusters.
+  bool CopyAndMoveCuesBeforeClusters(mkvparser::IMkvReader* reader,
+                                     IMkvWriter* writer);
+
   // Sets which track to use for the Cues element. Must have added the track
   // before calling this function. Returns true on success. |track_number| is
   // returned by the Add track functions.
@@ -1181,6 +1181,9 @@ class Segment {
   // Base filename for the chunked files.
   char* chunking_base_name_;
 
+  // File position offset where the Clusters end.
+  int64 cluster_end_offset_;
+
   // List of clusters.
   Cluster** cluster_list_;
 
@@ -1252,10 +1255,6 @@ class Segment {
   IMkvWriter* writer_cues_;
   IMkvWriter* writer_header_;
 
-  // Pointer to actual writer object and temp writer object
-  IMkvWriter* writer_;
-  IMkvWriteEOFReader* writer_temp_;
-
   LIBWEBM_DISALLOW_COPY_AND_ASSIGN(Segment);
 };
 

mkvwriteeofreader.cpp

@@ -1,146 +0,0 @@
-// Copyright (c) 2013 The WebM 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.
-
-#include "mkvwriteeofreader.hpp"
-
-#ifdef _MSC_VER
-#include <share.h>  // for _SH_DENYWR
-#endif
-
-#include <cstdio>
-#include <new>
-
-namespace mkvmuxer {
-
-MkvWriteEOFReader::MkvWriteEOFReader() : file_(NULL),
-                                         writes_allowed_(true) {
-}
-
-MkvWriteEOFReader::~MkvWriteEOFReader() {
-  Close();
-}
-
-int32 MkvWriteEOFReader::Write(const void* buffer, uint32 length) {
-  if (file_ == NULL || !writes_allowed_)
-    return -1;
-
-  if (length == 0)
-    return 0;
-
-  if (buffer == NULL)
-    return -1;
-
-  const size_t bytes_written = fwrite(buffer, 1, length, file_);
-
-  return (bytes_written == length) ? 0 : -1;
-}
-
-bool MkvWriteEOFReader::Open(const char* filename, bool create_temp_file) {
-  if (filename == NULL && !create_temp_file)
-    return false;
-
-  if (file_ != NULL)
-    return false;
-
-#ifdef _MSC_VER
-  file_ = create_temp_file ? tmpfile() : _fsopen(filename, "wb+", _SH_DENYWR);
-#else
-  file_ = create_temp_file ? tmpfile() : fopen(filename, "wb+");
-#endif
-  if (file_ == NULL)
-    return false;
-  return true;
-}
-
-void MkvWriteEOFReader::Close() {
-  if (file_ != NULL) {
-    fclose(file_);
-    file_ = NULL;
-  }
-}
-
-int64 MkvWriteEOFReader::Position() const {
-  if (file_ == NULL)
-    return 0;
-
-#ifdef _MSC_VER
-    return _ftelli64(file_);
-#else
-    return ftell(file_);
-#endif
-}
-
-int32 MkvWriteEOFReader::Position(int64 position) {
-  if (file_ == NULL)
-    return -1;
-
-#ifdef _MSC_VER
-    return _fseeki64(file_, position, SEEK_SET);
-#else
-    return fseek(file_, position, SEEK_SET);
-#endif
-}
-
-bool MkvWriteEOFReader::Seekable() const {
-  return true;
-}
-
-void MkvWriteEOFReader::ElementStartNotify(uint64, int64) {
-}
-
-int MkvWriteEOFReader::Read(int64 offset, int32 len, uint8* buffer) {
-  if (file_ == NULL)
-    return -1;
-
-  if (offset < 0)
-    return -1;
-
-  if (len < 0)
-    return -1;
-
-  if (writes_allowed_)
-    writes_allowed_ = false;
-
-  if (len == 0)
-    return 0;
-
-  fflush(file_);
-
-#ifdef _MSC_VER
-  int64 pos = _ftelli64(file_);
-#else
-  int64 pos = ftell(file_);
-#endif
-
-#ifdef WIN32
-  int status = _fseeki64(file_, offset, SEEK_SET);
-
-  if (status)
-    return -1;  //error
-#else
-  fseek(file_, offset, SEEK_SET);
-#endif
-
-  const size_t size = fread(buffer, 1, len, file_);
-
-#ifdef WIN32
-  status = _fseeki64(file_, pos, SEEK_SET);
-
-  if (status)
-    return -1;  //error
-#else
-  fseek(file_, pos, SEEK_SET);
-#endif
-
-  if (size < static_cast<size_t>(len))
-    return -1;  //error
-
-  return 0;  //success
-}
-
-}  // namespace mkvmuxer

mkvwriteeofreader.hpp

@@ -1,56 +0,0 @@
-// Copyright (c) 2013 The WebM 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.
-
-#ifndef MKVWRITEEOFREADER_HPP
-#define MKVWRITEEOFREADER_HPP
-
-#include <stdio.h>
-
-#include "mkvmuxer.hpp"
-#include "mkvmuxertypes.hpp"
-
-namespace mkvmuxer {
-
-// Default implementation of the IMkvWriteEOFReader interface.
-class MkvWriteEOFReader : public IMkvWriteEOFReader {
- public:
-  MkvWriteEOFReader();
-  virtual ~MkvWriteEOFReader();
-
-  // IMkvWriteEOFReader interface
-  virtual int64 Position() const;
-  virtual int32 Position(int64 position);
-  virtual bool Seekable() const;
-  virtual int32 Write(const void* buffer, uint32 length);
-  virtual void ElementStartNotify(uint64 element_id, int64 position);
-  virtual int Read(int64 position, int32 length, uint8* buffer);
-
-  // Creates and opens a file for reading and writing. |filename| is the name of
-  // the file to open. This function will overwrite the contents of |filename|.
-  // If |create_temp_file| is set, the |filename| parameter is ignored and a
-  // temporary file is created, which is automatically deleted upon close.
-  // Returns true on success.
-  bool Open(const char* filename, bool create_temp_file);
-
-  // Closes an opened file.
-  void Close();
-
- private:
-  // File handle to output file.
-  FILE* file_;
-
-  // Flag indicating if writes are allowed. Will be set to false on first call
-  // to Read().
-  bool writes_allowed_;
-
-  LIBWEBM_DISALLOW_COPY_AND_ASSIGN(MkvWriteEOFReader);
-};
-
-}  // end namespace mkvmuxer
-
-#endif  // MKVWRITEEOFREADER_HPP

sample_muxer.cpp

@@ -18,7 +18,6 @@
 
 // libwebm muxer includes
 #include "mkvmuxer.hpp"
-#include "mkvwriteeofreader.hpp"
 #include "mkvwriter.hpp"
 #include "mkvmuxerutil.hpp"
 
@@ -264,7 +263,8 @@ int main(int argc, char* argv[]) {
   // Set muxer header info
   mkvmuxer::MkvWriter writer;
 
-  if (!writer.Open(output)) {
+  char* temp_file = tmpnam(NULL);
+  if (!writer.Open(cues_before_clusters ? temp_file : output)) {
     printf("\n Filename is invalid or error while opening.\n");
     return EXIT_FAILURE;
   }
@@ -272,14 +272,6 @@ int main(int argc, char* argv[]) {
   // Set Segment element attributes
   mkvmuxer::Segment muxer_segment;
 
-  mkvmuxer::MkvWriteEOFReader writer_temp;
-  if (cues_before_clusters) {
-    if (!writer_temp.Open(NULL, true)) {
-      printf("\n Filename is invalid or error while opening.\n");
-      return EXIT_FAILURE;
-    }
-    muxer_segment.WriteCuesBeforeClusters(&writer_temp);
-  }
   if (!muxer_segment.Init(&writer)) {
     printf("\n Could not initialize muxer segment!\n");
     return EXIT_FAILURE;
@@ -520,13 +512,29 @@ int main(int argc, char* argv[]) {
     return EXIT_FAILURE;
   }
 
+  reader.Close();
+  writer.Close();
+
+  if (cues_before_clusters) {
+    if (reader.Open(temp_file)) {
+      printf("\n Filename is invalid or error while opening.\n");
+      return EXIT_FAILURE;
+    }
+    if (!writer.Open(output)) {
+      printf("\n Filename is invalid or error while opening.\n");
+      return EXIT_FAILURE;
+    }
+    if (!muxer_segment.CopyAndMoveCuesBeforeClusters(&reader, &writer)) {
+      printf("\n Unable to copy and move cues before clusters.\n");
+      return EXIT_FAILURE;
+    }
+    reader.Close();
+    writer.Close();
+    remove(temp_file);
+  }
+
   delete [] data;
   delete parser_segment;
 
-  writer.Close();
-  if (cues_before_clusters)
-    writer_temp.Close();
-  reader.Close();
-
   return EXIT_SUCCESS;
 }

sample_muxer_metadata.cc

@@ -168,7 +168,7 @@ bool SampleMuxerMetadata::AddChapter(const cue_t& cue) {
   iter_t i = cue.payload.begin();
   const iter_t j = cue.payload.end();
 
-  string title;
+  std::string title;
 
   for (;;) {
     title += *i++;