From bb7d1bb5a3ffcca0b7428c909ddbe4d828b3c2e8 Mon Sep 17 00:00:00 2001 From: JohannesItten Date: Fri, 3 Jul 2026 19:11:18 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20VideoReader=20bugs=20=E2=80=94=20stride,?= =?UTF-8?q?=20memory,=20audio=20detection,=20UB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass MXL grain stride (from sliceSizes[0]) into get_next_frame so YUV422P10toV210 writes with the correct line width instead of a self-computed value that may not match the MXL buffer. Fix audio stream detection: condition was inverted (!= -1 → == -1), so the first audio stream was never picked up. Add return false at end of get_next_frame to fix UB when g_running goes false and the loop exits without returning. Replace av_frame_unref/av_packet_unref with av_frame_free/av_packet_free in destructor — unref only releases data, not the struct itself. Add av_freep(&p10_data[0]) in destructor to free av_image_alloc memory. Remove unused p10_buffer and v210_buffer allocations. Read filename from config("file") instead of hardcoded path. Add early return if mxlCreateFlowWriter fails. Co-Authored-By: Claude Sonnet 4.6 --- nodes/videoin/main.cpp | 14 +++++++------- shared/VideoReader.hpp | 34 ++++++++-------------------------- 2 files changed, 15 insertions(+), 33 deletions(-) diff --git a/nodes/videoin/main.cpp b/nodes/videoin/main.cpp index 733f6a9..a54f9b5 100644 --- a/nodes/videoin/main.cpp +++ b/nodes/videoin/main.cpp @@ -15,12 +15,10 @@ extern "C" { #include "NodeBase.hpp" #include "VideoReader.hpp" -#include - - class VideoInNode : public dmf::NodeBase { - void run() override { - std::string filename = "/home/itten/test-vid/0.ts"; + void run() override { + const std::string filename = config().value("file", std::string{}); + if (filename.empty()) { log("config missing 'file'"); return; } log("VideoIn Node started with file: %s", filename.c_str()); dmf::VideoReader video_reader(filename); @@ -51,8 +49,10 @@ class VideoInNode : public dmf::NodeBase { &created ); if (vst != MXL_STATUS_OK) { - log("MXL flow writer is not created. Reason: %s", dmf::mxl_status_str(vst)); + log("mxlCreateFlowWriter failed (%s)", dmf::mxl_status_str(vst)); + return; } + const uint32_t video_stride = video_config.discrete.sliceSizes[0]; mxlRational video_rate = {fps_num, fps_den}; uint64_t video_index = mxlGetCurrentIndex(&video_rate); @@ -62,7 +62,7 @@ class VideoInNode : public dmf::NodeBase { mxlGrainInfo grain{}; vst = mxlFlowWriterOpenGrain(video_writer, video_index, &grain, &buf); if (vst == MXL_STATUS_OK) { - if (!video_reader.get_next_frame(buf, nullptr)) continue; + if (!video_reader.get_next_frame(buf, video_stride, nullptr)) continue; grain.flags = 0; grain.validSlices = grain.totalSlices; mxlFlowWriterCommitGrain(video_writer, &grain); diff --git a/shared/VideoReader.hpp b/shared/VideoReader.hpp index c6338a1..445b1b0 100644 --- a/shared/VideoReader.hpp +++ b/shared/VideoReader.hpp @@ -49,11 +49,12 @@ class VideoReader { avcodec_free_context(&codec_context); avformat_close_input(&format_context); sws_freeContext(sws_ctx); - av_frame_unref(frame); - av_packet_unref(packet); + av_freep(&p10_data[0]); + av_frame_free(&frame); + av_packet_free(&packet); } - bool get_next_frame(uint8_t* video_buf, uint8_t* audiobuf) { + bool get_next_frame(uint8_t* video_buf, uint32_t mxl_stride, uint8_t* audiobuf) { while (dmf::g_running.load(std::memory_order_relaxed)) { // Try to get a buffered frame from previous packet first if (avcodec_receive_frame(codec_context, frame) == 0) { @@ -76,7 +77,7 @@ class VideoReader { p10_linesizes[0], p10_linesizes[1], p10_linesizes[2], - v210_stride + mxl_stride ); av_frame_unref(frame); return true; @@ -93,6 +94,7 @@ class VideoReader { if (packet->stream_index != video_stream_index) continue; avcodec_send_packet(codec_context, packet); } + return false; } private: @@ -105,11 +107,8 @@ class VideoReader { // conversion data struct SwsContext *sws_ctx{}; - uint8_t* p10_buffer = nullptr; int p10_linesizes[4] = {0, 0, 0, 0}; uint8_t* p10_data[4] = {nullptr, nullptr, nullptr, nullptr}; - uint8_t* v210_buffer = nullptr; - int v210_stride = 0; bool open_file(std::string filename) { if (avformat_open_input(&format_context, filename.c_str(), nullptr, nullptr) != 0) { @@ -130,7 +129,7 @@ class VideoReader { if (data_type == AVMEDIA_TYPE_VIDEO) { video_stream_index = i; have_video = true; - } else if (data_type == AVMEDIA_TYPE_AUDIO && audio_stream_index != -1) { + } else if (data_type == AVMEDIA_TYPE_AUDIO && audio_stream_index == -1) { // TODO: show list of available audio tracks and allow user to pick // or handle multiple audio streams audio_stream_index = i; @@ -193,30 +192,13 @@ class VideoReader { return; } - // Allocate P10 image - int p10_buf_size = av_image_get_buffer_size( - AV_PIX_FMT_YUV422P10LE, - source_info.width, source_info.height, - 64 - ); - p10_buffer = (uint8_t*)av_malloc(p10_buf_size); + // Allocate P10 image (freed in destructor via av_freep(&p10_data[0])) av_image_alloc( p10_data, p10_linesizes, source_info.width, source_info.height, AV_PIX_FMT_YUV422P10LE, 64 ); - // Allocate v210 buffer - int blocks_per_row = (codec_context->width + 5) / 6; - int v210_bytes_per_row = blocks_per_row * 16; - v210_stride = ((v210_bytes_per_row + 63) / 64) * 64; - int v210_buffer_size = v210_stride * source_info.height; - - v210_buffer = (uint8_t*)av_malloc(v210_buffer_size); - if (!v210_buffer) { - throw std::runtime_error("Failed to allocate V210 buffer"); - return; - } } };