Parse headers outside of constructor

In several cases, the parser would parse a header
(say, a track header, or the segment info header)
in the constructor for the type.  The parser had
assumed (incorrectly) that the file was well-formed,
but this turned out to be an incorrect assumption.

The parse errors triggered some assertion failures,
but this is not acceptable in a production system.

The parser solved that problem by separating the
construction of the header object from the parsing
of the file.  There is now a separate parse method
to be called after construction of the object,
which returns a status whether the parse was
successful.

This change fixes the bugs from the following
tracker issues:

http://code.google.com/p/webm/issues/detail?id=399
http://code.google.com/p/webm/issues/detail?id=400

Change-Id: Idb09154ae7008429d8613ce3b3e8294f5a12de86

libwebm_2008.vcproj

@@ -48,6 +48,7 @@
 				UsePrecompiledHeader="0"
 				WarningLevel="4"
 				DebugInformationFormat="1"
+				DisableSpecificWarnings="4996"
 			/>
 			<Tool
 				Name="VCManagedResourceCompilerTool"

mkvmuxer.cpp

@@ -830,7 +830,7 @@ bool Tracks::AddTrack(Track* track, int32 number) {
   if (number < 0)
     return false;
 
-  int32 track_num = number;
+  uint32 track_num = number;
 
   // Check to make sure a track does not already have |track_num|.
   for (uint32 i = 0; i < track_entries_size_; ++i) {

mkvparser.hpp

@@ -30,23 +30,26 @@ protected:
 
 long long GetUIntLength(IMkvReader*, long long, long&);
 long long ReadUInt(IMkvReader*, long long, long&);
-long long SyncReadUInt(IMkvReader*, long long pos, long long stop, long&);
 long long UnserializeUInt(IMkvReader*, long long pos, long long size);
-float Unserialize4Float(IMkvReader*, long long);
-double Unserialize8Double(IMkvReader*, long long);
 
-#if 0
+long UnserializeFloat(IMkvReader*, long long pos, long long size, double&);
-short Unserialize2SInt(IMkvReader*, long long);
-signed char Unserialize1SInt(IMkvReader*, long long);
-#else
 long UnserializeInt(IMkvReader*, long long pos, long len, long long& result);
-#endif
+
+long UnserializeString(
+        IMkvReader*,
+        long long pos,
+        long long size,
+        char*& str);
+
+long ParseElementHeader(
+    IMkvReader* pReader,
+    long long& pos,  //consume id and size fields
+    long long stop,  //if you know size of element's parent
+    long long& id,
+    long long& size);
 
 bool Match(IMkvReader*, long long&, unsigned long, long long&);
-bool Match(IMkvReader*, long long&, unsigned long, char*&);
 bool Match(IMkvReader*, long long&, unsigned long, unsigned char*&, size_t&);
-bool Match(IMkvReader*, long long&, unsigned long, double&);
-bool Match(IMkvReader*, long long&, unsigned long, short&);
 
 void GetVersion(int& major, int& minor, int& build, int& revision);
 
@@ -80,9 +83,11 @@ public:
     const long long m_start;
     const long long m_size;
 
-    Block(long long start, long long size, IMkvReader*);
+    Block(long long start, long long size);
     ~Block();
 
+    long Parse(IMkvReader*);
+
     long long GetTrackNumber() const;
     long long GetTimeCode(const Cluster*) const;  //absolute, but not scaled
     long long GetTime(const Cluster*) const;      //absolute, and scaled (ns)
@@ -126,6 +131,7 @@ protected:
 
 public:
     virtual ~BlockEntry();
+
     bool EOS() const;
     const Cluster* GetCluster() const;
     long GetIndex() const;
@@ -148,6 +154,7 @@ class SimpleBlock : public BlockEntry
 
 public:
     SimpleBlock(Cluster*, long index, long long start, long long size);
+    long Parse();
 
     Kind GetKind() const;
     const Block* GetBlock() const;
@@ -173,6 +180,8 @@ public:
         long long next,
         long long duration);
 
+    long Parse();
+
     Kind GetKind() const;
     const Block* GetBlock() const;
 
@@ -283,13 +292,15 @@ class Track
     Track& operator=(const Track&);
 
 public:
+    enum Type { kVideo = 1, kAudio = 2 };
+
     Segment* const m_pSegment;
     const long long m_element_start;
     const long long m_element_size;
     virtual ~Track();
 
-    long long GetType() const;
+    long GetType() const;
-    long long GetNumber() const;
+    long GetNumber() const;
     unsigned long long GetUid() const;
     const char* GetNameAsUTF8() const;
     const char* GetCodecNameAsUTF8() const;
@@ -305,21 +316,29 @@ public:
         long long size;
     };
 
-    struct Info
+    class Info
     {
-        long long type;
+    public:
-        long long number;
+        Info();
+        ~Info();
+        int Copy(Info&) const;
+        void Clear();
+    private:
+        Info(const Info&);
+        Info& operator=(const Info&);
+    public:
+        long type;
+        long number;
         unsigned long long uid;
         char* nameAsUTF8;
         char* codecId;
+        char* codecNameAsUTF8;
         unsigned char* codecPrivate;
         size_t codecPrivateSize;
-        char* codecNameAsUTF8;
         bool lacing;
         Settings settings;
-
+    private:
-        Info();
+        int CopyStr(char* Info::*str, Info&) const;
-        void Clear();
     };
 
     long GetFirst(const BlockEntry*&) const;
@@ -335,19 +354,16 @@ public:
 protected:
     Track(
         Segment*,
-        const Info&,
         long long element_start,
         long long element_size);
-    const Info m_info;
+
+    Info m_info;
 
     class EOSBlock : public BlockEntry
     {
     public:
         EOSBlock();
 
-        //bool EOS() const;
-        //const Cluster* GetCluster() const;
-        //long GetIndex() const;
         Kind GetKind() const;
         const Block* GetBlock() const;
     };
@@ -365,12 +381,19 @@ class VideoTrack : public Track
     VideoTrack(const VideoTrack&);
     VideoTrack& operator=(const VideoTrack&);
 
-public:
     VideoTrack(
+        Segment*,
+        long long element_start,
+        long long element_size);
+
+public:
+    static long Parse(
         Segment*,
         const Info&,
         long long element_start,
-        long long element_size);
+        long long element_size,
+        VideoTrack*&);
+
     long long GetWidth() const;
     long long GetHeight() const;
     double GetFrameRate() const;
@@ -391,12 +414,18 @@ class AudioTrack : public Track
     AudioTrack(const AudioTrack&);
     AudioTrack& operator=(const AudioTrack&);
 
-public:
     AudioTrack(
+        Segment*,
+        long long element_start,
+        long long element_size);
+public:
+    static long Parse(
         Segment*,
         const Info&,
         long long element_start,
-        long long element_size);
+        long long element_size,
+        AudioTrack*&);
+
     double GetSamplingRate() const;
     long long GetChannels() const;
     long long GetBitDepth() const;
@@ -428,24 +457,27 @@ public:
         long long size,
         long long element_start,
         long long element_size);
-    virtual ~Tracks();
 
-    const Track* GetTrackByNumber(unsigned long tn) const;
+    ~Tracks();
+
+    long Parse();
+
+    unsigned long GetTracksCount() const;
+
+    const Track* GetTrackByNumber(long tn) const;
     const Track* GetTrackByIndex(unsigned long idx) const;
 
 private:
     Track** m_trackEntries;
     Track** m_trackEntriesEnd;
 
-    void ParseTrackEntry(
+    long ParseTrackEntry(
-        long long,
+        long long payload_start,
-        long long,
+        long long payload_size,
-        Track*&,
         long long element_start,
-        long long element_size);
+        long long element_size,
+        Track*&) const;
 
-public:
-    unsigned long GetTracksCount() const;
 };
 
 
@@ -470,6 +502,8 @@ public:
 
     ~SegmentInfo();
 
+    long Parse();
+
     long long GetTimeCodeScale() const;
     long long GetDuration() const;  //scaled
     const char* GetMuxingAppAsUTF8() const;
@@ -506,6 +540,8 @@ public:
 
     ~SeekHead();
 
+    long Parse();
+
     struct Entry
     {
         //the SeekHead entry payload
@@ -680,9 +716,10 @@ public:
     long long GetFirstTime() const;  //time (ns) of first (earliest) block
     long long GetLastTime() const;   //time (ns) of last (latest) block
 
-    const BlockEntry* GetFirst() const;
+    long GetFirst(const BlockEntry*&) const;
-    const BlockEntry* GetLast() const;
+    long GetLast(const BlockEntry*&) const;
-    const BlockEntry* GetNext(const BlockEntry*) const;
+    long GetNext(const BlockEntry* curr, const BlockEntry*& next) const;
+
     const BlockEntry* GetEntry(const Track*, long long ns = -1) const;
     const BlockEntry* GetEntry(
         const CuePoint&,
@@ -731,12 +768,12 @@ private:
     mutable long m_entries_size;
     mutable long m_entries_count;
 
-    long ParseSimpleBlock(long long, long long&, long&) const;
+    long ParseSimpleBlock(long long, long long&, long&);
-    long ParseBlockGroup(long long, long long&, long&) const;
+    long ParseBlockGroup(long long, long long&, long&);
 
-    void CreateBlock(long long id, long long pos, long long size) const;
+    long CreateBlock(long long id, long long pos, long long size);
-    void CreateBlockGroup(long long, long long, BlockEntry**&) const;
+    long CreateBlockGroup(long long, long long);
-    void CreateSimpleBlock(long long, long long, BlockEntry**&) const;
+    long CreateSimpleBlock(long long, long long);
 
 };
 

sample.cpp

@@ -11,6 +11,7 @@
 
 #include "mkvreader.hpp"
 #include "mkvparser.hpp"
+#include <memory>
 
 #ifdef _MSC_VER
 // Silences these warnings:
@@ -83,15 +84,18 @@ int main(int argc, char* argv[])
     printf("\t\tDoc Type\t\t: %s\n", ebmlHeader.m_docType);
     printf("\t\tPos\t\t\t: %lld\n", pos);
 
-    mkvparser::Segment* pSegment;
+    typedef mkvparser::Segment seg_t;
+    seg_t* pSegment_;
 
-    long long ret = mkvparser::Segment::CreateInstance(&reader, pos, pSegment);
+    long long ret = seg_t::CreateInstance(&reader, pos, pSegment_);
     if (ret)
     {
         printf("\n Segment::CreateInstance() failed.");
         return -1;
     }
 
+    const std::auto_ptr<seg_t> pSegment(pSegment_);
+
     ret  = pSegment->Load();
     if (ret < 0)
     {
@@ -157,8 +161,6 @@ int main(int argc, char* argv[])
     unsigned long i = 0;
     const unsigned long j = pTracks->GetTracksCount();
 
-    enum { VIDEO_TRACK = 1, AUDIO_TRACK = 2 };
-
     printf("\n\t\t\t   Track Info\n");
 
     while (i != j)
@@ -168,13 +170,13 @@ int main(int argc, char* argv[])
         if (pTrack == NULL)
             continue;
 
-        const long long trackType = pTrack->GetType();
+        const long trackType = pTrack->GetType();
-        const long long trackNumber = pTrack->GetNumber();
+        const long trackNumber = pTrack->GetNumber();
         const unsigned long long trackUid = pTrack->GetUid();
         const wchar_t* const pTrackName = utf8towcs(pTrack->GetNameAsUTF8());
 
-        printf("\t\tTrack Type\t\t: %lld\n", trackType);
+        printf("\t\tTrack Type\t\t: %ld\n", trackType);
-        printf("\t\tTrack Number\t\t: %lld\n", trackNumber);
+        printf("\t\tTrack Number\t\t: %ld\n", trackNumber);
         printf("\t\tTrack Uid\t\t: %lld\n", trackUid);
 
         if (pTrackName == NULL)
@@ -203,7 +205,7 @@ int main(int argc, char* argv[])
             delete [] pCodecName;
         }
 
-        if (trackType == VIDEO_TRACK)
+        if (trackType == mkvparser::Track::kVideo)
         {
             const VideoTrack* const pVideoTrack =
                 static_cast<const VideoTrack*>(pTrack);
@@ -218,7 +220,7 @@ int main(int argc, char* argv[])
             printf("\t\tVideo Rate\t\t: %f\n",rate);
         }
 
-        if (trackType == AUDIO_TRACK)
+        if (trackType == mkvparser::Track::kAudio)
         {
             const AudioTrack* const pAudioTrack =
                 static_cast<const AudioTrack*>(pTrack);
@@ -242,7 +244,6 @@ int main(int argc, char* argv[])
     if (clusterCount == 0)
     {
         printf("\t\tSegment has no clusters.\n");
-        delete pSegment;
         return -1;
     }
 
@@ -256,7 +257,16 @@ int main(int argc, char* argv[])
         const long long time_ns = pCluster->GetTime();
         printf("\t\tCluster Time (ns)\t: %lld\n", time_ns);
 
-        const BlockEntry* pBlockEntry = pCluster->GetFirst();
+        const BlockEntry* pBlockEntry;
+
+        long status = pCluster->GetFirst(pBlockEntry);
+
+        if (status < 0)  //error
+        {
+            printf("\t\tError parsing first block of cluster\n");
+            fflush(stdout);
+            return -1;
+        }
 
         while ((pBlockEntry != NULL) && !pBlockEntry->EOS())
         {
@@ -264,30 +274,42 @@ int main(int argc, char* argv[])
             const long long trackNum = pBlock->GetTrackNumber();
             const unsigned long tn = static_cast<unsigned long>(trackNum);
             const Track* const pTrack = pTracks->GetTrackByNumber(tn);
-            const long long trackType = pTrack->GetType();
-            const int frameCount = pBlock->GetFrameCount();
-            const long long time_ns = pBlock->GetTime(pCluster);
 
-            printf("\t\t\tBlock\t\t:%s,%s,%15lld\n",
+            if (pTrack == NULL)
-                   (trackType == VIDEO_TRACK) ? "V" : "A",
+                printf("\t\t\tBlock\t\t:UNKNOWN TRACK TYPE\n");
-                   pBlock->IsKey() ? "I" : "P",
+            else
-                   time_ns);
-
-            for (int i = 0; i < frameCount; ++i)
             {
-                const Block::Frame& theFrame = pBlock->GetFrame(i);
+                const long long trackType = pTrack->GetType();
-                const long size = theFrame.len;
+                const int frameCount = pBlock->GetFrameCount();
-                const long long offset = theFrame.pos;
+                const long long time_ns = pBlock->GetTime(pCluster);
-                printf("\t\t\t %15ld,%15llx\n", size, offset);
+
+                printf("\t\t\tBlock\t\t:%s,%s,%15lld\n",
+                       (trackType == mkvparser::Track::kVideo) ? "V" : "A",
+                       pBlock->IsKey() ? "I" : "P",
+                       time_ns);
+
+                for (int i = 0; i < frameCount; ++i)
+                {
+                    const Block::Frame& theFrame = pBlock->GetFrame(i);
+                    const long size = theFrame.len;
+                    const long long offset = theFrame.pos;
+                    printf("\t\t\t %15ld,%15llx\n", size, offset);
+                }
             }
 
-            pBlockEntry = pCluster->GetNext(pBlockEntry);
+            status = pCluster->GetNext(pBlockEntry, pBlockEntry);
+
+            if (status < 0)
+            {
+                printf("\t\t\tError parsing next block of cluster\n");
+                fflush(stdout);
+                return -1;
+            }
         }
 
         pCluster = pSegment->GetNext(pCluster);
     }
 
-    delete pSegment;
+    fflush(stdout);
     return 0;
-
 }

sample_muxer/sample_muxer.cpp

@@ -317,7 +317,15 @@ int main(int argc, char* argv[]) {
   const mkvparser::Cluster* cluster = parser_segment->GetFirst();
 
   while ((cluster != NULL) && !cluster->EOS()) {
-    const mkvparser::BlockEntry* block_entry = cluster->GetFirst();
+    const mkvparser::BlockEntry* block_entry;
+
+    long status = cluster->GetFirst(block_entry);
+
+    if (status)
+    {
+        printf("\n Could not get first block of cluster.\n");
+        return EXIT_FAILURE;
+    }
 
     while ((block_entry != NULL) && !block_entry->EOS()) {
       const mkvparser::Block* const block = block_entry->GetBlock();
@@ -362,7 +370,13 @@ int main(int argc, char* argv[]) {
         }
       }
 
-      block_entry = cluster->GetNext(block_entry);
+      status = cluster->GetNext(block_entry, block_entry);
+
+      if (status)
+      {
+          printf("\n Could not get next block of cluster.\n");
+          return EXIT_FAILURE;
+      }
     }
 
     cluster = parser_segment->GetNext(cluster);