Skip to content

Commit

Permalink
MSE: Emit a MEDIA_LOG warning on muxed sequence append mode attempt
Browse files Browse the repository at this point in the history
Due to spec-compliant processing of muxed sequence mode appends
producing problematic results following discontinuities in the muxed
media, this change adds a warning to chrome://media-internals if such an
append is detected. I've also revived spec discussion on this by filing
w3c/media-source#186

BUG=728477

Change-Id: I78858ff9c0982eb76bcdb907a7a3fcfdb5d36b87
Reviewed-on: https://chromium-review.googlesource.com/546963
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#483240}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 3a083529baf8043e94aa394b810acdc320d953c8
  • Loading branch information
wolenetz authored and Commit Bot committed Jun 29, 2017
1 parent 1e171d7 commit 73ccecc
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 0 deletions.
6 changes: 6 additions & 0 deletions base/test_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ MATCHER_P(SegmentMissingFrames, track_id, "") {
std::string(track_id));
}

MATCHER(MuxedSequenceModeWarning, "") {
return CONTAINS_STRING(arg,
"Warning: using MSE 'sequence' AppendMode for a "
"SourceBuffer with multiple tracks");
}

MATCHER(StreamParsingFailed, "") {
return CONTAINS_STRING(arg, "Append: stream parsing failed.");
}
Expand Down
28 changes: 28 additions & 0 deletions filters/chunk_demuxer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4770,6 +4770,34 @@ TEST_F(ChunkDemuxerTest, RemovingIdMustRemoveStreams) {
EXPECT_EQ(nullptr, GetStream(DemuxerStream::VIDEO));
}

TEST_F(ChunkDemuxerTest, SequenceModeMuxedAppendShouldWarn) {
ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));

demuxer_->SetSequenceMode(kSourceId, true);
EXPECT_MEDIA_LOG(MuxedSequenceModeWarning());

AppendMuxedCluster(MuxedStreamInfo(kAudioTrackNum, "0D10K"),
MuxedStreamInfo(kVideoTrackNum, "0D10K"));
}

TEST_F(ChunkDemuxerTest, SequenceModeSingleTrackNoWarning) {
std::string audio_id = "audio1";
std::string video_id = "video1";

EXPECT_MEDIA_LOG(MuxedSequenceModeWarning()).Times(0);

ASSERT_TRUE(InitDemuxerAudioAndVideoSources(audio_id, video_id));

demuxer_->SetSequenceMode(audio_id, true);
demuxer_->SetSequenceMode(video_id, true);

// Append audio and video data into separate source ids.
ASSERT_TRUE(AppendCluster(
audio_id, GenerateSingleStreamCluster(0, 23, kAudioTrackNum, 23)));
ASSERT_TRUE(AppendCluster(
video_id, GenerateSingleStreamCluster(0, 33, kVideoTrackNum, 33)));
}

TEST_F(ChunkDemuxerTest, Mp4Vp9CodecSupport) {
ChunkDemuxer::Status expected = ChunkDemuxer::kNotSupported;
#if BUILDFLAG(USE_PROPRIETARY_CODECS)
Expand Down
11 changes: 11 additions & 0 deletions filters/frame_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace media {

const int kMaxDroppedPrerollWarnings = 10;
const int kMaxDtsBeyondPtsWarnings = 10;
const int kMaxMuxedSequenceModeWarnings = 1;

// Helper class to capture per-track details needed by a frame processor. Some
// of this information may be duplicated in the short-term in the associated
Expand Down Expand Up @@ -207,6 +208,16 @@ bool FrameProcessor::ProcessFrames(

DCHECK(!frames.empty());

if (sequence_mode_ && track_buffers_.size() > 1) {
LIMITED_MEDIA_LOG(DEBUG, media_log_, num_muxed_sequence_mode_warnings_,
kMaxMuxedSequenceModeWarnings)
<< "Warning: using MSE 'sequence' AppendMode for a SourceBuffer with "
"multiple tracks may cause loss of track synchronization. In some "
"cases, buffered range gaps and playback stalls can occur. It is "
"recommended to instead use 'segments' mode for a multitrack "
"SourceBuffer.";
}

// Implements the coded frame processing algorithm's outer loop for step 1.
// Note that ProcessFrame() implements an inner loop for a single frame that
// handles "jump to the Loop Top step to restart processing of the current
Expand Down
1 change: 1 addition & 0 deletions filters/frame_processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ class MEDIA_EXPORT FrameProcessor {
// Counters that limit spam to |media_log_| for frame processor warnings.
int num_dropped_preroll_warnings_ = 0;
int num_dts_beyond_pts_warnings_ = 0;
int num_muxed_sequence_mode_warnings_ = 0;

DISALLOW_COPY_AND_ASSIGN(FrameProcessor);
};
Expand Down

0 comments on commit 73ccecc

Please sign in to comment.