From ac1d2692cfa4106523d5fd8f550e142e810492be Mon Sep 17 00:00:00 2001 From: Kongqun Yang Date: Thu, 29 Sep 2016 17:26:33 -0700 Subject: [PATCH] [WebM] Fix corner case segment generation problem The original code accumulates sample durations in seconds (double) and then generates new segment / cluster by comparing the accumulated value with configured value. There may be loss of precision when accumulating values in double. For example, with a GOP of 96 frames and frame rate of 24 fps; the accumulated frame duration for 96 frames is 3.999999999999, which is very close to 4.0 but not 4.0. Problem would arise if segment duration is set to 4. The created segments would have a duration of 8 seconds instead of 4 seconds. The new code accumulates sample durations in uint64_t relative to input time scale; the configured segment duration will be converted to timescale for comparison. This avoids loss of precision. Change-Id: I3ae24be82a7ce45f923a6f90fea495b8b6b2e7ef --- .../formats/webm/multi_segment_segmenter.cc | 5 ++--- packager/media/formats/webm/segmenter.cc | 22 +++++++++---------- packager/media/formats/webm/segmenter.h | 9 +++++--- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/packager/media/formats/webm/multi_segment_segmenter.cc b/packager/media/formats/webm/multi_segment_segmenter.cc index 1efd3a3415..337746f16e 100644 --- a/packager/media/formats/webm/multi_segment_segmenter.cc +++ b/packager/media/formats/webm/multi_segment_segmenter.cc @@ -50,10 +50,9 @@ Status MultiSegmentSegmenter::FinalizeSegment() { const uint64_t size = cluster()->Size(); const uint64_t start_webm_timecode = cluster()->timecode(); const uint64_t start_timescale = FromWebMTimecode(start_webm_timecode); - const uint64_t length = static_cast( - cluster_length_sec() * info()->time_scale()); muxer_listener()->OnNewSegment(writer_->file()->file_name(), - start_timescale, length, size); + start_timescale, + cluster_length_in_time_scale(), size); } VLOG(1) << "WEBM file '" << writer_->file()->file_name() << "' finalized."; diff --git a/packager/media/formats/webm/segmenter.cc b/packager/media/formats/webm/segmenter.cc index 70b5e446c1..4b75be2978 100644 --- a/packager/media/formats/webm/segmenter.cc +++ b/packager/media/formats/webm/segmenter.cc @@ -42,8 +42,8 @@ Segmenter::Segmenter(const MuxerOptions& options) first_timestamp_(0), sample_duration_(0), segment_payload_pos_(0), - cluster_length_sec_(0), - segment_length_sec_(0), + cluster_length_in_time_scale_(0), + segment_length_in_time_scale_(0), track_id_(0) {} Segmenter::~Segmenter() {} @@ -141,20 +141,22 @@ Status Segmenter::AddSample(scoped_refptr sample) { new_segment = true; // First frame, so no previous frame to write. wrote_frame = true; - } else if (segment_length_sec_ >= options_.segment_duration) { + } else if (segment_length_in_time_scale_ >= + options_.segment_duration * info_->time_scale()) { if (sample->is_key_frame() || !options_.segment_sap_aligned) { status = WriteFrame(true /* write_duration */); status.Update(NewSegment(sample->pts())); new_segment = true; - segment_length_sec_ = 0; - cluster_length_sec_ = 0; + segment_length_in_time_scale_ = 0; + cluster_length_in_time_scale_ = 0; wrote_frame = true; } - } else if (cluster_length_sec_ >= options_.fragment_duration) { + } else if (cluster_length_in_time_scale_ >= + options_.fragment_duration * info_->time_scale()) { if (sample->is_key_frame() || !options_.fragment_sap_aligned) { status = WriteFrame(true /* write_duration */); status.Update(NewSubsegment(sample->pts())); - cluster_length_sec_ = 0; + cluster_length_in_time_scale_ = 0; wrote_frame = true; } } @@ -187,10 +189,8 @@ Status Segmenter::AddSample(scoped_refptr sample) { // Add the sample to the durations even though we have not written the frame // yet. This is needed to make sure we split Clusters at the correct point. // These are only used in this method. - const double duration_sec = - static_cast(sample->duration()) / info_->time_scale(); - cluster_length_sec_ += duration_sec; - segment_length_sec_ += duration_sec; + cluster_length_in_time_scale_ += sample->duration(); + segment_length_in_time_scale_ += sample->duration(); prev_sample_ = sample; return Status::OK; diff --git a/packager/media/formats/webm/segmenter.h b/packager/media/formats/webm/segmenter.h index a5697d802b..e9f6b95678 100644 --- a/packager/media/formats/webm/segmenter.h +++ b/packager/media/formats/webm/segmenter.h @@ -104,7 +104,9 @@ class Segmenter { int track_id() const { return track_id_; } uint64_t segment_payload_pos() const { return segment_payload_pos_; } - double cluster_length_sec() const { return cluster_length_sec_; } + uint64_t cluster_length_in_time_scale() const { + return cluster_length_in_time_scale_; + } virtual Status DoInitialize(std::unique_ptr writer) = 0; virtual Status DoFinalize() = 0; @@ -154,8 +156,9 @@ class Segmenter { // file. This is also the size of the header before the SeekHead. uint64_t segment_payload_pos_; - double cluster_length_sec_; - double segment_length_sec_; + // Durations in timescale. + uint64_t cluster_length_in_time_scale_; + uint64_t segment_length_in_time_scale_; int track_id_;