From 771944a3f5f3c66bd71593e18506bf79e915926d Mon Sep 17 00:00:00 2001 From: Aaron Vaage Date: Wed, 31 Jan 2018 11:10:13 -0800 Subject: [PATCH] Output Empty WebVtt Segments According to the HLS+WebVTT spec, if there is no text in a segment a webvtt file with no cues can be added to the manifest. By outputting empty segments it allows the accumulated duration in the Master Playlist to better represent the duration of the text stream. Spec Reference: //tools.ietf.org/html/draft-pantos-http-live-streaming-23 Section 3.5. WebVTT Bug: #205 Change-Id: I5de01200fd9fa99c57949c773e8ee926b0f6ba8a --- .../webvtt/webvtt_pipeline_unittest.cc | 18 +++++++++---- .../media/formats/webvtt/webvtt_segmenter.cc | 27 +++++++++---------- .../media/formats/webvtt/webvtt_segmenter.h | 4 ++- .../webvtt/webvtt_segmenter_unittest.cc | 8 ++++-- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/packager/media/formats/webvtt/webvtt_pipeline_unittest.cc b/packager/media/formats/webvtt/webvtt_pipeline_unittest.cc index 3423e7ae43..565858891c 100644 --- a/packager/media/formats/webvtt/webvtt_pipeline_unittest.cc +++ b/packager/media/formats/webvtt/webvtt_pipeline_unittest.cc @@ -26,6 +26,7 @@ TEST(WebVttTextPipelineTest, SegmentedOutput) { const char* kOutput1 = "memory://output/template-1.vtt"; const char* kOutput2 = "memory://output/template-2.vtt"; const char* kOutput3 = "memory://output/template-3.vtt"; + const char* kOutput4 = "memory://output/template-4.vtt"; const char* kInputFile = "memory://input.vtt"; const char* kInput = @@ -53,8 +54,14 @@ TEST(WebVttTextPipelineTest, SegmentedOutput) { "Thank you.\n"; // Segment One - // 00:00:10.000 to 00:00:20.000 + // 00:00:00.000 to 00:00:10.000 const char* kExpectedOutput1 = + "WEBVTT\n" + "\n"; + + // Segment Two + // 00:00:10.000 to 00:00:20.000 + const char* kExpectedOutput2 = "WEBVTT\n" "\n" "1\n" @@ -62,9 +69,9 @@ TEST(WebVttTextPipelineTest, SegmentedOutput) { "This blade has a dark past.\n" "\n"; - // Segment Two + // Segment Three // 00:00:20.000 to 00:00:30.000 - const char* kExpectedOutput2 = + const char* kExpectedOutput3 = "WEBVTT\n" "\n" "1\n" @@ -80,9 +87,9 @@ TEST(WebVttTextPipelineTest, SegmentedOutput) { "You're a fool for traveling alone,\nso completely unprepared.\n" "\n"; - // Segment Three + // Segment Four // 00:00:30.000 to 00:00:40.000 - const char* kExpectedOutput3 = + const char* kExpectedOutput4 = "WEBVTT\n" "\n" "3\n" @@ -128,6 +135,7 @@ TEST(WebVttTextPipelineTest, SegmentedOutput) { ASSERT_FILE_STREQ(kOutput1, kExpectedOutput1); ASSERT_FILE_STREQ(kOutput2, kExpectedOutput2); ASSERT_FILE_STREQ(kOutput3, kExpectedOutput3); + ASSERT_FILE_STREQ(kOutput4, kExpectedOutput4); } } // namespace media } // namespace shaka diff --git a/packager/media/formats/webvtt/webvtt_segmenter.cc b/packager/media/formats/webvtt/webvtt_segmenter.cc index c1bbca89bc..d122599fa3 100644 --- a/packager/media/formats/webvtt/webvtt_segmenter.cc +++ b/packager/media/formats/webvtt/webvtt_segmenter.cc @@ -35,7 +35,10 @@ Status WebVttSegmenter::Process(std::unique_ptr stream_data) { Status WebVttSegmenter::OnFlushRequest(size_t input_stream_index) { Status status; while (status.ok() && samples_.size()) { - status.Update(OnSegmentEnd()); + // It is not possible for there to be any gaps, or else we would have + // already ended the segments before them. So just close the last remaining + // open segments. + OnSegmentEnd(samples_.top().segment); } return status.ok() ? FlushAllDownstreams() : status; } @@ -68,23 +71,19 @@ Status WebVttSegmenter::OnTextSample(std::shared_ptr sample) { samples_.push(seg_sample); } - Status status; - - while (status.ok() && samples_.size() && - samples_.top().segment < start_segment) { - // WriteNextSegment will pop elements from |samples_| which will - // eventually allow the loop to exit. - status.Update(OnSegmentEnd()); + // Output all the segments that come before the start of this cue's first + // segment. + for (; current_segment_ < start_segment; current_segment_++) { + Status status = OnSegmentEnd(current_segment_); + if (!status.ok()) { + return status; + } } - return status; + return Status::OK; } -Status WebVttSegmenter::OnSegmentEnd() { - DCHECK(samples_.size()); - - const uint64_t segment = samples_.top().segment; - +Status WebVttSegmenter::OnSegmentEnd(uint64_t segment) { Status status; while (status.ok() && samples_.size() && samples_.top().segment == segment) { status.Update( diff --git a/packager/media/formats/webvtt/webvtt_segmenter.h b/packager/media/formats/webvtt/webvtt_segmenter.h index 743215493d..d3d19a78cc 100644 --- a/packager/media/formats/webvtt/webvtt_segmenter.h +++ b/packager/media/formats/webvtt/webvtt_segmenter.h @@ -56,8 +56,10 @@ class WebVttSegmenter : public MediaHandler { Status InitializeInternal() override; Status OnTextSample(std::shared_ptr sample); - Status OnSegmentEnd(); + Status OnSegmentEnd(uint64_t segment); + + uint64_t current_segment_ = 0; uint64_t segment_duration_ms_; std::priority_queue, diff --git a/packager/media/formats/webvtt/webvtt_segmenter_unittest.cc b/packager/media/formats/webvtt/webvtt_segmenter_unittest.cc index d56933a89d..5197d872ae 100644 --- a/packager/media/formats/webvtt/webvtt_segmenter_unittest.cc +++ b/packager/media/formats/webvtt/webvtt_segmenter_unittest.cc @@ -142,7 +142,7 @@ TEST_F(WebVttSegmenterTest, CreatesSegmentsForCues) { // | | // | | [---B---] // | | -TEST_F(WebVttSegmenterTest, SkipsEmptySegments) { +TEST_F(WebVttSegmenterTest, DoesntSkipsEmptySegments) { const uint64_t kSampleDuration = kSegmentDuration / 2; { @@ -160,7 +160,11 @@ TEST_F(WebVttSegmenterTest, SkipsEmptySegments) { OnProcess(IsSegmentInfo(kStreamIndex, kStartTimeSigned, kSegmentDuration, !kSubSegment, !kEncrypted))); - // There is no segment two + // Segment two (empty) + EXPECT_CALL(*Output(kOutputIndex), + OnProcess(IsSegmentInfo( + kStreamIndex, kStartTimeSigned + kSegmentDuration, + kSegmentDuration, !kSubSegment, !kEncrypted))); // Segment Three EXPECT_CALL(*Output(kOutputIndex),