From 96475acf62f23d08480b16f560fe074cc65e0f40 Mon Sep 17 00:00:00 2001 From: JohannesItten Date: Fri, 3 Jul 2026 19:13:19 +0300 Subject: [PATCH] refactor: VideoReader and videoin cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VideoReader: - Guard get_source_info/allocate_conversion_buffers behind have_video check — prevents crash on audio-only files - Remove unused audiobuf parameter from get_next_frame - Remove dead return statements after throw - Remove unused SourceInfo::stride field and AudioInfo struct - Pass const std::string& instead of by value in constructor/open_file - Remove redundant struct keyword on SourceInfo source_info{} - Fix video_stream_index never guarded against -1 in open_file - Check av_image_alloc and avcodec_parameters_to_context return values - Remove extra av_packet_unref after seek (was harmless but confusing) - Use SWS_BILINEAR for sws_getContext flags instead of 0 - Replace NULL with nullptr in sws_getContext - Remove unused #include videoin main.cpp: - Early return if have_video is false after open - Inline make_video_flow_def call (remove intermediate variable) - Add grain count log line matching other nodes - Change continue to break when get_next_frame returns false - Make video_rate const - Remove double blank line before main() - Remove trailing spaces Co-Authored-By: Claude Sonnet 4.6 --- nodes/videoin/main.cpp | 51 +++---- shared/VideoReader.hpp | 306 +++++++++++++++++++---------------------- 2 files changed, 161 insertions(+), 196 deletions(-) diff --git a/nodes/videoin/main.cpp b/nodes/videoin/main.cpp index a54f9b5..d39893c 100644 --- a/nodes/videoin/main.cpp +++ b/nodes/videoin/main.cpp @@ -4,7 +4,6 @@ extern "C" { #include #include #include - #include } #include @@ -19,51 +18,45 @@ class VideoInNode : public dmf::NodeBase { 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()); + log("file: %s", filename.c_str()); + dmf::VideoReader video_reader(filename); + if (!video_reader.have_video) { log("no video stream found"); return; } const auto video_flow_info = config().at("video_flow_id"); const auto video_flow_id = video_flow_info.at("id").get(); - const int width = video_flow_info.value("width", video_reader.source_info.width); - const int height = video_flow_info.value("height", video_reader.source_info.height); - const int fps_num = video_flow_info.value("fps_num", video_reader.source_info.fps_num); - const int fps_den = video_flow_info.value("fps_den", video_reader.source_info.fps_den); + const int width = video_flow_info.value("width", video_reader.source_info.width); + const int height = video_flow_info.value("height", video_reader.source_info.height); + const int fps_num = video_flow_info.value("fps_num", video_reader.source_info.fps_num); + const int fps_den = video_flow_info.value("fps_den", video_reader.source_info.fps_den); - mxlFlowWriter video_writer = nullptr; - mxlFlowConfigInfo video_config = {}; - std::string video_flow_def = dmf::make_video_flow_def( - video_flow_id, - node_id(), - width, - height, - fps_num, - fps_den - ); + log("video flow=%s %dx%d @ %d/%d fps", video_flow_id.c_str(), width, height, fps_num, fps_den); + + mxlFlowWriter video_writer = nullptr; + mxlFlowConfigInfo video_cfg = {}; bool created = false; mxlStatus vst = mxlCreateFlowWriter( instance(), - video_flow_def.c_str(), - "", - &video_writer, - &video_config, - &created - ); + dmf::make_video_flow_def(video_flow_id, node_id(), width, height, fps_num, fps_den).c_str(), + "", &video_writer, &video_cfg, &created); if (vst != MXL_STATUS_OK) { log("mxlCreateFlowWriter failed (%s)", dmf::mxl_status_str(vst)); return; } - const uint32_t video_stride = video_config.discrete.sliceSizes[0]; + const uint32_t video_stride = video_cfg.discrete.sliceSizes[0]; + log("video stride=%u B/line grain=%u B ring=%u grains", + video_stride, video_stride * static_cast(height), video_cfg.discrete.grainCount); - mxlRational video_rate = {fps_num, fps_den}; + const mxlRational video_rate = {fps_num, fps_den}; uint64_t video_index = mxlGetCurrentIndex(&video_rate); while (dmf::g_running.load(std::memory_order_relaxed)) { - uint8_t* buf = nullptr; + uint8_t* buf = nullptr; mxlGrainInfo grain{}; vst = mxlFlowWriterOpenGrain(video_writer, video_index, &grain, &buf); if (vst == MXL_STATUS_OK) { - if (!video_reader.get_next_frame(buf, video_stride, nullptr)) continue; - grain.flags = 0; + if (!video_reader.get_next_frame(buf, video_stride)) break; + grain.flags = 0; grain.validSlices = grain.totalSlices; mxlFlowWriterCommitGrain(video_writer, &grain); } @@ -72,12 +65,12 @@ class VideoInNode : public dmf::NodeBase { video_index++; } + log("stopped at video_index=%llu", video_index); mxlReleaseFlowWriter(instance(), video_writer); } }; - -int main() { +int main() { VideoInNode node; return node.execute(); } diff --git a/shared/VideoReader.hpp b/shared/VideoReader.hpp index 445b1b0..65f7925 100644 --- a/shared/VideoReader.hpp +++ b/shared/VideoReader.hpp @@ -4,9 +4,8 @@ extern "C" { #include #include #include - #include #include - #include + #include } #include @@ -15,191 +14,164 @@ extern "C" { #include "V210.hpp" namespace dmf { + class VideoReader { - public: - struct SourceInfo { - int width = 0; - int height = 0; - int fps_num = 0; - int fps_den = 0; - int stride = 0; - AVPixelFormat pix_fmt{}; - }; +public: + struct SourceInfo { + int width = 0; + int height = 0; + int fps_num = 0; + int fps_den = 0; + AVPixelFormat pix_fmt{}; + }; - struct AudioInfo { - int sample_rate = 0; - int channels = 0; - int samples = 0; - int channel_stride = 0; // floats between channel planes - }; + SourceInfo source_info{}; + bool has_audio = false; + bool have_video = false; - struct SourceInfo source_info{}; - bool has_audio = false; - bool have_video = false; - - VideoReader(std::string filename) { - if (!open_file(filename)) { - return; - } + explicit VideoReader(const std::string& filename) { + if (!open_file(filename)) + return; + if (have_video) { get_source_info(); allocate_conversion_buffers(); } + } - ~VideoReader() { - avcodec_free_context(&codec_context); + ~VideoReader() { + avcodec_free_context(&codec_context); + avformat_close_input(&format_context); + sws_freeContext(sws_ctx); + av_freep(&p10_data[0]); + av_frame_free(&frame); + av_packet_free(&packet); + } + + // Returns true when a frame was decoded and written into video_buf. + // Returns false when g_running goes false. + bool get_next_frame(uint8_t* video_buf, uint32_t mxl_stride) { + while (dmf::g_running.load(std::memory_order_relaxed)) { + // Drain any frames buffered in the decoder first + if (avcodec_receive_frame(codec_context, frame) == 0) { + sws_scale( + sws_ctx, + frame->data, + frame->linesize, + 0, + source_info.height, + p10_data, + p10_linesizes + ); + dmf::v210::YUV422P10toV210( + reinterpret_cast(p10_data[0]), + reinterpret_cast(p10_data[1]), + reinterpret_cast(p10_data[2]), + video_buf, + source_info.width, + source_info.height, + p10_linesizes[0], + p10_linesizes[1], + p10_linesizes[2], + mxl_stride + ); + av_frame_unref(frame); + return true; + } + + // No buffered frame — read next packet + av_packet_unref(packet); + if (av_read_frame(format_context, packet) < 0) { + // EOF — loop back to start + avformat_seek_file(format_context, -1, 0, 0, 0, AVSEEK_FLAG_BACKWARD); + avcodec_flush_buffers(codec_context); + continue; + } + if (packet->stream_index != video_stream_index) continue; + avcodec_send_packet(codec_context, packet); + } + return false; + } + +private: + AVFormatContext* format_context = nullptr; + AVCodecContext* codec_context = nullptr; + AVPacket* packet = av_packet_alloc(); + AVFrame* frame = av_frame_alloc(); + int video_stream_index = -1; + int audio_stream_index = -1; + + SwsContext* sws_ctx = nullptr; + int p10_linesizes[4] = {0, 0, 0, 0}; + uint8_t* p10_data[4] = {nullptr, nullptr, nullptr, nullptr}; + + bool open_file(const std::string& filename) { + if (avformat_open_input(&format_context, filename.c_str(), nullptr, nullptr) != 0) + throw std::runtime_error("Could not open file: " + filename); + + if (avformat_find_stream_info(format_context, nullptr) < 0) { avformat_close_input(&format_context); - sws_freeContext(sws_ctx); - av_freep(&p10_data[0]); - av_frame_free(&frame); - av_packet_free(&packet); + throw std::runtime_error("Could not find stream info"); } - 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) { - sws_scale( - sws_ctx, - frame->data, - frame->linesize, - 0, - source_info.height, - p10_data, - p10_linesizes - ); - dmf::v210::YUV422P10toV210( - reinterpret_cast(p10_data[0]), - reinterpret_cast(p10_data[1]), - reinterpret_cast(p10_data[2]), - video_buf, - source_info.width, - source_info.height, - p10_linesizes[0], - p10_linesizes[1], - p10_linesizes[2], - mxl_stride - ); - av_frame_unref(frame); - return true; - } - // No buffered frame — read next packet - av_packet_unref(packet); - if (av_read_frame(format_context, packet) < 0) { - // EOF — seek back to start and keep going - avformat_seek_file(format_context, -1, 0, 0, 0, AVSEEK_FLAG_BACKWARD); - avcodec_flush_buffers(codec_context); - av_packet_unref(packet); - continue; - } - if (packet->stream_index != video_stream_index) continue; - avcodec_send_packet(codec_context, packet); + for (unsigned int i = 0; i < format_context->nb_streams; ++i) { + const AVMediaType type = format_context->streams[i]->codecpar->codec_type; + if (type == AVMEDIA_TYPE_VIDEO && video_stream_index == -1) { + video_stream_index = static_cast(i); + have_video = true; + } else if (type == AVMEDIA_TYPE_AUDIO && audio_stream_index == -1) { + audio_stream_index = static_cast(i); + has_audio = true; } - return false; } - private: - AVFormatContext* format_context = nullptr; - AVCodecContext* codec_context = nullptr; - AVPacket* packet = av_packet_alloc(); - AVFrame* frame = av_frame_alloc(); - int video_stream_index = -1; - int audio_stream_index = -1; - - // conversion data - struct SwsContext *sws_ctx{}; - int p10_linesizes[4] = {0, 0, 0, 0}; - uint8_t* p10_data[4] = {nullptr, nullptr, nullptr, nullptr}; - - bool open_file(std::string filename) { - if (avformat_open_input(&format_context, filename.c_str(), nullptr, nullptr) != 0) { - throw std::runtime_error("Could not open file: " + filename); - return false; - } - - // Find stream info - if (avformat_find_stream_info(format_context, nullptr) < 0) { - avformat_close_input(&format_context); - throw std::runtime_error("Could not find stream info"); - return false; - } - - // Find streams - for (unsigned int i = 0; i < format_context->nb_streams; i++) { - AVMediaType data_type = format_context->streams[i]->codecpar->codec_type; - if (data_type == AVMEDIA_TYPE_VIDEO) { - video_stream_index = i; - have_video = true; - } 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; - has_audio = true; - } - } - - if (video_stream_index == -1 && audio_stream_index == -1) { - avformat_close_input(&format_context); - throw std::runtime_error("No audio/video stream found"); - return false; - } - - return true; + if (video_stream_index == -1 && audio_stream_index == -1) { + avformat_close_input(&format_context); + throw std::runtime_error("No audio/video stream found in: " + filename); } - void get_source_info() { - // Get codec parameters - AVCodecParameters* codec_params = format_context->streams[video_stream_index]->codecpar; - const AVCodec* codec = avcodec_find_decoder(codec_params->codec_id); - - if (!codec) { - avformat_close_input(&format_context); - throw std::runtime_error("Unsupported codec"); - } - - // Open codec - codec_context = avcodec_alloc_context3(codec); - avcodec_parameters_to_context(codec_context, codec_params); - - if (avcodec_open2(codec_context, codec, nullptr) < 0) { - avcodec_free_context(&codec_context); - avformat_close_input(&format_context); - throw std::runtime_error("Could not open codec"); - } + return true; + } - AVRational fps = codec_context->framerate; - if (fps.num == 0 || fps.den == 0) { - fps = format_context->streams[video_stream_index]->avg_frame_rate; - } + void get_source_info() { + AVCodecParameters* codec_params = format_context->streams[video_stream_index]->codecpar; + const AVCodec* codec = avcodec_find_decoder(codec_params->codec_id); + if (!codec) + throw std::runtime_error("Unsupported codec"); - source_info.width = codec_context->width; - source_info.height = codec_context->height; - source_info.fps_num = fps.num; - source_info.fps_den = fps.den; - source_info.pix_fmt = codec_context->pix_fmt; + codec_context = avcodec_alloc_context3(codec); + if (avcodec_parameters_to_context(codec_context, codec_params) < 0) + throw std::runtime_error("Could not copy codec parameters"); + + if (avcodec_open2(codec_context, codec, nullptr) < 0) { + avcodec_free_context(&codec_context); + throw std::runtime_error("Could not open codec"); } - void allocate_conversion_buffers() - { - // Create Sws context to convert to planar YUV 4:2:2, 20bpp, (1 Cr & Cb sample per 2x1 Y samples), LE - sws_ctx = sws_getContext( - source_info.width, source_info.height, source_info.pix_fmt, - source_info.width, source_info.height, AV_PIX_FMT_YUV422P10LE, - 0, NULL, NULL, NULL - ); + AVRational fps = codec_context->framerate; + if (fps.num == 0 || fps.den == 0) + fps = format_context->streams[video_stream_index]->avg_frame_rate; - if (!sws_ctx) { - throw std::runtime_error("Failed to create SwsContext"); - return; - } + source_info.width = codec_context->width; + source_info.height = codec_context->height; + source_info.fps_num = fps.num; + source_info.fps_den = fps.den; + source_info.pix_fmt = codec_context->pix_fmt; + } - // Allocate P10 image (freed in destructor via av_freep(&p10_data[0])) - av_image_alloc( - p10_data, p10_linesizes, + void allocate_conversion_buffers() { + sws_ctx = sws_getContext( + source_info.width, source_info.height, source_info.pix_fmt, + source_info.width, source_info.height, AV_PIX_FMT_YUV422P10LE, + SWS_BILINEAR, nullptr, nullptr, nullptr + ); + if (!sws_ctx) + throw std::runtime_error("Failed to create SwsContext"); + + if (av_image_alloc(p10_data, p10_linesizes, source_info.width, source_info.height, - AV_PIX_FMT_YUV422P10LE, 64 - ); - - } - + AV_PIX_FMT_YUV422P10LE, 64) < 0) + throw std::runtime_error("Failed to allocate YUV422P10 buffer"); + } }; -} \ No newline at end of file + +} // namespace dmf