VideoPlayer: Make PlaybackManager use OwnPtr

VideoPlayerWidget was keeping a reference to PlaybackManager when
changing files, so the old and new managers would both send frames to
be presented at the same time, causing it to flicker back and forth
between the two videos. However, PlaybackManager no longer relies on
event bubbling to pass events to its parent. By changing it to send
events directly to an Object, it can avoid being ref counted, so that
it will get destroyed with its containing object and stop sending
events.

Userland/Applications/VideoPlayer/VideoPlayerWidget.cpp

@@ -69,9 +69,15 @@ VideoPlayerWidget::VideoPlayerWidget(GUI::Window& window)
     m_volume_slider->set_fixed_width(100);
 }
 
+void VideoPlayerWidget::close_file()
+{
+    if (m_playback_manager)
+        m_playback_manager = nullptr;
+}
+
 void VideoPlayerWidget::open_file(StringView filename)
 {
-    auto load_file_result = Video::PlaybackManager::from_file(this, filename);
+    auto load_file_result = Video::PlaybackManager::from_file(*this, filename);
 
     if (load_file_result.is_error()) {
         on_decoding_error(load_file_result.release_error());
@@ -81,6 +87,7 @@ void VideoPlayerWidget::open_file(StringView filename)
     m_path = filename;
     update_title();
 
+    close_file();
     m_playback_manager = load_file_result.release_value();
     resume_playback();
 }

Userland/Applications/VideoPlayer/VideoPlayerWidget.h

@@ -22,6 +22,7 @@ class VideoPlayerWidget final : public GUI::Widget {
     C_OBJECT(VideoPlayerWidget)
 
 public:
+    void close_file();
     void open_file(StringView filename);
     void resume_playback();
     void pause_playback();
@@ -59,7 +60,7 @@ private:
     RefPtr<GUI::Action> m_cycle_sizing_modes_action;
     RefPtr<GUI::HorizontalSlider> m_volume_slider;
 
-    RefPtr<Video::PlaybackManager> m_playback_manager;
+    OwnPtr<Video::PlaybackManager> m_playback_manager;
 };
 
 }

Userland/Libraries/LibVideo/PlaybackManager.cpp

@@ -14,7 +14,7 @@
 
 namespace Video {
 
-DecoderErrorOr<NonnullRefPtr<PlaybackManager>> PlaybackManager::from_file(Object* event_handler, StringView filename)
+DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> PlaybackManager::from_file(Core::Object& event_handler, StringView filename)
 {
     NonnullOwnPtr<Demuxer> demuxer = TRY(MatroskaDemuxer::from_file(filename));
     auto video_tracks = demuxer->get_tracks_for_type(TrackType::Video);
@@ -24,12 +24,11 @@ DecoderErrorOr<NonnullRefPtr<PlaybackManager>> PlaybackManager::from_file(Object
 
     dbgln_if(PLAYBACK_MANAGER_DEBUG, "Selecting video track number {}", track.identifier());
 
-    NonnullOwnPtr<VideoDecoder> decoder = make<VP9::Decoder>();
-    return PlaybackManager::construct(event_handler, demuxer, track, decoder);
+    return make<PlaybackManager>(event_handler, demuxer, track, make<VP9::Decoder>());
 }
 
-PlaybackManager::PlaybackManager(Object* event_handler, NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>& decoder)
-    : Object(event_handler)
+PlaybackManager::PlaybackManager(Core::Object& event_handler, NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>&& decoder)
+    : m_event_handler(event_handler)
     , m_main_loop(Core::EventLoop::current())
     , m_demuxer(move(demuxer))
     , m_selected_video_track(video_track)
@@ -68,22 +67,10 @@ void PlaybackManager::set_playback_status(PlaybackStatus status)
             m_present_timer->stop();
         }
 
-        m_main_loop.post_event(*this, make<PlaybackStatusChangeEvent>(status, old_status));
+        m_main_loop.post_event(m_event_handler, make<PlaybackStatusChangeEvent>(status, old_status));
     }
 }
 
-void PlaybackManager::event(Core::Event& event)
-{
-    if (event.type() == DecoderErrorOccurred) {
-        auto& error_event = static_cast<DecoderErrorEvent&>(event);
-        VERIFY(error_event.error().category() != DecoderErrorCategory::EndOfStream);
-        set_playback_status(PlaybackStatus::Corrupted);
-    }
-
-    // Allow events to bubble up in all cases.
-    event.ignore();
-}
-
 void PlaybackManager::resume_playback()
 {
     set_playback_status(PlaybackStatus::Playing);
@@ -127,7 +114,7 @@ void PlaybackManager::on_decoder_error(DecoderError error)
         break;
     default:
         set_playback_status(PlaybackStatus::Corrupted);
-        m_main_loop.post_event(*this, make<DecoderErrorEvent>(move(error)));
+        m_main_loop.post_event(m_event_handler, make<DecoderErrorEvent>(move(error)));
         break;
     }
 }
@@ -153,7 +140,7 @@ void PlaybackManager::update_presented_frame()
     }
 
     if (!out_of_queued_frames && frame_item_to_display.has_value()) {
-        m_main_loop.post_event(*this, make<VideoFramePresentEvent>(frame_item_to_display->bitmap()));
+        m_main_loop.post_event(m_event_handler, make<VideoFramePresentEvent>(frame_item_to_display->bitmap()));
         m_last_present_in_media_time = current_playback_time();
         m_last_present_in_real_time = Time::now_monotonic();
         frame_item_to_display.clear();
@@ -197,7 +184,7 @@ void PlaybackManager::restart_playback()
 
 void PlaybackManager::post_decoder_error(DecoderError error)
 {
-    m_main_loop.post_event(*this, make<DecoderErrorEvent>(error));
+    m_main_loop.post_event(m_event_handler, make<DecoderErrorEvent>(error));
 }
 
 bool PlaybackManager::decode_and_queue_one_sample()

Userland/Libraries/LibVideo/PlaybackManager.h

@@ -91,15 +91,12 @@ private:
 static constexpr size_t FRAME_BUFFER_COUNT = 4;
 using VideoFrameQueue = Queue<FrameQueueItem, FRAME_BUFFER_COUNT>;
 
-class PlaybackManager : public Core::Object {
-    C_OBJECT(PlaybackManager)
-
+class PlaybackManager {
 public:
-    static DecoderErrorOr<NonnullRefPtr<PlaybackManager>> from_file(Object* event_handler, StringView file);
-    static DecoderErrorOr<NonnullRefPtr<PlaybackManager>> from_data(Object* event_handler, Span<u8> data);
+    static DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> from_file(Core::Object& event_handler, StringView file);
+    static DecoderErrorOr<NonnullOwnPtr<PlaybackManager>> from_data(Core::Object& event_handler, Span<u8> data);
 
-    PlaybackManager(Object* event_handler, NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>& decoder);
-    ~PlaybackManager() override = default;
+    PlaybackManager(Core::Object& event_handler, NonnullOwnPtr<Demuxer>& demuxer, Track video_track, NonnullOwnPtr<VideoDecoder>&& decoder);
 
     void resume_playback();
     void pause_playback();
@@ -110,7 +107,6 @@ public:
 
     u64 number_of_skipped_frames() const { return m_skipped_frames; }
 
-    void event(Core::Event& event) override;
     void on_decoder_error(DecoderError error);
 
     Time current_playback_time();
@@ -129,6 +125,7 @@ private:
     bool decode_and_queue_one_sample();
     void on_decode_timer();
 
+    Core::Object& m_event_handler;
     Core::EventLoop& m_main_loop;
 
     PlaybackStatus m_status { PlaybackStatus::Stopped };