From b0db56cd39e20035d426bb30a6e7bd76318bebee Mon Sep 17 00:00:00 2001
From: Zaggy1024 <zaggy1024@gmail.com>
Date: Mon, 6 Feb 2023 04:11:32 -0600
Subject: [PATCH] VideoPlayer/LibVideo: Seek accurately when scrolling in the
 seek bar

Fast seeking does not work correctly when seeking in small increments,
so it is necessary to use accurate seeking when using certain actions.

The PlaybackManager has been changed to accept the seek mode as a
parameter to `seek_to_timestamp` to facilitate this. This now means
that it no longer has to track a seek mode preference.
---
 .../VideoPlayer/VideoPlayerWidget.cpp             | 15 +++------------
 .../Applications/VideoPlayer/VideoPlayerWidget.h  |  1 -
 Userland/Libraries/LibVideo/PlaybackManager.cpp   |  4 ++--
 Userland/Libraries/LibVideo/PlaybackManager.h     |  7 +------
 4 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/Userland/Applications/VideoPlayer/VideoPlayerWidget.cpp b/Userland/Applications/VideoPlayer/VideoPlayerWidget.cpp
index 1facab92a49..8541e631ec5 100644
--- a/Userland/Applications/VideoPlayer/VideoPlayerWidget.cpp
+++ b/Userland/Applications/VideoPlayer/VideoPlayerWidget.cpp
@@ -56,7 +56,8 @@ ErrorOr<void> VideoPlayerWidget::setup_interface()
         auto progress = value / static_cast<double>(m_seek_slider->max());
         auto duration = m_playback_manager->duration().to_milliseconds();
         Time timestamp = Time::from_milliseconds(static_cast<i64>(round(progress * static_cast<double>(duration))));
-        m_playback_manager->seek_to_timestamp(timestamp);
+        auto seek_mode_to_use = m_seek_slider->knob_dragging() ? seek_mode() : Video::PlaybackManager::SeekMode::Accurate;
+        m_playback_manager->seek_to_timestamp(timestamp, seek_mode_to_use);
         set_current_timestamp(m_playback_manager->current_playback_time());
     };
 
@@ -100,7 +101,6 @@ void VideoPlayerWidget::open_file(StringView filename)
     close_file();
     m_playback_manager = load_file_result.release_value();
     update_seek_slider_max();
-    update_seek_mode();
     resume_playback();
 }
 
@@ -277,13 +277,6 @@ void VideoPlayerWidget::set_seek_mode(Video::PlaybackManager::SeekMode seek_mode
     m_use_fast_seeking->set_checked(seek_mode == Video::PlaybackManager::SeekMode::Fast);
 }
 
-void VideoPlayerWidget::update_seek_mode()
-{
-    if (!m_playback_manager)
-        return;
-    m_playback_manager->set_seek_mode(seek_mode());
-}
-
 ErrorOr<void> VideoPlayerWidget::initialize_menubar(GUI::Window& window)
 {
     // File menu
@@ -303,9 +296,7 @@ ErrorOr<void> VideoPlayerWidget::initialize_menubar(GUI::Window& window)
 
     // FIXME: Maybe seek mode should be in an options dialog instead. The playback menu may get crowded.
     //        For now, leave it here for convenience.
-    m_use_fast_seeking = GUI::Action::create_checkable("&Fast Seeking", [&](auto&) {
-        update_seek_mode();
-    });
+    m_use_fast_seeking = GUI::Action::create_checkable("&Fast Seeking", [&](auto&) {});
     TRY(playback_menu->try_add_action(*m_use_fast_seeking));
     set_seek_mode(Video::PlaybackManager::DEFAULT_SEEK_MODE);
 
diff --git a/Userland/Applications/VideoPlayer/VideoPlayerWidget.h b/Userland/Applications/VideoPlayer/VideoPlayerWidget.h
index bdc53cb630c..a72590d95e0 100644
--- a/Userland/Applications/VideoPlayer/VideoPlayerWidget.h
+++ b/Userland/Applications/VideoPlayer/VideoPlayerWidget.h
@@ -46,7 +46,6 @@ private:
     void set_current_timestamp(Time);
     void set_time_label(Time);
     void on_decoding_error(Video::DecoderError const&);
-    void update_seek_mode();
 
     void cycle_sizing_modes();
 
diff --git a/Userland/Libraries/LibVideo/PlaybackManager.cpp b/Userland/Libraries/LibVideo/PlaybackManager.cpp
index 9a808402b81..5a49f32bd22 100644
--- a/Userland/Libraries/LibVideo/PlaybackManager.cpp
+++ b/Userland/Libraries/LibVideo/PlaybackManager.cpp
@@ -109,9 +109,9 @@ void PlaybackManager::timer_callback()
     TRY_OR_FATAL_ERROR(m_playback_handler->on_timer_callback());
 }
 
-void PlaybackManager::seek_to_timestamp(Time target_timestamp)
+void PlaybackManager::seek_to_timestamp(Time target_timestamp, SeekMode seek_mode)
 {
-    TRY_OR_FATAL_ERROR(m_playback_handler->seek(target_timestamp, m_seek_mode));
+    TRY_OR_FATAL_ERROR(m_playback_handler->seek(target_timestamp, seek_mode));
 }
 
 Optional<Time> PlaybackManager::seek_demuxer_to_most_recent_keyframe(Time timestamp, Optional<Time> earliest_available_sample)
diff --git a/Userland/Libraries/LibVideo/PlaybackManager.h b/Userland/Libraries/LibVideo/PlaybackManager.h
index 3a92b2453dc..39c42c09b16 100644
--- a/Userland/Libraries/LibVideo/PlaybackManager.h
+++ b/Userland/Libraries/LibVideo/PlaybackManager.h
@@ -98,15 +98,12 @@ public:
     void resume_playback();
     void pause_playback();
     void restart_playback();
-    void seek_to_timestamp(Time);
+    void seek_to_timestamp(Time, SeekMode = DEFAULT_SEEK_MODE);
     bool is_playing() const
     {
         return m_playback_handler->is_playing();
     }
 
-    SeekMode seek_mode() { return m_seek_mode; }
-    void set_seek_mode(SeekMode mode) { m_seek_mode = mode; }
-
     u64 number_of_skipped_frames() const { return m_skipped_frames; }
 
     void on_decoder_error(DecoderError error);
@@ -140,8 +137,6 @@ private:
 
     Time m_last_present_in_media_time = Time::zero();
 
-    SeekMode m_seek_mode = DEFAULT_SEEK_MODE;
-
     NonnullOwnPtr<Demuxer> m_demuxer;
     Track m_selected_video_track;
     NonnullOwnPtr<VideoDecoder> m_decoder;