Skip to content

Commit

Permalink
[media] FFmpegDemuxer: Detect seek failures
Browse files Browse the repository at this point in the history
Previously the result from av_seek_frame() was ignored. In the (rare)
case of a seek failure, this allowed demuxing to continue from the
previous location while trying to render from the seek location,
resulting in a confused UI state.

With this change, seek failures result in playback error.

Bug: 1424380
Change-Id: I6d5d80f5bb5132900c153db22a0079e76f074568
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4416363
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1130101}
  • Loading branch information
Dan Sanders authored and Chromium LUCI CQ committed Apr 13, 2023
1 parent 4094adf commit 553e067
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 10 deletions.
Binary file modified content/test/data/media/bear.flac
Binary file not shown.
22 changes: 15 additions & 7 deletions media/filters/ffmpeg_demuxer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ void FFmpegDemuxer::Seek(base::TimeDelta time, PipelineStatusCallback cb) {
DCHECK(!pending_seek_cb_);
TRACE_EVENT_ASYNC_BEGIN0("media", "FFmpegDemuxer::Seek", this);
pending_seek_cb_ = std::move(cb);
SeekInternal(time, base::BindOnce(&FFmpegDemuxer::OnSeekFrameSuccess,
SeekInternal(time, base::BindOnce(&FFmpegDemuxer::OnSeekFrameDone,
weak_factory_.GetWeakPtr()));
}

Expand All @@ -1073,7 +1073,7 @@ bool FFmpegDemuxer::IsSeekable() const {
}

void FFmpegDemuxer::SeekInternal(base::TimeDelta time,
base::OnceClosure seek_cb) {
base::OnceCallback<void(int)> seek_cb) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());

// FFmpeg requires seeks to be adjusted according to the lowest starting time.
Expand Down Expand Up @@ -1115,10 +1115,10 @@ void FFmpegDemuxer::SeekInternal(base::TimeDelta time,
const AVStream* seeking_stream = demux_stream->av_stream();
DCHECK(seeking_stream);

blocking_task_runner_->PostTaskAndReply(
blocking_task_runner_->PostTaskAndReplyWithResult(
FROM_HERE,
base::BindOnce(base::IgnoreResult(&av_seek_frame),
glue_->format_context(), seeking_stream->index,
base::BindOnce(&av_seek_frame, glue_->format_context(),
seeking_stream->index,
ConvertToTimeBase(seeking_stream->time_base, seek_time),
// Always seek to a timestamp <= to the desired timestamp.
AVSEEK_FLAG_BACKWARD),
Expand Down Expand Up @@ -1656,7 +1656,7 @@ FFmpegDemuxerStream* FFmpegDemuxer::FindPreferredStreamForSeeking(
return nullptr;
}

void FFmpegDemuxer::OnSeekFrameSuccess() {
void FFmpegDemuxer::OnSeekFrameDone(int result) {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
DCHECK(pending_seek_cb_);

Expand All @@ -1666,6 +1666,12 @@ void FFmpegDemuxer::OnSeekFrameSuccess() {
return;
}

if (result < 0) {
MEDIA_LOG(ERROR, media_log_) << GetDisplayName() << ": demuxer seek failed";
RunPendingSeekCB(PIPELINE_ERROR_READ);
return;
}

// Tell streams to flush buffers due to seeking.
for (const auto& stream : streams_) {
if (stream)
Expand Down Expand Up @@ -1729,8 +1735,10 @@ void FFmpegDemuxer::OnEnabledAudioTracksChanged(

void FFmpegDemuxer::OnVideoSeekedForTrackChange(
DemuxerStream* video_stream,
base::OnceClosure seek_completed_cb) {
base::OnceClosure seek_completed_cb,
int result) {
static_cast<FFmpegDemuxerStream*>(video_stream)->FlushBuffers(true);
// TODO(crbug.com/1424380): Report seek failures for track changes too.
std::move(seek_completed_cb).Run();
}

Expand Down
8 changes: 5 additions & 3 deletions media/filters/ffmpeg_demuxer.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer {
FFmpegDemuxerStream* FindPreferredStreamForSeeking(base::TimeDelta seek_time);

// FFmpeg callbacks during seeking.
void OnSeekFrameSuccess();
void OnSeekFrameDone(int result);

// FFmpeg callbacks during reading + helper method to initiate reads.
void ReadFrameIfNeeded();
Expand Down Expand Up @@ -334,9 +334,11 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer {

void SetLiveness(StreamLiveness liveness);

void SeekInternal(base::TimeDelta time, base::OnceClosure seek_cb);
void SeekInternal(base::TimeDelta time,
base::OnceCallback<void(int)> seek_cb);
void OnVideoSeekedForTrackChange(DemuxerStream* video_stream,
base::OnceClosure seek_completed_cb);
base::OnceClosure seek_completed_cb,
int result);
void SeekOnVideoTrackChange(base::TimeDelta seek_to_time,
TrackChangeCB seek_completed_cb,
DemuxerStream::Type stream_type,
Expand Down
Binary file modified media/test/data/bear.flac
Binary file not shown.

0 comments on commit 553e067

Please sign in to comment.