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.
This commit is contained in:
Megamouse 2023-08-12 23:06:22 +02:00
parent f40a6d496a
commit 39bbf17caf
12 changed files with 24 additions and 22 deletions

View file

@ -158,14 +158,14 @@ public:
has_error = false; has_error = false;
} }
void add_frame(std::vector<u8>& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) override void add_frame(std::vector<u8>& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) override
{ {
std::lock_guard lock(m_mtx); std::lock_guard lock(m_mtx);
if (m_flush) if (m_flush)
return; 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() encoder_frame get_frame()
@ -587,7 +587,7 @@ void rec_info::start_image_provider()
{ {
std::vector<u8> frame(frame_size); std::vector<u8> frame(frame_size);
std::memcpy(frame.data(), video_input_buffer.get_ptr(), 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(); const usz pos = (start_offset + i) % video_ringbuffer.size();
utils::image_sink::encoder_frame& frame_data = video_ringbuffer[pos]; 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 // TODO: add audio data to encoder
} }

View file

@ -259,7 +259,7 @@ void GLGSRender::flip(const rsx::display_flip_info_t& info)
} }
else 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);
} }
} }

View file

@ -30,6 +30,6 @@ public:
virtual display_handle_t handle() const = 0; virtual display_handle_t handle() const = 0;
virtual bool can_consume_frame() const = 0; virtual bool can_consume_frame() const = 0;
virtual void present_frame(std::vector<u8>& data, const u32 width, const u32 height, bool is_bgra) const = 0; virtual void present_frame(std::vector<u8>& data, u32 pitch, u32 width, u32 height, bool is_bgra) const = 0;
virtual void take_screenshot(const std::vector<u8> sshot_data, const u32 sshot_width, const u32 sshot_height, bool is_bgra) = 0; virtual void take_screenshot(const std::vector<u8> sshot_data, u32 sshot_width, u32 sshot_height, bool is_bgra) = 0;
}; };

View file

@ -712,7 +712,7 @@ void VKGSRender::flip(const rsx::display_flip_info_t& info)
} }
else 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);
} }
} }
} }

View file

@ -66,6 +66,7 @@ camera_settings_dialog::camera_settings_dialog(QWidget* parent)
{ {
if (camera_info.isNull()) continue; if (camera_info.isNull()) continue;
ui->combo_camera->addItem(camera_info.description(), QVariant::fromValue(camera_info)); 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<int>::of(&QComboBox::currentIndexChanged), this, &camera_settings_dialog::handle_camera_change); connect(ui->combo_camera, QOverload<int>::of(&QComboBox::currentIndexChanged), this, &camera_settings_dialog::handle_camera_change);

View file

@ -762,13 +762,13 @@ bool gs_frame::can_consume_frame() const
return video_provider.can_consume_frame(); return video_provider.can_consume_frame();
} }
void gs_frame::present_frame(std::vector<u8>& data, const u32 width, const u32 height, bool is_bgra) const void gs_frame::present_frame(std::vector<u8>& data, u32 pitch, u32 width, u32 height, bool is_bgra) const
{ {
utils::video_provider& video_provider = g_fxo->get<utils::video_provider>(); utils::video_provider& video_provider = g_fxo->get<utils::video_provider>();
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<u8> data, const u32 sshot_width, const u32 sshot_height, bool is_bgra) void gs_frame::take_screenshot(std::vector<u8> data, u32 sshot_width, u32 sshot_height, bool is_bgra)
{ {
std::thread( std::thread(
[sshot_width, sshot_height, is_bgra](std::vector<u8> sshot_data) [sshot_width, sshot_height, is_bgra](std::vector<u8> sshot_data)

View file

@ -69,8 +69,8 @@ public:
bool get_mouse_lock_state(); bool get_mouse_lock_state();
bool can_consume_frame() const override; bool can_consume_frame() const override;
void present_frame(std::vector<u8>& data, const u32 width, const u32 height, bool is_bgra) const override; void present_frame(std::vector<u8>& data, u32 pitch, u32 width, u32 height, bool is_bgra) const override;
void take_screenshot(std::vector<u8> data, const u32 sshot_width, const u32 sshot_height, bool is_bgra) override; void take_screenshot(std::vector<u8> data, u32 sshot_width, u32 sshot_height, bool is_bgra) override;
protected: protected:
void paintEvent(QPaintEvent *event) override; void paintEvent(QPaintEvent *event) override;

View file

@ -15,7 +15,7 @@ namespace utils
image_sink() = default; image_sink() = default;
virtual void stop(bool flush = true) = 0; virtual void stop(bool flush = true) = 0;
virtual void add_frame(std::vector<u8>& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) = 0; virtual void add_frame(std::vector<u8>& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) = 0;
s64 get_pts(usz timestamp_ms) const s64 get_pts(usz timestamp_ms) const
{ {
@ -32,12 +32,13 @@ namespace utils
struct encoder_frame struct encoder_frame
{ {
encoder_frame() = default; encoder_frame() = default;
encoder_frame(usz timestamp_ms, u32 width, u32 height, s32 av_pixel_format, std::vector<u8>&& data) encoder_frame(usz timestamp_ms, u32 pitch, u32 width, u32 height, s32 av_pixel_format, std::vector<u8>&& data)
: timestamp_ms(timestamp_ms), width(width), height(height), av_pixel_format(av_pixel_format), data(std::move(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 s64 pts = -1; // Optional
usz timestamp_ms = 0; usz timestamp_ms = 0;
u32 pitch = 0;
u32 width = 0; u32 width = 0;
u32 height = 0; u32 height = 0;
s32 av_pixel_format = 0; // NOTE: Make sure this is a valid AVPixelFormat s32 av_pixel_format = 0; // NOTE: Make sure this is a valid AVPixelFormat

View file

@ -604,14 +604,14 @@ namespace utils
m_audio_codec_id = codec_id; m_audio_codec_id = codec_id;
} }
void video_encoder::add_frame(std::vector<u8>& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) void video_encoder::add_frame(std::vector<u8>& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms)
{ {
// Do not allow new frames while flushing // Do not allow new frames while flushing
if (m_flush) if (m_flush)
return; return;
std::lock_guard lock(m_mtx); 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) void video_encoder::pause(bool flush)

View file

@ -120,7 +120,7 @@ namespace utils
void set_sample_rate(u32 sample_rate); void set_sample_rate(u32 sample_rate);
void set_audio_bitrate(u32 bitrate); void set_audio_bitrate(u32 bitrate);
void set_audio_codec(s32 codec_id); void set_audio_codec(s32 codec_id);
void add_frame(std::vector<u8>& frame, const u32 width, const u32 height, s32 pixel_format, usz timestamp_ms) override; void add_frame(std::vector<u8>& frame, u32 pitch, u32 width, u32 height, s32 pixel_format, usz timestamp_ms) override;
void pause(bool flush = true); void pause(bool flush = true);
void stop(bool flush = true) override; void stop(bool flush = true) override;
void encode(); void encode();

View file

@ -92,7 +92,7 @@ namespace utils
return pts > m_last_pts_incoming; return pts > m_last_pts_incoming;
} }
void video_provider::present_frame(std::vector<u8>& data, const u32 width, const u32 height, bool is_bgra) void video_provider::present_frame(std::vector<u8>& data, u32 pitch, u32 width, u32 height, bool is_bgra)
{ {
std::lock_guard lock(m_mutex); std::lock_guard lock(m_mutex);
@ -132,6 +132,6 @@ namespace utils
m_last_pts_incoming = pts; m_last_pts_incoming = pts;
m_current_encoder_frame++; 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);
} }
} }

View file

@ -20,7 +20,7 @@ namespace utils
bool set_image_sink(std::shared_ptr<image_sink> sink, recording_mode type); bool set_image_sink(std::shared_ptr<image_sink> sink, recording_mode type);
void set_pause_time(usz pause_time_ms); void set_pause_time(usz pause_time_ms);
bool can_consume_frame(); bool can_consume_frame();
void present_frame(std::vector<u8>& data, const u32 width, const u32 height, bool is_bgra); void present_frame(std::vector<u8>& data, u32 pitch, u32 width, u32 height, bool is_bgra);
private: private:
recording_mode m_type = recording_mode::stopped; recording_mode m_type = recording_mode::stopped;