Simplify WebVtt Output

Before, the webvtt output handler was written so that it could
share code between a segmented and non-segmented handler. As
we are not worried about that right now, this change simplifies
the handler to just be about segmented output.

Change-Id: I29dbc4e3a4ffbeb7ea10e23db489ee74b398a6c4

packager/media/formats/webvtt/webvtt.gyp

@@ -17,6 +17,8 @@
         'text_padder.h',
         'text_readers.cc',
         'text_readers.h',
+        'webvtt_file_buffer.cc',
+        'webvtt_file_buffer.h',
         'webvtt_output_handler.cc',
         'webvtt_output_handler.h',
         'webvtt_parser.cc',

packager/media/formats/webvtt/webvtt_file_buffer.cc

@@ -0,0 +1,71 @@
+// Copyright 2018 Google LLC All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd
+
+#include "packager/media/formats/webvtt/webvtt_file_buffer.h"
+
+#include "packager/media/base/text_sample.h"
+#include "packager/media/formats/webvtt/webvtt_timestamp.h"
+
+namespace shaka {
+namespace media {
+namespace {
+const char* kHeader = "WEBVTT\n\n";
+}
+
+WebVttFileBuffer::WebVttFileBuffer() {
+  // Make sure we start with the same state that we would end up with if
+  // the caller reset our state.
+  Reset();
+}
+
+void WebVttFileBuffer::Reset() {
+  sample_count_ = 0;
+
+  buffer_.clear();
+  buffer_.append(kHeader);
+}
+
+void WebVttFileBuffer::Append(const TextSample& sample) {
+  DCHECK_GT(buffer_.size(), 0u) << "The buffer should at least have a header";
+
+  sample_count_++;
+
+  // Ids are optional
+  if (sample.id().length()) {
+    buffer_.append(sample.id());
+    buffer_.append("\n");  // end of id
+  }
+
+  // Write the times that the sample elapses.
+  buffer_.append(MsToWebVttTimestamp(sample.start_time()));
+  buffer_.append(" --> ");
+  buffer_.append(MsToWebVttTimestamp(sample.EndTime()));
+
+  // Settings are optional
+  if (sample.settings().length()) {
+    buffer_.append(" ");
+    buffer_.append(sample.settings());
+  }
+  buffer_.append("\n");  // end of time & settings
+
+  buffer_.append(sample.payload());
+  buffer_.append("\n");  // end of payload
+  buffer_.append("\n");  // end of sample
+}
+
+bool WebVttFileBuffer::WriteTo(File* file) {
+  DCHECK(file);
+  DCHECK_GT(buffer_.size(), 0u) << "The buffer should at least have a header";
+
+  const int written = file->Write(buffer_.c_str(), buffer_.size());
+  if (written < 0) {
+    return false;
+  }
+
+  return static_cast<size_t>(written) == buffer_.size();
+}
+}  // namespace media
+}  // namespace shaka

packager/media/formats/webvtt/webvtt_file_buffer.h

@@ -0,0 +1,43 @@
+// Copyright 2018 Google LLC All rights reserved.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file or at
+// https://developers.google.com/open-source/licenses/bsd
+
+#ifndef PACKAGER_MEDIA_FORMATS_WEBVTT_WEBVTT_FILE_BUFFER_H_
+#define PACKAGER_MEDIA_FORMATS_WEBVTT_WEBVTT_FILE_BUFFER_H_
+
+#include <string>
+
+#include "packager/file/file.h"
+
+namespace shaka {
+namespace media {
+
+class TextSample;
+
+// A class to abstract writing a webvtt file to disk. This class will handle
+// all the formatting requirements for a webvtt file.
+class WebVttFileBuffer {
+ public:
+  WebVttFileBuffer();
+  virtual ~WebVttFileBuffer() = default;
+
+  void Reset();
+  void Append(const TextSample& sample);
+
+  bool WriteTo(File* file);
+
+  // Get the number of samples that have been appended to this file.
+  size_t sample_count() const { return sample_count_; }
+
+ private:
+  // TODO(vaage): Add missing deleted copy and assign constructors.
+  std::string buffer_;
+  size_t sample_count_ = 0;
+};
+
+}  // namespace media
+}  // namespace shaka
+
+#endif  // PACKAGER_MEDIA_FORMATS_WEBVTT_WEBVTT_FILE_BUFFER_H_

packager/media/formats/webvtt/webvtt_output_handler.cc

@@ -6,10 +6,13 @@
 
 #include "packager/media/formats/webvtt/webvtt_output_handler.h"
 
+#include <algorithm>  // needed for min and max
+
 #include "packager/base/logging.h"
 #include "packager/file/file.h"
+#include "packager/file/file_closer.h"
 #include "packager/media/base/muxer_util.h"
-#include "packager/media/formats/webvtt/webvtt_timestamp.h"
+#include "packager/status_macros.h"
 
 namespace shaka {
 namespace media {
@@ -17,79 +20,18 @@ namespace {
 double kMillisecondsToSeconds = 1000.0;
 }  // namespace
 
-void WebVttOutputHandler::WriteCue(const std::string& id,
-                                   uint64_t start_ms,
-                                   uint64_t end_ms,
-                                   const std::string& settings,
-                                   const std::string& payload) {
-  DCHECK_GT(payload.size(), 0u);
-
-  // Build a block of text that makes up the cue so that we can use a loop to
-  // write all the lines.
-  const std::string start = MsToWebVttTimestamp(start_ms);
-  const std::string end = MsToWebVttTimestamp(end_ms);
-
-  // Ids are optional
-  if (id.length()) {
-    buffer_.append(id);
-    buffer_.append("\n");  // end of id
-  }
-
-  buffer_.append(start);
-  buffer_.append(" --> ");
-  buffer_.append(end);
-
-  // Settings are optional
-  if (settings.length()) {
-    buffer_.append(" ");
-    buffer_.append(settings);
-  }
-  buffer_.append("\n");  // end of time & settings
-
-  buffer_.append(payload);
-  buffer_.append("\n");  // end of payload
-  buffer_.append("\n");  // end of cue
-}
-
-Status WebVttOutputHandler::WriteSegmentToFile(const std::string& filename) {
-  // Need blank line between "WEBVTT" and the first cue
-  const std::string WEBVTT_HEADER = "WEBVTT\n\n";
-
-  File* file = File::Open(filename.c_str(), "w");
-
-  if (file == nullptr) {
-    return Status(error::FILE_FAILURE, "Failed to open " + filename);
-  }
-
-  size_t written;
-  written = file->Write(WEBVTT_HEADER.c_str(), WEBVTT_HEADER.size());
-  if (written != WEBVTT_HEADER.size()) {
-    return Status(error::FILE_FAILURE, "Failed to write webvtt header to file");
-  }
-
-  written = file->Write(buffer_.c_str(), buffer_.size());
-  if (written != buffer_.size()) {
-    return Status(error::FILE_FAILURE,
-                  "Failed to write webvtt cotnent to file");
-  }
-
-  // Since all the cues have been written to disk, there is no reason to hold
-  // onto that information anymore.
-  buffer_.clear();
-
-  bool closed = file->Close();
-  if (!closed) {
-    return Status(error::FILE_FAILURE, "Failed to close " + filename);
-  }
+WebVttTextOutputHandler::WebVttTextOutputHandler(
+    const MuxerOptions& muxer_options,
+    std::unique_ptr<MuxerListener> muxer_listener)
+    : muxer_options_(muxer_options),
+      muxer_listener_(std::move(muxer_listener)) {}
 
+Status WebVttTextOutputHandler::InitializeInternal() {
   return Status::OK;
 }
 
-Status WebVttOutputHandler::InitializeInternal() {
-  return Status::OK;
-}
-
-Status WebVttOutputHandler::Process(std::unique_ptr<StreamData> stream_data) {
+Status WebVttTextOutputHandler::Process(
+    std::unique_ptr<StreamData> stream_data) {
   switch (stream_data->stream_data_type) {
     case StreamDataType::kStreamInfo:
       return OnStreamInfo(*stream_data->stream_info);
@@ -98,81 +40,19 @@ Status WebVttOutputHandler::Process(std::unique_ptr<StreamData> stream_data) {
     case StreamDataType::kCueEvent:
       return OnCueEvent(*stream_data->cue_event);
     case StreamDataType::kTextSample:
-      return OnTextSample(*stream_data->text_sample);
+      OnTextSample(*stream_data->text_sample);
+      return Status::OK;
     default:
       return Status(error::INTERNAL_ERROR,
                     "Invalid stream data type for this handler");
   }
 }
 
-Status WebVttOutputHandler::OnFlushRequest(size_t input_stream_index) {
-  OnStreamEnd();
-  return Status::OK;
-}
+Status WebVttTextOutputHandler::OnFlushRequest(size_t input_stream_index) {
+  DCHECK_EQ(buffer_.sample_count(), 0u)
+      << "There should have been a segment info before flushing that would "
+         "have cleared out all the samples.";
 
-WebVttSegmentedOutputHandler::WebVttSegmentedOutputHandler(
-    const MuxerOptions& muxer_options,
-    std::unique_ptr<MuxerListener> muxer_listener)
-    : muxer_options_(muxer_options),
-      muxer_listener_(std::move(muxer_listener)) {}
-
-Status WebVttSegmentedOutputHandler::OnStreamInfo(const StreamInfo& info) {
-  muxer_listener_->OnMediaStart(muxer_options_, info, info.time_scale(),
-                                MuxerListener::kContainerText);
-  return Status::OK;
-}
-
-Status WebVttSegmentedOutputHandler::OnSegmentInfo(const SegmentInfo& info) {
-  total_duration_ms_ += info.duration;
-
-  const std::string& segment_template = muxer_options_.segment_template;
-  const uint32_t index = segment_index_++;
-  const uint64_t start = info.start_timestamp;
-  const uint64_t duration = info.duration;
-  const uint32_t bandwidth = 0;
-
-  // Write all the samples to the file.
-  const std::string filename =
-      GetSegmentName(segment_template, start, index, bandwidth);
-
-  // Write everything to the file before telling the manifest so that the
-  // file will exist on disk.
-  Status write_status = WriteSegmentToFile(filename);
-  if (!write_status.ok()) {
-    return write_status;
-  }
-
-  // Update the manifest with our new file.
-  const uint64_t size = File::GetFileSize(filename.c_str());
-  muxer_listener_->OnNewSegment(filename, start, duration, size);
-
-  return Status::OK;
-}
-
-Status WebVttSegmentedOutputHandler::OnCueEvent(const CueEvent& event) {
-  double timestamp_seconds = event.time_in_seconds;
-  double timestamp_ms = timestamp_seconds * kMillisecondsToSeconds;
-  uint64_t timestamp = static_cast<uint64_t>(timestamp_ms);
-  muxer_listener_->OnCueEvent(timestamp, event.cue_data);
-  return Status::OK;
-}
-
-Status WebVttSegmentedOutputHandler::OnTextSample(const TextSample& sample) {
-  const std::string& id = sample.id();
-  const uint64_t start_ms = sample.start_time();
-  const uint64_t end_ms = sample.EndTime();
-  const std::string& settings = sample.settings();
-  const std::string& payload = sample.payload();
-
-  // Only write cues that have payloads. Cue without payloads won't present
-  // any information to the user.
-  if (payload.size()) {
-    WriteCue(id, start_ms, end_ms, settings, payload);
-  }
-  return Status::OK;
-}
-
-Status WebVttSegmentedOutputHandler::OnStreamEnd() {
   const float duration_ms = static_cast<float>(total_duration_ms_);
   const float duration_seconds = duration_ms / 1000.0f;
 
@@ -182,5 +62,60 @@ Status WebVttSegmentedOutputHandler::OnStreamEnd() {
   return Status::OK;
 }
 
+Status WebVttTextOutputHandler::OnStreamInfo(const StreamInfo& info) {
+  muxer_listener_->OnMediaStart(muxer_options_, info, info.time_scale(),
+                                MuxerListener::kContainerText);
+  return Status::OK;
+}
+
+Status WebVttTextOutputHandler::OnSegmentInfo(const SegmentInfo& info) {
+  total_duration_ms_ += info.duration;
+
+  const std::string& segment_template = muxer_options_.segment_template;
+  const uint32_t index = segment_index_++;
+  const uint64_t start = info.start_timestamp;
+  const uint64_t duration = info.duration;
+  const uint32_t bandwidth = muxer_options_.bandwidth;
+
+  const std::string filename =
+      GetSegmentName(segment_template, start, index, bandwidth);
+
+  // Write everything to the file before telling the manifest so that the
+  // file will exist on disk.
+  std::unique_ptr<File, FileCloser> file(File::Open(filename.c_str(), "w"));
+
+  if (!file) {
+    return Status(error::FILE_FAILURE, "Failed to open " + filename);
+  }
+
+  buffer_.WriteTo(file.get());
+  buffer_.Reset();
+
+  if (!file.release()->Close()) {
+    return Status(error::FILE_FAILURE, "Failed to close " + filename);
+  }
+
+  // Update the manifest with our new file.
+  const uint64_t size = File::GetFileSize(filename.c_str());
+  muxer_listener_->OnNewSegment(filename, start, duration, size);
+
+  return Status::OK;
+}
+
+Status WebVttTextOutputHandler::OnCueEvent(const CueEvent& event) {
+  double timestamp_seconds = event.time_in_seconds;
+  double timestamp_ms = timestamp_seconds * kMillisecondsToSeconds;
+  uint64_t timestamp = static_cast<uint64_t>(timestamp_ms);
+  muxer_listener_->OnCueEvent(timestamp, event.cue_data);
+  return Status::OK;
+}
+
+void WebVttTextOutputHandler::OnTextSample(const TextSample& sample) {
+  // Skip empty samples. It is normal to see empty samples as earlier in the
+  // pipeline we pad the stream to remove gaps.
+  if (sample.payload().size()) {
+    buffer_.Append(sample);
+  }
+}
 }  // namespace media
 }  // namespace shaka

packager/media/formats/webvtt/webvtt_output_handler.h

@@ -14,61 +14,28 @@
 #include "packager/media/base/media_handler.h"
 #include "packager/media/base/muxer_options.h"
 #include "packager/media/event/muxer_listener.h"
+#include "packager/media/formats/webvtt/webvtt_file_buffer.h"
 
 namespace shaka {
 namespace media {
 
-// WebVttOutputHandler is the base class for all WebVtt text output handlers.
-// It handles taking in the samples and writing the text out, but relies on
-// sub classes to handle the logic of when and where to write the information.
-class WebVttOutputHandler : public MediaHandler {
+class WebVttTextOutputHandler : public MediaHandler {
  public:
-  WebVttOutputHandler() = default;
-  virtual ~WebVttOutputHandler() = default;
-
- protected:
-  virtual Status OnStreamInfo(const StreamInfo& info) = 0;
-  virtual Status OnSegmentInfo(const SegmentInfo& info) = 0;
-  virtual Status OnCueEvent(const CueEvent& event) = 0;
-  virtual Status OnTextSample(const TextSample& sample) = 0;
-  virtual Status OnStreamEnd() = 0;
-
-  // Top level functions for output. These functions should be used by
-  // subclasses to write to files.
-  void WriteCue(const std::string& id,
-                uint64_t start,
-                uint64_t end,
-                const std::string& settings,
-                const std::string& payload);
-  // Writes the current state of the current segment to disk. This will
-  // reset the internal state and set it up for the next segment.
-  Status WriteSegmentToFile(const std::string& filename);
+  WebVttTextOutputHandler(const MuxerOptions& muxer_options,
+                          std::unique_ptr<MuxerListener> muxer_listener);
+  // TODO(vaage): Add missing default destructor.
 
  private:
-  WebVttOutputHandler(const WebVttOutputHandler&) = delete;
-  WebVttOutputHandler& operator=(const WebVttOutputHandler&) = delete;
+  // TODO(vaage): Add missing deleted copy and assign constructors.
 
   Status InitializeInternal() override;
   Status Process(std::unique_ptr<StreamData> stream_data) override;
   Status OnFlushRequest(size_t input_stream_index) override;
 
-  // A buffer of characters waiting to be written to a file.
-  std::string buffer_;
-};
-
-// This WebVttt output handler should only be used when the source WebVTT
-// content needs to be segmented across multiple files.
-class WebVttSegmentedOutputHandler : public WebVttOutputHandler {
- public:
-  WebVttSegmentedOutputHandler(const MuxerOptions& muxer_options,
-                               std::unique_ptr<MuxerListener> muxer_listener);
-
- private:
-  Status OnStreamInfo(const StreamInfo& info) override;
-  Status OnSegmentInfo(const SegmentInfo& info) override;
-  Status OnCueEvent(const CueEvent& event) override;
-  Status OnTextSample(const TextSample& sample) override;
-  Status OnStreamEnd() override;
+  Status OnStreamInfo(const StreamInfo& info);
+  Status OnSegmentInfo(const SegmentInfo& info);
+  Status OnCueEvent(const CueEvent& event);
+  void OnTextSample(const TextSample& sample);
 
   Status OnSegmentEnded();
 
@@ -80,6 +47,8 @@ class WebVttSegmentedOutputHandler : public WebVttOutputHandler {
   // Sum together all segment durations so we know how long the stream is.
   uint64_t total_duration_ms_ = 0;
   uint32_t segment_index_ = 0;
+
+  WebVttFileBuffer buffer_;
 };
 
 }  // namespace media

packager/media/formats/webvtt/webvtt_output_handler_unittest.cc

@@ -47,14 +47,14 @@ class WebVttSegmentedOutputTest : public MediaHandlerTestBase {
     std::unique_ptr<MockMuxerListener> muxer_listener(new MockMuxerListener);
     muxer_listener_ = muxer_listener.get();
 
-    out_ = std::make_shared<WebVttSegmentedOutputHandler>(
-        muxer_options, std::move(muxer_listener));
+    out_ = std::make_shared<WebVttTextOutputHandler>(muxer_options,
+                                                     std::move(muxer_listener));
 
     ASSERT_OK(SetUpAndInitializeGraph(out_, kInputCount, kOutputCount));
   }
 
   MockMuxerListener* muxer_listener_ = nullptr;
-  std::shared_ptr<WebVttSegmentedOutputHandler> out_;
+  std::shared_ptr<WebVttTextOutputHandler> out_;
 };
 
 TEST_F(WebVttSegmentedOutputTest, WithNoSegmentAndWithNoSamples) {

packager/packager.cc

@@ -493,7 +493,7 @@ Status CreateHlsTextJob(const StreamDescriptor& stream,
   MuxerOptions muxer_options = CreateMuxerOptions(stream, packaging_params);
   muxer_options.bandwidth = stream.bandwidth ? stream.bandwidth : 256;
 
-  auto output = std::make_shared<WebVttSegmentedOutputHandler>(
+  auto output = std::make_shared<WebVttTextOutputHandler>(
       muxer_options, std::move(muxer_listener));
 
   std::unique_ptr<FileReader> reader;