[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
This commit is contained in:
Kongqun Yang 2016-09-29 17:26:33 -07:00 committed by KongQun Yang
parent f5dc908a0d
commit ac1d2692cf
3 changed files with 19 additions and 17 deletions

View File

@ -50,10 +50,9 @@ Status MultiSegmentSegmenter::FinalizeSegment() {
const uint64_t size = cluster()->Size(); const uint64_t size = cluster()->Size();
const uint64_t start_webm_timecode = cluster()->timecode(); const uint64_t start_webm_timecode = cluster()->timecode();
const uint64_t start_timescale = FromWebMTimecode(start_webm_timecode); const uint64_t start_timescale = FromWebMTimecode(start_webm_timecode);
const uint64_t length = static_cast<uint64_t>(
cluster_length_sec() * info()->time_scale());
muxer_listener()->OnNewSegment(writer_->file()->file_name(), 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."; VLOG(1) << "WEBM file '" << writer_->file()->file_name() << "' finalized.";

View File

@ -42,8 +42,8 @@ Segmenter::Segmenter(const MuxerOptions& options)
first_timestamp_(0), first_timestamp_(0),
sample_duration_(0), sample_duration_(0),
segment_payload_pos_(0), segment_payload_pos_(0),
cluster_length_sec_(0), cluster_length_in_time_scale_(0),
segment_length_sec_(0), segment_length_in_time_scale_(0),
track_id_(0) {} track_id_(0) {}
Segmenter::~Segmenter() {} Segmenter::~Segmenter() {}
@ -141,20 +141,22 @@ Status Segmenter::AddSample(scoped_refptr<MediaSample> sample) {
new_segment = true; new_segment = true;
// First frame, so no previous frame to write. // First frame, so no previous frame to write.
wrote_frame = true; 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) { if (sample->is_key_frame() || !options_.segment_sap_aligned) {
status = WriteFrame(true /* write_duration */); status = WriteFrame(true /* write_duration */);
status.Update(NewSegment(sample->pts())); status.Update(NewSegment(sample->pts()));
new_segment = true; new_segment = true;
segment_length_sec_ = 0; segment_length_in_time_scale_ = 0;
cluster_length_sec_ = 0; cluster_length_in_time_scale_ = 0;
wrote_frame = true; 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) { if (sample->is_key_frame() || !options_.fragment_sap_aligned) {
status = WriteFrame(true /* write_duration */); status = WriteFrame(true /* write_duration */);
status.Update(NewSubsegment(sample->pts())); status.Update(NewSubsegment(sample->pts()));
cluster_length_sec_ = 0; cluster_length_in_time_scale_ = 0;
wrote_frame = true; wrote_frame = true;
} }
} }
@ -187,10 +189,8 @@ Status Segmenter::AddSample(scoped_refptr<MediaSample> sample) {
// Add the sample to the durations even though we have not written the frame // 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. // yet. This is needed to make sure we split Clusters at the correct point.
// These are only used in this method. // These are only used in this method.
const double duration_sec = cluster_length_in_time_scale_ += sample->duration();
static_cast<double>(sample->duration()) / info_->time_scale(); segment_length_in_time_scale_ += sample->duration();
cluster_length_sec_ += duration_sec;
segment_length_sec_ += duration_sec;
prev_sample_ = sample; prev_sample_ = sample;
return Status::OK; return Status::OK;

View File

@ -104,7 +104,9 @@ class Segmenter {
int track_id() const { return track_id_; } int track_id() const { return track_id_; }
uint64_t segment_payload_pos() const { return segment_payload_pos_; } 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<MkvWriter> writer) = 0; virtual Status DoInitialize(std::unique_ptr<MkvWriter> writer) = 0;
virtual Status DoFinalize() = 0; virtual Status DoFinalize() = 0;
@ -154,8 +156,9 @@ class Segmenter {
// file. This is also the size of the header before the SeekHead. // file. This is also the size of the header before the SeekHead.
uint64_t segment_payload_pos_; uint64_t segment_payload_pos_;
double cluster_length_sec_; // Durations in timescale.
double segment_length_sec_; uint64_t cluster_length_in_time_scale_;
uint64_t segment_length_in_time_scale_;
int track_id_; int track_id_;