Fix WebMStreamParser to handle Cues between Clusters correctly.

media/formats/webm/webm_stream_parser.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/formats/webm/webm_stream_parser.h"
 
 #include <string>
 
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "media/formats/webm/webm_cluster_parser.h"
 #include "media/formats/webm/webm_constants.h"
 #include "media/formats/webm/webm_content_encodings.h"
 #include "media/formats/webm/webm_crypto_helpers.h"
 #include "media/formats/webm/webm_info_parser.h"
 #include "media/formats/webm/webm_tracks_parser.h"
 
 namespace media {
 
 WebMStreamParser::WebMStreamParser()
     : state_(kWaitingForInit),
-      unknown_segment_size_(false),
-      parsing_cluster_(false) {
+      unknown_segment_size_(false) {
 }
 
 WebMStreamParser::~WebMStreamParser() {
 }
 
 void WebMStreamParser::Init(const InitCB& init_cb,
                             const NewConfigCB& config_cb,
                             const NewBuffersCB& new_buffers_cb,
                             bool ignore_text_tracks,
                             const NeedKeyCB& need_key_cb,
@@ skipping 17 matching lines @@
   need_key_cb_ = need_key_cb;
   new_segment_cb_ = new_segment_cb;
   end_of_segment_cb_ = end_of_segment_cb;
   log_cb_ = log_cb;
 }
 
 void WebMStreamParser::Flush() {
   DCHECK_NE(state_, kWaitingForInit);
 
   byte_queue_.Reset();
-  parsing_cluster_ = false;
-
-  if (state_ != kParsingClusters)
-    return;
-
   cluster_parser_->Reset();

[wolenetz, 2014/06/18 22:47:53]

cluster_parser_ may still be null prior to completing the first init segment
parse. If Flush() occurred then, boom. Please protect against this.

[Sergey Ulanov, 2014/06/18 23:26:50]

Done.

+  if (state_ == kParsingClusters) {
+    ChangeState(kParsingHeaders);
+    end_of_segment_cb_.Run();
+  }
 }
 
 bool WebMStreamParser::Parse(const uint8* buf, int size) {
   DCHECK_NE(state_, kWaitingForInit);
 
   if (state_ == kError)
     return false;
 
   byte_queue_.Push(buf, size);
 
@@ skipping 17 matching lines @@
       case kWaitingForInit:
       case kError:
         return false;
     }
 
     if (result < 0) {
       ChangeState(kError);
       return false;
     }
 
     if (state_ == oldState && result == 0)

[wolenetz, 2014/06/18 22:47:53]

aside: This state_ == oldState is key to the simplification.

[Sergey Ulanov, 2014/06/18 23:26:49]

Yes

       break;
 
     DCHECK_GE(result, 0);
     cur += result;
     cur_size -= result;
     bytes_parsed += result;
   }
 
   byte_queue_.Pop(bytes_parsed);
   return true;
@@ skipping 28 matching lines @@
     case kWebMIdCues:
     case kWebMIdChapters:
       // TODO(matthewjheaney): Implement support for chapters.
       if (cur_size < (result + element_size)) {
         // We don't have the whole element yet. Signal we need more data.
         return 0;
       }
       // Skip the element.
       return result + element_size;
       break;
+    case kWebMIdCluster:
+      if (!cluster_parser_) {
+        MEDIA_LOG(log_cb_) << "Found Cluster element before Info.";
+        return -1;
+      }
+      ChangeState(kParsingClusters);
+      new_segment_cb_.Run();
+      return 0;
+      break;
     case kWebMIdSegment:
       // Segment of unknown size indicates live stream.
       if (element_size == kWebMUnknownSize)
         unknown_segment_size_ = true;
       // Just consume the segment header.
       return result;
       break;
     case kWebMIdInfo:
       // We've found the element we are looking for.
       break;
@@ skipping 60 matching lines @@
       tracks_parser.audio_track_num(),
       tracks_parser.GetAudioDefaultDuration(timecode_scale_in_us),
       tracks_parser.video_track_num(),
       tracks_parser.GetVideoDefaultDuration(timecode_scale_in_us),
       tracks_parser.text_tracks(),
       tracks_parser.ignored_tracks(),
       tracks_parser.audio_encryption_key_id(),
       tracks_parser.video_encryption_key_id(),
       log_cb_));
 
-  ChangeState(kParsingClusters);
-
   if (!init_cb_.is_null())
     base::ResetAndReturn(&init_cb_).Run(true, params);
 
   return bytes_parsed;
 }
 
 int WebMStreamParser::ParseCluster(const uint8* data, int size) {
   if (!cluster_parser_)
     return -1;
 
-  int result = 0;
-  int bytes_parsed;
-  bool cluster_ended;
-  do {
-    cluster_ended = false;
+  int bytes_parsed = cluster_parser_->Parse(data, size);
+  if (bytes_parsed < 0)
+    return bytes_parsed;
 
-    // If we are not parsing a cluster then handle the case when the next
-    // element is not a cluster.
-    if (!parsing_cluster_) {
-      int id;
-      int64 element_size;
-      bytes_parsed = WebMParseElementHeader(data, size, &id, &element_size);
+  const BufferQueue& audio_buffers = cluster_parser_->GetAudioBuffers();
+  const BufferQueue& video_buffers = cluster_parser_->GetVideoBuffers();
+  const TextBufferQueueMap& text_map = cluster_parser_->GetTextBuffers();
 
-      if (bytes_parsed < 0)
-        return bytes_parsed;
+  bool cluster_ended = cluster_parser_->cluster_ended();
 
-      if (bytes_parsed == 0)
-        return result;
+  if ((!audio_buffers.empty() || !video_buffers.empty() ||
+       !text_map.empty()) &&
+      !new_buffers_cb_.Run(audio_buffers, video_buffers, text_map)) {
+    return -1;
+  }
 
-      if (id != kWebMIdCluster) {
-        ChangeState(kParsingHeaders);
-        return result;
-      }
-    }
+  if (cluster_ended) {
+    ChangeState(kParsingHeaders);
+    end_of_segment_cb_.Run();
+  }
 
-    bytes_parsed = cluster_parser_->Parse(data, size);
-
-    if (bytes_parsed < 0)
-      return bytes_parsed;
-
-    // If cluster detected, immediately notify new segment if we have not
-    // already done this.
-    if (!parsing_cluster_ && bytes_parsed > 0) {
-      parsing_cluster_ = true;
-      new_segment_cb_.Run();
-    }
-
-    const BufferQueue& audio_buffers = cluster_parser_->GetAudioBuffers();
-    const BufferQueue& video_buffers = cluster_parser_->GetVideoBuffers();
-    const TextBufferQueueMap& text_map = cluster_parser_->GetTextBuffers();
-
-    cluster_ended = cluster_parser_->cluster_ended();
-
-    if ((!audio_buffers.empty() || !video_buffers.empty() ||
-         !text_map.empty()) &&
-        !new_buffers_cb_.Run(audio_buffers, video_buffers, text_map)) {
-      return -1;
-    }
-
-    if (cluster_ended) {
-      parsing_cluster_ = false;
-      end_of_segment_cb_.Run();
-    }
-
-    result += bytes_parsed;
-    data += bytes_parsed;
-    size -= bytes_parsed;
-
-    // WebMClusterParser returns 0 and |cluster_ended| is true if previously
-    // parsing an unknown-size cluster and |data| does not continue that
-    // cluster. Try parsing again in that case.
-  } while (size > 0 && (bytes_parsed > 0 || cluster_ended));
-
-  return result;
+  return bytes_parsed;
 }
 
 void WebMStreamParser::FireNeedKey(const std::string& key_id) {
   std::vector<uint8> key_id_vector(key_id.begin(), key_id.end());
   need_key_cb_.Run(kWebMEncryptInitDataType, key_id_vector);
 }
 
 }  // namespace media