Commit 05766bb9 authored by Ted Meyer's avatar Ted Meyer Committed by Commit Bot

Fix flake in FFmpegDemuxer::SeekOnVideoTrackChange

There is a small number of crashes in m70 caused by video-only
src= players being disabled when put in the background. The check for
track count size in FFmpegDemuxer::OnSelectedVideoTrackChanged wasn't
sufficient to ensure that the vector index lookup in
SeekOnVideoTrackChange() would succede, as FindAndEnableProperTracks()
can sometimes not find the tracks it's looking for.

Bug: 887648
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I23f9457da5f67887f6e7b1a63699c9c7fa0107ed
Reviewed-on: https://chromium-review.googlesource.com/1238994
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593389}
parent cda0200a
...@@ -1765,8 +1765,15 @@ void FFmpegDemuxer::SeekOnVideoTrackChange( ...@@ -1765,8 +1765,15 @@ void FFmpegDemuxer::SeekOnVideoTrackChange(
TrackChangeCB seek_completed_cb, TrackChangeCB seek_completed_cb,
DemuxerStream::Type stream_type, DemuxerStream::Type stream_type,
const std::vector<DemuxerStream*>& streams) { const std::vector<DemuxerStream*>& streams) {
DCHECK_EQ(streams.size(), 1u);
DCHECK_EQ(stream_type, DemuxerStream::VIDEO); DCHECK_EQ(stream_type, DemuxerStream::VIDEO);
if (streams.size() != 1u) {
// If FFmpegDemuxer::FindAndEnableProperTracks() was not able to find the
// selected streams in the ID->DemuxerStream map, then its possible for
// this vector to be empty. If that's the case, we don't want to bother
// with seeking, and just call the callback immediately.
std::move(seek_completed_cb).Run(stream_type, streams);
return;
}
SeekInternal(seek_to_time, SeekInternal(seek_to_time,
base::BindOnce(&FFmpegDemuxer::OnVideoSeekedForTrackChange, base::BindOnce(&FFmpegDemuxer::OnVideoSeekedForTrackChange,
weak_factory_.GetWeakPtr(), streams[0], weak_factory_.GetWeakPtr(), streams[0],
......
...@@ -160,6 +160,17 @@ class FFmpegDemuxerTest : public testing::Test { ...@@ -160,6 +160,17 @@ class FFmpegDemuxerTest : public testing::Test {
InitializeDemuxerInternal(expected_pipeline_status, base::Time()); InitializeDemuxerInternal(expected_pipeline_status, base::Time());
} }
void SeekOnVideoTrackChangePassthrough(
base::TimeDelta time,
base::OnceCallback<void(DemuxerStream::Type,
const std::vector<DemuxerStream*>&)> cb,
DemuxerStream::Type type,
const std::vector<DemuxerStream*>& streams) {
// The tests can't access private methods directly because gtest uses
// some magic macros that break the 'friend' declaration.
demuxer_->SeekOnVideoTrackChange(time, std::move(cb), type, streams);
}
MOCK_METHOD2(OnReadDoneCalled, void(int, int64_t)); MOCK_METHOD2(OnReadDoneCalled, void(int, int64_t));
struct ReadExpectation { struct ReadExpectation {
...@@ -1714,4 +1725,18 @@ TEST_F(FFmpegDemuxerTest, MultitrackMemoryUsage) { ...@@ -1714,4 +1725,18 @@ TEST_F(FFmpegDemuxerTest, MultitrackMemoryUsage) {
EXPECT_EQ(156011, demuxer_->GetMemoryUsage()); EXPECT_EQ(156011, demuxer_->GetMemoryUsage());
} }
TEST_F(FFmpegDemuxerTest, SeekOnVideoTrackChangeWontSeekIfEmpty) {
// We only care about video tracks.
CreateDemuxer("bear-320x240-video-only.webm");
InitializeDemuxer();
std::vector<DemuxerStream*> streams;
base::RunLoop loop;
SeekOnVideoTrackChangePassthrough(
base::TimeDelta(), base::BindOnce(QuitLoop, loop.QuitClosure()),
DemuxerStream::VIDEO, streams);
loop.Run();
}
} // namespace media } // namespace media
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