Commit 07e52c45 authored by Matt Wolenetz's avatar Matt Wolenetz Committed by Commit Bot

MSE: Remove media-internals logging of "uncommon" same timestamp sequences

With recent removal of LegacyByDts buffering logic, this change removes
obsolete code in SourceBufferStream that detects and logs to media-internals
when a non-keyframe is followed in append sequence by a keyframe with the same
decode timestamp. With DTS timelines of two distinct GOPs now made immaterial
in the ByPts buffering logic (below FrameProcessor), this log has even less
value than the marginal value it had when we still had LegacyByDts.

BUG=771349

Change-Id: I21c84fb1df4e314719bc8b0ae7809f2c9cbdc44f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1595172Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658838}
parent 2711c4b0
...@@ -8,13 +8,6 @@ ...@@ -8,13 +8,6 @@
namespace media { namespace media {
// static
bool SourceBufferRange::IsUncommonSameTimestampSequence(
bool prev_is_keyframe,
bool current_is_keyframe) {
return current_is_keyframe && !prev_is_keyframe;
}
SourceBufferRange::SourceBufferRange( SourceBufferRange::SourceBufferRange(
GapPolicy gap_policy, GapPolicy gap_policy,
const InterbufferDistanceCB& interbuffer_distance_cb) const InterbufferDistanceCB& interbuffer_distance_cb)
......
...@@ -37,24 +37,6 @@ class MEDIA_EXPORT SourceBufferRange { ...@@ -37,24 +37,6 @@ class MEDIA_EXPORT SourceBufferRange {
ALLOW_GAPS ALLOW_GAPS
}; };
// Sequential buffers with the same decode timestamp make sense under certain
// conditions, typically when the first buffer is a keyframe. Due to some
// atypical media append behaviors where a new keyframe might have the same
// decode timestamp as a previous non-keyframe, the playback of the sequence
// might involve some throwaway decode work. This method supports detecting
// this situation so that callers can log warnings (it returns true in this
// case only).
// For all other cases, including more typical same-DTS sequences, this method
// returns false. Examples of typical situations where DTS of two consecutive
// frames can be equal:
// - Video: VP8 Alt-Ref frames.
// - Video: IPBPBP...: DTS for I frame and for P frame can be equal.
// - Text track cues that start at same time.
// Returns true if |prev_is_keyframe| and |current_is_keyframe| indicate a
// same timestamp situation that is atypical. False is returned otherwise.
static bool IsUncommonSameTimestampSequence(bool prev_is_keyframe,
bool current_is_keyframe);
SourceBufferRange(GapPolicy gap_policy, SourceBufferRange(GapPolicy gap_policy,
const InterbufferDistanceCB& interbuffer_distance_cb); const InterbufferDistanceCB& interbuffer_distance_cb);
......
...@@ -35,15 +35,6 @@ const int kMaxGarbageCollectAlgorithmWarningLogs = 20; ...@@ -35,15 +35,6 @@ const int kMaxGarbageCollectAlgorithmWarningLogs = 20;
// Limit the number of MEDIA_LOG() logs for splice overlap trimming. // Limit the number of MEDIA_LOG() logs for splice overlap trimming.
const int kMaxAudioSpliceLogs = 20; const int kMaxAudioSpliceLogs = 20;
// Limit the number of MEDIA_LOG() logs for same DTS for non-keyframe followed
// by keyframe. Prior to relaxing the "media segments must begin with a
// keyframe" requirement, we issued decode error for this situation. That was
// likely too strict, and now that the keyframe requirement is relaxed, we have
// no knowledge of media segment boundaries here. Now, we log but don't trigger
// decode error, since we allow these sequences which may cause extra decoder
// work or other side-effects.
const int kMaxStrangeSameTimestampsLogs = 20;
// Helper method that returns true if |ranges| is sorted in increasing order, // Helper method that returns true if |ranges| is sorted in increasing order,
// false otherwise. // false otherwise.
bool IsRangeListSorted(const SourceBufferStream::RangeList& ranges) { bool IsRangeListSorted(const SourceBufferStream::RangeList& ranges) {
...@@ -718,16 +709,6 @@ bool SourceBufferStream::IsDtsMonotonicallyIncreasing( ...@@ -718,16 +709,6 @@ bool SourceBufferStream::IsDtsMonotonicallyIncreasing(
<< "Buffers did not monotonically increase."; << "Buffers did not monotonically increase.";
return false; return false;
} }
if (current_timestamp == prev_timestamp &&
SourceBufferRange::IsUncommonSameTimestampSequence(
prev_is_keyframe, current_is_keyframe)) {
LIMITED_MEDIA_LOG(DEBUG, media_log_, num_strange_same_timestamps_logs_,
kMaxStrangeSameTimestampsLogs)
<< "Detected an append sequence with keyframe following a "
"non-keyframe, both with the same decode timestamp of "
<< current_timestamp.InSecondsF();
}
} }
prev_timestamp = current_timestamp; prev_timestamp = current_timestamp;
......
...@@ -569,7 +569,6 @@ class MEDIA_EXPORT SourceBufferStream { ...@@ -569,7 +569,6 @@ class MEDIA_EXPORT SourceBufferStream {
int num_splice_logs_ = 0; int num_splice_logs_ = 0;
int num_track_buffer_gap_warning_logs_ = 0; int num_track_buffer_gap_warning_logs_ = 0;
int num_garbage_collect_algorithm_logs_ = 0; int num_garbage_collect_algorithm_logs_ = 0;
int num_strange_same_timestamps_logs_ = 0;
DISALLOW_COPY_AND_ASSIGN(SourceBufferStream); DISALLOW_COPY_AND_ASSIGN(SourceBufferStream);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment