[haiku-commits] Change in haiku[master]: ffmpeg/MediaPlayer: fix seeking in audio with cover art

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: waddlesplash <waddlesplash@xxxxxxxxx>, haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 3 May 2020 15:41:07 +0000

From Adrien Destugues <pulkomandy@xxxxxxxxx>:

Adrien Destugues has uploaded this change for review. ( 
https://review.haiku-os.org/c/haiku/+/2562 ;)


Change subject: ffmpeg/MediaPlayer: fix seeking in audio with cover art
......................................................................

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.
---
M src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp
M src/add-ons/media/plugins/ffmpeg/Utilities.h
M src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp
M src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp
4 files changed, 24 insertions(+), 9 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/62/2562/1

diff --git a/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp 
b/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp
index 23fd7ff..9aeb4ae 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp
+++ b/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp
@@ -396,12 +396,18 @@
                        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);

+                       // 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;
+
                        // TODO: Fix up interlaced video for real
                        if (frameRate == 50.0f)
                                frameRate = 25.0f;
diff --git a/src/add-ons/media/plugins/ffmpeg/Utilities.h 
b/src/add-ons/media/plugins/ffmpeg/Utilities.h
index 956adb1..6796b1c 100644
--- a/src/add-ons/media/plugins/ffmpeg/Utilities.h
+++ b/src/add-ons/media/plugins/ffmpeg/Utilities.h
@@ -214,8 +214,12 @@
 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 e969030..86a861c 100644
--- a/src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp
+++ b/src/apps/mediaplayer/media_node_framework/PlaybackManager.cpp
@@ -945,7 +945,7 @@
 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 @@
 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 @@
                        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 43f802b..ec04e50 100644
--- a/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp
+++ b/src/apps/mediaplayer/media_node_framework/video/VideoProducer.cpp
@@ -767,8 +767,12 @@
                                                // 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)) {
@@ -807,6 +811,7 @@
                                break;
                }
        }
+       fManager->SetCurrentVideoTime(INT64_MAX);
        TRACE("_FrameGeneratorThread: frame generator thread done.\n");
        return B_OK;
 }

--
To view, visit https://review.haiku-os.org/c/haiku/+/2562
To unsubscribe, or for help writing mail filters, visit 
https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: Ie1dd1358cbb41c11649103dfce52a0e1317b26f8
Gerrit-Change-Number: 2562
Gerrit-PatchSet: 1
Gerrit-Owner: Adrien Destugues <pulkomandy@xxxxxxxxx>
Gerrit-MessageType: newchange

Other related posts:

  • » [haiku-commits] Change in haiku[master]: ffmpeg/MediaPlayer: fix seeking in audio with cover art - Gerrit