From 39bbf17cafd4c9550c078bd72a3d47885b85f554 Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sat, 12 Aug 2023 23:06:22 +0200 Subject: [PATCH] cellRec: fix width of encoder frames Turns out the pitch was accidentally used as width, leading to an out of bounds read/write. I kept the pitch in the struct for completeness' sake. It may be needed later, if only for error checks. --- rpcs3/Emu/Cell/Modules/cellRec.cpp | 8 ++++---- rpcs3/Emu/RSX/GL/GLPresent.cpp | 2 +- rpcs3/Emu/RSX/GSFrameBase.h | 4 ++-- rpcs3/Emu/RSX/VK/VKPresent.cpp | 2 +- rpcs3/rpcs3qt/camera_settings_dialog.cpp | 1 + rpcs3/rpcs3qt/gs_frame.cpp | 6 +++--- rpcs3/rpcs3qt/gs_frame.h | 4 ++-- rpcs3/util/image_sink.h | 7 ++++--- rpcs3/util/media_utils.cpp | 4 ++-- rpcs3/util/media_utils.h | 2 +- rpcs3/util/video_provider.cpp | 4 ++-- rpcs3/util/video_provider.h | 2 +- 12 files changed, 24 insertions(+), 22 deletions(-) diff --git a/rpcs3/Emu/Cell/Modules/cellRec.cpp b/rpcs3/Emu/Cell/Modules/cellRec.cpp index 77064e248f..f3e4e7ea95 100644 --- a/rpcs3/Emu/Cell/Modules/cellRec.cpp +++ b/rpcs3/Emu/Cell/Modules/cellRec.cpp @@ -158,14 +158,14 @@ public: has_error = false; } - void add_frame(std::vector& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) override + void add_frame(std::vector& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) override { std::lock_guard lock(m_mtx); if (m_flush) return; - m_frames_to_encode.emplace_back(timestamp_ms, width, height, pixel_format, std::move(frame)); + m_frames_to_encode.emplace_back(timestamp_ms, pitch, width, height, pixel_format, std::move(frame)); } encoder_frame get_frame() @@ -587,7 +587,7 @@ void rec_info::start_image_provider() { std::vector frame(frame_size); std::memcpy(frame.data(), video_input_buffer.get_ptr(), frame.size()); - encoder->add_frame(frame, input_format.pitch, input_format.height, input_format.av_pixel_format, timestamp_ms); + encoder->add_frame(frame, input_format.pitch, input_format.width, input_format.height, input_format.av_pixel_format, timestamp_ms); } } @@ -680,7 +680,7 @@ void rec_info::stop_image_provider(bool flush) { const usz pos = (start_offset + i) % video_ringbuffer.size(); utils::image_sink::encoder_frame& frame_data = video_ringbuffer[pos]; - encoder->add_frame(frame_data.data, frame_data.width, frame_data.height, frame_data.av_pixel_format, encoder->get_timestamp_ms(frame_data.pts - start_pts)); + encoder->add_frame(frame_data.data, frame_data.pitch, frame_data.width, frame_data.height, frame_data.av_pixel_format, encoder->get_timestamp_ms(frame_data.pts - start_pts)); // TODO: add audio data to encoder } diff --git a/rpcs3/Emu/RSX/GL/GLPresent.cpp b/rpcs3/Emu/RSX/GL/GLPresent.cpp index b99a230c2a..5fdedf5ee8 100644 --- a/rpcs3/Emu/RSX/GL/GLPresent.cpp +++ b/rpcs3/Emu/RSX/GL/GLPresent.cpp @@ -259,7 +259,7 @@ void GLGSRender::flip(const rsx::display_flip_info_t& info) } else { - m_frame->present_frame(sshot_frame, buffer_width, buffer_height, false); + m_frame->present_frame(sshot_frame, buffer_width * 4, buffer_width, buffer_height, false); } } diff --git a/rpcs3/Emu/RSX/GSFrameBase.h b/rpcs3/Emu/RSX/GSFrameBase.h index 680445dd6b..e81aa55253 100644 --- a/rpcs3/Emu/RSX/GSFrameBase.h +++ b/rpcs3/Emu/RSX/GSFrameBase.h @@ -30,6 +30,6 @@ public: virtual display_handle_t handle() const = 0; virtual bool can_consume_frame() const = 0; - virtual void present_frame(std::vector& data, const u32 width, const u32 height, bool is_bgra) const = 0; - virtual void take_screenshot(const std::vector sshot_data, const u32 sshot_width, const u32 sshot_height, bool is_bgra) = 0; + virtual void present_frame(std::vector& data, u32 pitch, u32 width, u32 height, bool is_bgra) const = 0; + virtual void take_screenshot(const std::vector sshot_data, u32 sshot_width, u32 sshot_height, bool is_bgra) = 0; }; diff --git a/rpcs3/Emu/RSX/VK/VKPresent.cpp b/rpcs3/Emu/RSX/VK/VKPresent.cpp index 9ff2dee4f7..2f69505b96 100644 --- a/rpcs3/Emu/RSX/VK/VKPresent.cpp +++ b/rpcs3/Emu/RSX/VK/VKPresent.cpp @@ -712,7 +712,7 @@ void VKGSRender::flip(const rsx::display_flip_info_t& info) } else { - m_frame->present_frame(sshot_frame, buffer_width, buffer_height, is_bgra); + m_frame->present_frame(sshot_frame, buffer_width * 4, buffer_width, buffer_height, is_bgra); } } } diff --git a/rpcs3/rpcs3qt/camera_settings_dialog.cpp b/rpcs3/rpcs3qt/camera_settings_dialog.cpp index 89f72cef8d..3e2ae3caab 100644 --- a/rpcs3/rpcs3qt/camera_settings_dialog.cpp +++ b/rpcs3/rpcs3qt/camera_settings_dialog.cpp @@ -66,6 +66,7 @@ camera_settings_dialog::camera_settings_dialog(QWidget* parent) { if (camera_info.isNull()) continue; ui->combo_camera->addItem(camera_info.description(), QVariant::fromValue(camera_info)); + camera_log.notice("Found camera: '%s'", camera_info.description()); } connect(ui->combo_camera, QOverload::of(&QComboBox::currentIndexChanged), this, &camera_settings_dialog::handle_camera_change); diff --git a/rpcs3/rpcs3qt/gs_frame.cpp b/rpcs3/rpcs3qt/gs_frame.cpp index 44b927738a..dfc3d6917b 100644 --- a/rpcs3/rpcs3qt/gs_frame.cpp +++ b/rpcs3/rpcs3qt/gs_frame.cpp @@ -762,13 +762,13 @@ bool gs_frame::can_consume_frame() const return video_provider.can_consume_frame(); } -void gs_frame::present_frame(std::vector& data, const u32 width, const u32 height, bool is_bgra) const +void gs_frame::present_frame(std::vector& data, u32 pitch, u32 width, u32 height, bool is_bgra) const { utils::video_provider& video_provider = g_fxo->get(); - video_provider.present_frame(data, width, height, is_bgra); + video_provider.present_frame(data, pitch, width, height, is_bgra); } -void gs_frame::take_screenshot(std::vector data, const u32 sshot_width, const u32 sshot_height, bool is_bgra) +void gs_frame::take_screenshot(std::vector data, u32 sshot_width, u32 sshot_height, bool is_bgra) { std::thread( [sshot_width, sshot_height, is_bgra](std::vector sshot_data) diff --git a/rpcs3/rpcs3qt/gs_frame.h b/rpcs3/rpcs3qt/gs_frame.h index b39c790e01..4872de1dad 100644 --- a/rpcs3/rpcs3qt/gs_frame.h +++ b/rpcs3/rpcs3qt/gs_frame.h @@ -69,8 +69,8 @@ public: bool get_mouse_lock_state(); bool can_consume_frame() const override; - void present_frame(std::vector& data, const u32 width, const u32 height, bool is_bgra) const override; - void take_screenshot(std::vector data, const u32 sshot_width, const u32 sshot_height, bool is_bgra) override; + void present_frame(std::vector& data, u32 pitch, u32 width, u32 height, bool is_bgra) const override; + void take_screenshot(std::vector data, u32 sshot_width, u32 sshot_height, bool is_bgra) override; protected: void paintEvent(QPaintEvent *event) override; diff --git a/rpcs3/util/image_sink.h b/rpcs3/util/image_sink.h index 831232fa30..3c23eca514 100644 --- a/rpcs3/util/image_sink.h +++ b/rpcs3/util/image_sink.h @@ -15,7 +15,7 @@ namespace utils image_sink() = default; virtual void stop(bool flush = true) = 0; - virtual void add_frame(std::vector& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) = 0; + virtual void add_frame(std::vector& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) = 0; s64 get_pts(usz timestamp_ms) const { @@ -32,12 +32,13 @@ namespace utils struct encoder_frame { encoder_frame() = default; - encoder_frame(usz timestamp_ms, u32 width, u32 height, s32 av_pixel_format, std::vector&& data) - : timestamp_ms(timestamp_ms), width(width), height(height), av_pixel_format(av_pixel_format), data(std::move(data)) + encoder_frame(usz timestamp_ms, u32 pitch, u32 width, u32 height, s32 av_pixel_format, std::vector&& data) + : timestamp_ms(timestamp_ms), pitch(pitch), width(width), height(height), av_pixel_format(av_pixel_format), data(std::move(data)) {} s64 pts = -1; // Optional usz timestamp_ms = 0; + u32 pitch = 0; u32 width = 0; u32 height = 0; s32 av_pixel_format = 0; // NOTE: Make sure this is a valid AVPixelFormat diff --git a/rpcs3/util/media_utils.cpp b/rpcs3/util/media_utils.cpp index 83eacc0b51..3dddd5b0f8 100644 --- a/rpcs3/util/media_utils.cpp +++ b/rpcs3/util/media_utils.cpp @@ -604,14 +604,14 @@ namespace utils m_audio_codec_id = codec_id; } - void video_encoder::add_frame(std::vector& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) + void video_encoder::add_frame(std::vector& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) { // Do not allow new frames while flushing if (m_flush) return; std::lock_guard lock(m_mtx); - m_frames_to_encode.emplace_back(timestamp_ms, width, height, pixel_format, std::move(frame)); + m_frames_to_encode.emplace_back(timestamp_ms, pitch, width, height, pixel_format, std::move(frame)); } void video_encoder::pause(bool flush) diff --git a/rpcs3/util/media_utils.h b/rpcs3/util/media_utils.h index 5c25d14be9..2718a80617 100644 --- a/rpcs3/util/media_utils.h +++ b/rpcs3/util/media_utils.h @@ -120,7 +120,7 @@ namespace utils void set_sample_rate(u32 sample_rate); void set_audio_bitrate(u32 bitrate); void set_audio_codec(s32 codec_id); - void add_frame(std::vector& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) override; + void add_frame(std::vector& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) override; void pause(bool flush = true); void stop(bool flush = true) override; void encode(); diff --git a/rpcs3/util/video_provider.cpp b/rpcs3/util/video_provider.cpp index cf3e910f93..d919137733 100644 --- a/rpcs3/util/video_provider.cpp +++ b/rpcs3/util/video_provider.cpp @@ -92,7 +92,7 @@ namespace utils return pts > m_last_pts_incoming; } - void video_provider::present_frame(std::vector& data, const u32 width, const u32 height, bool is_bgra) + void video_provider::present_frame(std::vector& data, u32 pitch, u32 width, u32 height, bool is_bgra) { std::lock_guard lock(m_mutex); @@ -132,6 +132,6 @@ namespace utils m_last_pts_incoming = pts; m_current_encoder_frame++; - m_image_sink->add_frame(data, width, height, is_bgra ? AVPixelFormat::AV_PIX_FMT_BGRA : AVPixelFormat::AV_PIX_FMT_RGBA, timestamp_ms); + m_image_sink->add_frame(data, pitch, width, height, is_bgra ? AVPixelFormat::AV_PIX_FMT_BGRA : AVPixelFormat::AV_PIX_FMT_RGBA, timestamp_ms); } } diff --git a/rpcs3/util/video_provider.h b/rpcs3/util/video_provider.h index 8fd4e12483..31a051a112 100644 --- a/rpcs3/util/video_provider.h +++ b/rpcs3/util/video_provider.h @@ -20,7 +20,7 @@ namespace utils bool set_image_sink(std::shared_ptr sink, recording_mode type); void set_pause_time(usz pause_time_ms); bool can_consume_frame(); - void present_frame(std::vector& data, const u32 width, const u32 height, bool is_bgra); + void present_frame(std::vector& data, u32 pitch, u32 width, u32 height, bool is_bgra); private: recording_mode m_type = recording_mode::stopped;