From 1c889d23fed8c85cf6f17885fc5bc9af5c5970ff Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Sun, 3 May 2020 17:35:25 +0200 Subject: [PATCH] ffmpeg/MediaPlayer: fix seeking in audio with cover art MediaPlayer is basing its time on both the audio and video frames. This doesn't go so well when there is a single video frame, resulting in the whole file being one single "timepoint". Avoid this problem by having the video decoder set the frame time to "infinite" when the video stream is finished, which allows for the audio timings to be used in this case. Also improve the framerate handling of ffmpeg further, to avoid MediaPlayer trying to frameskip at 90000fps (it would give up frameskipping after a few frames and eventually notice that the next frame was the end of stream, but still, not very clean). Now we report an FPS of 0 instead, which should make it clear to applications what to expect from single-frame files. It seems the cover art is now hidden by a black screen, I'm not sure why. But I'll leave debugging this for another day. Fixes #13622. Change-Id: Ie1dd1358cbb41c11649103dfce52a0e1317b26f8 Reviewed-on: https://review.haiku-os.org/c/haiku/+/2562 Reviewed-by: Adrien Destugues --- .../media/plugins/ffmpeg/AVFormatReader.cpp | 14 ++++++++------ src/add-ons/media/plugins/ffmpeg/Utilities.h | 8 ++++++-- .../media_node_framework/PlaybackManager.cpp | 6 +++--- .../media_node_framework/video/VideoProducer.cpp | 7 ++++++- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp b/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp index 23fd7ff55c..6f92bf9a96 100644 --- a/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp +++ b/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp @@ -396,15 +396,17 @@ StreamBase::FrameRate() const break; case AVMEDIA_TYPE_VIDEO: { - AVRational frame_rate = av_guess_frame_rate(NULL, fStream, NULL); - if (frame_rate.den != 0 && frame_rate.num != 0) - frameRate = av_q2d(frame_rate); + AVRational frameRateFrac = av_guess_frame_rate(NULL, fStream, NULL); + if (frameRateFrac.den != 0 && frameRateFrac.num != 0) + frameRate = av_q2d(frameRateFrac); else if (fStream->time_base.den != 0 && fStream->time_base.num != 0) frameRate = 1 / av_q2d(fStream->time_base); - // TODO: Fix up interlaced video for real - if (frameRate == 50.0f) - frameRate = 25.0f; + // Catch the obviously wrong default framerate when ffmpeg cannot + // guess anything because there are not two frames to compute a + // framerate + if (fStream->nb_frames < 2 && frameRate == 90000.0f) + return 0.0f; break; } default: diff --git a/src/add-ons/media/plugins/ffmpeg/Utilities.h b/src/add-ons/media/plugins/ffmpeg/Utilities.h index d43084928d..32674fbf0d 100644 --- a/src/add-ons/media/plugins/ffmpeg/Utilities.h +++ b/src/add-ons/media/plugins/ffmpeg/Utilities.h @@ -215,8 +215,12 @@ CalculateBytesPerRowWithColorSpaceAndVideoWidth(color_space colorSpace, int vide inline void ConvertAVCodecContextToVideoFrameRate(AVCodecContext& contextIn, float& frameRateOut) { - // assert that av_q2d(contextIn.time_base) > 0 and computable - assert(contextIn.time_base.num > 0); + // A framerate of 0 is allowed for single-frame "video" (cover art, for + // example) + if (contextIn.time_base.num == 0) + frameRateOut = 0.0f; + + // assert that we can compute something assert(contextIn.time_base.den > 0); // The following code is based on private get_fps() function of FFmpeg's diff --git a/src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp b/src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp index e9690301a6..86a861c34a 100644 --- a/src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp +++ b/src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp @@ -945,7 +945,7 @@ PlaybackManager::PlaylistTimeForFrame(int64 frame) const void PlaybackManager::SetCurrentAudioTime(bigtime_t time) { -TRACE("PlaybackManager::SetCurrentAudioTime(%lld)\n", time); + TRACE("PlaybackManager::SetCurrentAudioTime(%lld)\n", time); bigtime_t lastFrameTime = _TimeForLastFrame(); fCurrentAudioTime = time; bigtime_t newLastFrameTime = _TimeForLastFrame(); @@ -972,7 +972,7 @@ PlaybackManager::SetCurrentVideoFrame(int64 frame) void PlaybackManager::SetCurrentVideoTime(bigtime_t time) { -TRACE("PlaybackManager::SetCurrentVideoTime(%lld)\n", time); + TRACE("PlaybackManager::SetCurrentVideoTime(%lld)\n", time); bigtime_t lastFrameTime = _TimeForLastFrame(); fCurrentVideoTime = time; bigtime_t newLastFrameTime = _TimeForLastFrame(); @@ -1546,7 +1546,7 @@ frameCount); frame = startFrame + index; break; } -TRACE("PlaybackManager::_FrameForRangeFrame() done: %ld\n", frame); + TRACE("PlaybackManager::_FrameForRangeFrame() done: %" PRId64 "\n", frame); return frame; } diff --git a/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp b/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp index 863b41fe82..b5c2b86703 100644 --- a/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp +++ b/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp @@ -775,8 +775,12 @@ VideoProducer::_FrameGeneratorThread() // to color space! memset(buffer->Data(), 0, h->size_used); err = B_OK; - } else if (err == B_LAST_BUFFER_ERROR) + } else if (err == B_LAST_BUFFER_ERROR) { + wasCached = true; + // Don't send the buffer: we don't have a buffer + err = B_OK; running = false; + } // Send the buffer on down to the consumer if (wasCached || ((err = SendBuffer(buffer, fOutput.source, fOutput.destination)) != B_OK)) { @@ -815,6 +819,7 @@ VideoProducer::_FrameGeneratorThread() break; } } + fManager->SetCurrentVideoTime(INT64_MAX); TRACE("_FrameGeneratorThread: frame generator thread done.\n"); return B_OK; }