LibVideo+VideoPlayer: Convert playback event handler to callbacks

To pass events from LibVideo's PlaybackManager to interested parties, we
currently dispatch Core::Event objects that outside callers listen for.
Dispatching events in this manner rely on a Core::EventLoop. In order to
use PlaybackManager from LibWeb, change this mechanism to instead use a
set of callbacks to inform callers of events.

Userland/Applications/VideoPlayer/VideoPlayerWidget.cpp

@@ -112,7 +112,7 @@ void VideoPlayerWidget::close_file()
 
 void VideoPlayerWidget::open_file(StringView filename)
 {
-    auto load_file_result = Video::PlaybackManager::from_file(*this, filename);
+    auto load_file_result = Video::PlaybackManager::from_file(filename);
 
     if (load_file_result.is_error()) {
         on_decoding_error(load_file_result.release_error());
@@ -121,9 +121,30 @@ void VideoPlayerWidget::open_file(StringView filename)
 
     m_path = filename;
     update_title();
-
     close_file();
+
     m_playback_manager = load_file_result.release_value();
+
+    m_playback_manager->on_video_frame = [this](auto frame) {
+        m_video_display->set_bitmap(move(frame));
+        m_video_display->repaint();
+
+        update_seek_slider_max();
+        set_current_timestamp(m_playback_manager->current_playback_time());
+    };
+
+    m_playback_manager->on_playback_state_change = [this]() {
+        update_play_pause_icon();
+    };
+
+    m_playback_manager->on_decoder_error = [this](auto error) {
+        on_decoding_error(error);
+    };
+
+    m_playback_manager->on_fatal_playback_error = [this](auto) {
+        close_file();
+    };
+
     update_seek_slider_max();
     resume_playback();
 }
@@ -240,32 +261,6 @@ void VideoPlayerWidget::set_time_label(Time timestamp)
     m_timestamp_label->set_text(string_builder.string_view());
 }
 
-void VideoPlayerWidget::event(Core::Event& event)
-{
-    if (event.type() == Video::EventType::DecoderErrorOccurred) {
-        auto& error_event = static_cast<Video::DecoderErrorEvent&>(event);
-        on_decoding_error(error_event.error());
-        error_event.accept();
-    } else if (event.type() == Video::EventType::VideoFramePresent) {
-        auto& frame_event = static_cast<Video::VideoFramePresentEvent&>(event);
-
-        m_video_display->set_bitmap(frame_event.frame());
-        m_video_display->repaint();
-
-        update_seek_slider_max();
-        set_current_timestamp(m_playback_manager->current_playback_time());
-
-        frame_event.accept();
-    } else if (event.type() == Video::EventType::PlaybackStateChange) {
-        update_play_pause_icon();
-        event.accept();
-    } else if (event.type() == Video::EventType::FatalPlaybackError) {
-        close_file();
-    }
-
-    Widget::event(event);
-}
-
 void VideoPlayerWidget::drop_event(GUI::DropEvent& event)
 {
     event.accept();

Userland/Applications/VideoPlayer/VideoPlayerWidget.h

@@ -51,8 +51,6 @@ private:
 
     void toggle_fullscreen();
 
-    void event(Core::Event&) override;
-
     virtual void drop_event(GUI::DropEvent&) override;
 
     DeprecatedString m_path;

Userland/Libraries/LibVideo/PlaybackManager.cpp

@@ -25,7 +25,27 @@ namespace Video {
         _fatal_expression.release_value();                                                           \
     })
 
-DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> PlaybackManager::from_file(Core::Object& event_handler, StringView filename)
+class DefaultPlaybackTimer final : public PlaybackTimer {
+public:
+    static ErrorOr<NonnullOwnPtr<DefaultPlaybackTimer>> create(int interval_ms, Function<void()>&& timeout_handler)
+    {
+        auto timer = TRY(Core::Timer::create_single_shot(interval_ms, move(timeout_handler)));
+        return adopt_nonnull_own_or_enomem(new (nothrow) DefaultPlaybackTimer(move(timer)));
+    }
+
+    virtual void start() override { m_timer->start(); }
+    virtual void start(int interval_ms) override { m_timer->start(interval_ms); }
+
+private:
+    explicit DefaultPlaybackTimer(NonnullRefPtr<Core::Timer> timer)
+        : m_timer(move(timer))
+    {
+    }
+
+    NonnullRefPtr<Core::Timer> m_timer;
+};
+
+DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> PlaybackManager::from_file(StringView filename, PlaybackTimerCreator playback_timer_creator)
 {
     NonnullOwnPtr<Demuxer> demuxer = TRY(Matroska::MatroskaDemuxer::from_file(filename));
     auto video_tracks = TRY(demuxer->get_tracks_for_type(TrackType::Video));
@@ -35,20 +55,24 @@ DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> PlaybackManager::from_file(Core::
 
     dbgln_if(PLAYBACK_MANAGER_DEBUG, "Selecting video track number {}", track.identifier());
 
-    return make<PlaybackManager>(event_handler, demuxer, track, make<VP9::Decoder>());
+    return make<PlaybackManager>(demuxer, track, make<VP9::Decoder>(), move(playback_timer_creator));
 }
 
-PlaybackManager::PlaybackManager(Core::Object& event_handler, NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>&& decoder)
+PlaybackManager::PlaybackManager(NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>&& decoder, PlaybackTimerCreator playback_timer_creator)
-    : m_event_handler(event_handler)
+    : m_demuxer(move(demuxer))
-    , m_main_loop(Core::EventLoop::current())
-    , m_demuxer(move(demuxer))
     , m_selected_video_track(video_track)
     , m_decoder(move(decoder))
     , m_frame_queue(make<VideoFrameQueue>())
     , m_playback_handler(make<StartingStateHandler>(*this, false))
 {
-    m_present_timer = Core::Timer::create_single_shot(0, [&] { timer_callback(); }).release_value_but_fixme_should_propagate_errors();
+    if (playback_timer_creator) {
-    m_decode_timer = Core::Timer::create_single_shot(0, [&] { on_decode_timer(); }).release_value_but_fixme_should_propagate_errors();
+        m_present_timer = playback_timer_creator(0, [&] { timer_callback(); }).release_value_but_fixme_should_propagate_errors();
+        m_decode_timer = playback_timer_creator(0, [&] { on_decode_timer(); }).release_value_but_fixme_should_propagate_errors();
+    } else {
+        m_present_timer = DefaultPlaybackTimer::create(0, [&] { timer_callback(); }).release_value_but_fixme_should_propagate_errors();
+        m_decode_timer = DefaultPlaybackTimer::create(0, [&] { on_decode_timer(); }).release_value_but_fixme_should_propagate_errors();
+    }
+
     TRY_OR_FATAL_ERROR(m_playback_handler->on_enter());
 }
 
@@ -84,9 +108,8 @@ void PlaybackManager::dispatch_fatal_error(Error error)
     dbgln_if(PLAYBACK_MANAGER_DEBUG, "Encountered fatal error: {}", error.string_literal());
     // FIXME: For threading, this will have to use a pre-allocated event to send to the main loop
     //        to be able to gracefully handle OOM.
-    VERIFY(&m_main_loop == &Core::EventLoop::current());
+    if (on_fatal_playback_error)
-    FatalPlaybackErrorEvent event { move(error) };
+        on_fatal_playback_error(move(error));
-    m_event_handler.dispatch_event(event);
 }
 
 void PlaybackManager::dispatch_decoder_error(DecoderError error)
@@ -99,14 +122,18 @@ void PlaybackManager::dispatch_decoder_error(DecoderError error)
     default:
         dbgln("Playback error encountered: {}", error.string_literal());
         TRY_OR_FATAL_ERROR(m_playback_handler->stop());
-        m_main_loop.post_event(m_event_handler, make<DecoderErrorEvent>(move(error)));
+
+        if (on_decoder_error)
+            on_decoder_error(move(error));
+
         break;
     }
 }
 
 void PlaybackManager::dispatch_new_frame(RefPtr<Gfx::Bitmap> frame)
 {
-    m_main_loop.post_event(m_event_handler, make<VideoFramePresentEvent>(frame));
+    if (on_video_frame)
+        on_video_frame(move(frame));
 }
 
 bool PlaybackManager::dispatch_frame_queue_item(FrameQueueItem&& item)
@@ -123,7 +150,8 @@ bool PlaybackManager::dispatch_frame_queue_item(FrameQueueItem&& item)
 
 void PlaybackManager::dispatch_state_change()
 {
-    m_main_loop.post_event(m_event_handler, TRY_OR_FATAL_ERROR(try_make<PlaybackStateChangeEvent>()));
+    if (on_playback_state_change)
+        on_playback_state_change();
 }
 
 void PlaybackManager::timer_callback()

Userland/Libraries/LibVideo/PlaybackManager.h

@@ -11,7 +11,6 @@
 #include <AK/NonnullOwnPtr.h>
 #include <AK/Queue.h>
 #include <AK/Time.h>
-#include <LibCore/EventLoop.h>
 #include <LibCore/SharedCircularQueue.h>
 #include <LibGfx/Bitmap.h>
 #include <LibThreading/ConditionVariable.h>
@@ -83,6 +82,14 @@ private:
 static constexpr size_t FRAME_BUFFER_COUNT = 4;
 using VideoFrameQueue = Queue<FrameQueueItem, FRAME_BUFFER_COUNT>;
 
+class PlaybackTimer {
+public:
+    virtual ~PlaybackTimer() = default;
+
+    virtual void start() = 0;
+    virtual void start(int interval_ms) = 0;
+};
+
 class PlaybackManager {
 public:
     enum class SeekMode {
@@ -92,9 +99,11 @@ public:
 
     static constexpr SeekMode DEFAULT_SEEK_MODE = SeekMode::Accurate;
 
-    static DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> from_file(Core::Object& event_handler, StringView file);
+    using PlaybackTimerCreator = Function<ErrorOr<NonnullOwnPtr<PlaybackTimer>>(int, Function<void()>)>;
 
-    PlaybackManager(Core::Object& event_handler, NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>&& decoder);
+    static DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> from_file(StringView file, PlaybackTimerCreator = nullptr);
+
+    PlaybackManager(NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>&& decoder, PlaybackTimerCreator);
 
     void resume_playback();
     void pause_playback();
@@ -110,7 +119,10 @@ public:
     Time current_playback_time();
     Time duration();
 
-    Function<void(NonnullRefPtr<Gfx::Bitmap>, Time)> on_frame_present;
+    Function<void(RefPtr<Gfx::Bitmap>)> on_video_frame;
+    Function<void()> on_playback_state_change;
+    Function<void(DecoderError)> on_decoder_error;
+    Function<void(Error)> on_fatal_playback_error;
 
 private:
     class PlaybackStateHandler;
@@ -137,9 +149,6 @@ private:
     void dispatch_state_change();
     void dispatch_fatal_error(Error);
 
-    Core::Object& m_event_handler;
-    Core::EventLoop& m_main_loop;
-
     Time m_last_present_in_media_time = Time::zero();
 
     NonnullOwnPtr<Demuxer> m_demuxer;
@@ -148,10 +157,10 @@ private:
 
     NonnullOwnPtr<VideoFrameQueue> m_frame_queue;
 
-    RefPtr<Core::Timer> m_present_timer;
+    OwnPtr<PlaybackTimer> m_present_timer;
     unsigned m_decoding_buffer_time_ms = 16;
 
-    RefPtr<Core::Timer> m_decode_timer;
+    OwnPtr<PlaybackTimer> m_decode_timer;
 
     NonnullOwnPtr<PlaybackStateHandler> m_playback_handler;
     Optional<FrameQueueItem> m_next_frame;
@@ -201,65 +210,4 @@ private:
     };
 };
 
-enum EventType : unsigned {
-    DecoderErrorOccurred = (('v' << 2) | ('i' << 1) | 'd') << 4,
-    VideoFramePresent,
-    PlaybackStateChange,
-    FatalPlaybackError,
-};
-
-class DecoderErrorEvent : public Core::Event {
-public:
-    explicit DecoderErrorEvent(DecoderError error)
-        : Core::Event(DecoderErrorOccurred)
-        , m_error(move(error))
-    {
-    }
-    virtual ~DecoderErrorEvent() = default;
-
-    DecoderError const& error() { return m_error; }
-
-private:
-    DecoderError m_error;
-};
-
-class VideoFramePresentEvent : public Core::Event {
-public:
-    VideoFramePresentEvent() = default;
-    explicit VideoFramePresentEvent(RefPtr<Gfx::Bitmap> frame)
-        : Core::Event(VideoFramePresent)
-        , m_frame(move(frame))
-    {
-    }
-    virtual ~VideoFramePresentEvent() = default;
-
-    RefPtr<Gfx::Bitmap> frame() { return m_frame; }
-
-private:
-    RefPtr<Gfx::Bitmap> m_frame;
-};
-
-class PlaybackStateChangeEvent : public Core::Event {
-public:
-    explicit PlaybackStateChangeEvent()
-        : Core::Event(PlaybackStateChange)
-    {
-    }
-    virtual ~PlaybackStateChangeEvent() = default;
-};
-
-class FatalPlaybackErrorEvent : public Core::Event {
-public:
-    explicit FatalPlaybackErrorEvent(Error error)
-        : Core::Event(FatalPlaybackError)
-        , m_error(move(error))
-    {
-    }
-    virtual ~FatalPlaybackErrorEvent() = default;
-    Error const& error() { return m_error; }
-
-private:
-    Error m_error;
-};
-
 }