feat: add video decoder support - #32
Conversation
Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
…ecompression Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
…er decompression related functions Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
There was a problem hiding this comment.
Pull request overview
This PR introduces video decoder support for the accelerated image processor, complementing the video encoder support added in PR #29. The implementation uses FFmpeg libraries to decode H.264, H.265, and AV1 video streams with CUDA hardware acceleration.
Changes:
- New
accelerated_image_processor_decompressionpackage with FFmpeg-based video decompression - New
decompress_nodeROS node to subscribe to compressed video topics and publish decompressed images - QoS and conversion utilities to support topic type detection and FFmpeg message handling
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
src/accelerated_image_processor_decompression/src/video_decompressor/ffmpeg.cpp |
Core FFmpeg video decoder implementation with CUDA acceleration |
src/accelerated_image_processor_decompression/include/accelerated_image_processor_decompression/video_decompressor.hpp |
Video decompressor base class interface |
src/accelerated_image_processor_decompression/include/accelerated_image_processor_decompression/builder.hpp |
Builder pattern for creating decompressor instances |
src/accelerated_image_processor_decompression/src/builder.cpp |
Builder implementation |
src/accelerated_image_processor_decompression/test/ffmpeg_video_decompressor.cpp |
Unit tests for FFmpeg video decompressor |
src/accelerated_image_processor_decompression/test/test_utility.hpp |
Test utility for generating encoded video frames |
src/accelerated_image_processor_decompression/test/builder.cpp |
Unit tests for builder pattern |
src/accelerated_image_processor_decompression/CMakeLists.txt |
Build configuration for decompression package |
src/accelerated_image_processor_decompression/package.xml |
Package dependencies |
src/accelerated_image_processor_ros/src/decompress_node.cpp |
ROS node implementation for video decompression |
src/accelerated_image_processor_ros/src/decompress_node.hpp |
ROS node header |
src/accelerated_image_processor_ros/src/conversion.cpp |
Conversion functions for FFmpeg messages |
src/accelerated_image_processor_ros/include/accelerated_image_processor_ros/conversion.hpp |
Conversion function declarations |
src/accelerated_image_processor_ros/src/qos.cpp |
QoS helper functions including topic type detection |
src/accelerated_image_processor_ros/include/accelerated_image_processor_ros/qos.hpp |
QoS helper function declarations |
src/accelerated_image_processor_ros/test/conversion.cpp |
Tests for conversion functions |
src/accelerated_image_processor_ros/launch/decompress.launch.xml |
Launch file for decompress node |
src/accelerated_image_processor_ros/config/decompress.param.yaml |
Configuration parameters |
src/accelerated_image_processor_ros/CMakeLists.txt |
Updated build configuration |
src/accelerated_image_processor_ros/package.xml |
Added decompression dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void cleanup_decoder() | ||
| { | ||
| if (packet_) { | ||
| av_packet_free(&packet_); | ||
| } | ||
| if (codec_ctx_) { | ||
| avcodec_free_context(&codec_ctx_); | ||
| } | ||
| if (hw_device_ctx_) { | ||
| av_buffer_unref(&hw_device_ctx_); | ||
| } | ||
| } |
There was a problem hiding this comment.
Memory leak: The decoded_frame_ allocated by av_frame_alloc() is never freed. The cleanup_decoder() function should call av_frame_free(&decoded_frame_) to properly release this resource. While av_frame_unref() releases the reference to the frame data, it doesn't free the AVFrame structure itself.
| explicit FfmpegTestDataProvider() | ||
| { | ||
| // Embed frame count into the frame contents | ||
| // NOTE: Large counter shown on the right middle depicts second of the video | ||
| // NOTE: To embed frame count into the pixel value (so that we can confirm the count is as | ||
| // expected programatically), | ||
| // `std::string filter_descr = "geq=lum='mod(N*10, 255)',format=pix_fmts=yuv420p";` | ||
| // is another option. This increases pixel brightness by 10 (back to 0 if the frame number | ||
| // reaches 255) frame by frame | ||
|
|
||
| std::string filter_descr = | ||
| "drawtext=text='%{n}':fontsize=150:fontcolor=white:x=100:y=100,format=pix_fmts=yuv420p"; | ||
|
|
||
| AVFilterGraph * graph = avfilter_graph_alloc(); | ||
| AVFilterContext * src_ctx = nullptr; | ||
| AVFilterContext * sink_ctx = nullptr; | ||
|
|
||
| const AVFilter * src = avfilter_get_by_name("testsrc"); | ||
| const AVFilter * sink = avfilter_get_by_name("buffersink"); | ||
| AVFilterInOut * inputs = avfilter_inout_alloc(); | ||
| AVFilterInOut * outputs = avfilter_inout_alloc(); | ||
|
|
||
| av_opt_set_int(src_ctx, "sample_aspect_ratio", 0, 0); | ||
| av_opt_set_int(sink_ctx, "sample_aspect_ratio", 0, 0); | ||
|
|
||
| // avfilter_graph_create_filter(&src_ctx, src, "src", filter_descr.c_str(), nullptr, graph); | ||
| std::string src_filter_descr = "size=" + std::to_string(WIDTH) + "x" + std::to_string(HEIGHT) + | ||
| ":rate=" + std::to_string(FPS); | ||
| avfilter_graph_create_filter(&src_ctx, src, "src", src_filter_descr.c_str(), nullptr, graph); | ||
| avfilter_graph_create_filter(&sink_ctx, sink, "sink", nullptr, nullptr, graph); | ||
|
|
||
| outputs->name = av_strdup("in"); | ||
| outputs->filter_ctx = src_ctx; | ||
| outputs->pad_idx = 0; | ||
| outputs->next = nullptr; | ||
|
|
||
| inputs->name = av_strdup("out"); | ||
| inputs->filter_ctx = sink_ctx; | ||
| inputs->pad_idx = 0; | ||
| inputs->next = nullptr; | ||
|
|
||
| // av_log_set_level(AV_LOG_DEBUG); | ||
|
|
||
| if (avfilter_graph_parse_ptr(graph, filter_descr.c_str(), &inputs, &outputs, nullptr) < 0) { | ||
| throw std::runtime_error("Failed to parse filter graph"); | ||
| } | ||
| if (avfilter_graph_config(graph, nullptr) < 0) { | ||
| throw std::runtime_error("Failed to configure filter graph"); | ||
| } | ||
|
|
||
| // 2. Find encoder | ||
| const AVCodec * codec = avcodec_find_encoder_by_name(CodecName::value); | ||
| if (!codec) { | ||
| throw std::runtime_error("Codec not found"); | ||
| } | ||
| encoder_ctx_ = avcodec_alloc_context3(codec); | ||
| encoder_ctx_->width = WIDTH; | ||
| encoder_ctx_->height = HEIGHT; | ||
| encoder_ctx_->time_base = AVRational{1, FPS}; | ||
| encoder_ctx_->framerate = AVRational{FPS, 1}; | ||
| encoder_ctx_->gop_size = 10; | ||
| encoder_ctx_->max_b_frames = 0; | ||
| encoder_ctx_->pix_fmt = AV_PIX_FMT_YUV420P; | ||
| encoder_ctx_->bit_rate = BITRATE; | ||
|
|
||
| // Enable encoder acceleration options to reduce test duration | ||
| if (std::string(CodecName::value) == "libx264" || std::string(CodecName::value) == "libx265") { | ||
| av_opt_set(encoder_ctx_->priv_data, "tune", "zerolatency", 0); | ||
| av_opt_set(encoder_ctx_->priv_data, "preset", "ultrafast", 0); | ||
| } else if (std::string(CodecName::value) == "libaom-av1") { | ||
| // accelerated options for AV1 | ||
| // Without these, the test cases for AV1 takes over 2min, which causes colcon test timeout | ||
| // cpu-used: It can specify 0--8. larger value lower compression (8 is fastest) | ||
| av_opt_set(encoder_ctx_->priv_data, "cpu-used", "8", 0); | ||
| // usage: set `realtime` to minimize delay and processing time | ||
| av_opt_set(encoder_ctx_->priv_data, "usage", "realtime", 0); | ||
| } | ||
|
|
||
| if (avcodec_open2(encoder_ctx_, codec, nullptr) < 0) { | ||
| throw std::runtime_error("Could not open encoder"); | ||
| } | ||
|
|
||
| // 3. Prepare packet buffer | ||
| pkt_ = av_packet_alloc(); | ||
| frame_ = av_frame_alloc(); | ||
| frame_->format = encoder_ctx_->pix_fmt; | ||
| frame_->width = encoder_ctx_->width; | ||
| frame_->height = encoder_ctx_->height; | ||
| av_frame_get_buffer(frame_, 32); | ||
|
|
||
| // 4. Store graph for later use | ||
| graph_ = graph; | ||
| src_ctx_ = src_ctx; | ||
| sink_ctx_ = sink_ctx; | ||
| } |
There was a problem hiding this comment.
Resource leak in constructor: The AVFilterInOut objects allocated at lines 105-106 with avfilter_inout_alloc() are never freed. These should be freed with avfilter_inout_free() after avfilter_graph_parse_ptr() is called. Additionally, if any of the exceptions are thrown (lines 130, 133, 139, 165), previously allocated resources (graph, inputs, outputs, encoder_ctx_, etc.) will leak. Consider using RAII wrappers or adding proper cleanup on all error paths.
There was a problem hiding this comment.
Thank you for your PR! The code and tests looks good to me. I also confirmed all of tests passed.
It seems OK to merge after fixing typos, and I'd appreciate if you add README!
I have two questions, is my understanding correct?
- To get decompressed image, we need at least two or more processing.
- If we try to receive the results of
FfmpegVideoDecompressor::process(...)after N processes, we will only get a maximum of N-1 decompressed images. If we want to get all of them, we should register a postprocess function and process them there.
Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
…ption Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
Signed-off-by: Manato HIRABAYASHI <manato.hirabayashi@tier4.jp>
|
@ktro2828
I added the README for the decompression package in 920dbf4 !
Thanks for the confirmation!
Yes, FYI: I implemented FYI2: In the current implementation of |
PR Type
Related Links
Description
This pull request introduces the video decoder support based on the FFMPEG libraries.
At this moment, the implementation assumes
H.264,H.265, andAV1encoding, which were introduced by the former [pull request]((#29).The key modifications are:
accelerated_image_processor_decompression[1]decompress_node[1]The following diagram depicts an overall structure:
classDiagram direction TD %% ---------- Base Processor ---------- class BaseProcessor { } %% ---------- Video Decompressor ---------- class VideoDecompressor class FfmpegVideoDecompressor { +process(image: Image): optional<Image> +process_impl(image: Image): Image -process_packet(image: Image): vector<Image> } BaseProcessor <|-- VideoDecompressor VideoDecompressor <|-- FfmpegVideoDecompressor %% ---------- ROS Node ---------- class `rclcpp::Node` class DecompressNode { +on_ffmpeg_packet(msg: ffmpeg_packet::ConstSharedPtr): void +publish_decompressed(image: Image): void } `rclcpp::Node` <|-- DecompressNode %% ---------- Association ---------- DecompressNode *-- FfmpegVideoDecompressor : instanticate as `decompressor` note for FfmpegVideoDecompressor "process_impl() is overridden as dummy due to the return type incompatibility. <br>The actual packet handling is performed in proces_packet()"Review Procedure
Build package
colcon build --symlink-install --cmake-args -DCMAKE_BUILD_TYPE=Release \ --packages-up-to accelerated_image_processor_ros source ./install/setup.bashLaunch the node
I also added test codes for the newly introduced functions. All tests passed on my environment:
colcon test --event-handlers console_cohesion+ --packages-select accelerated_image_processor_decompression
colcon test --event-handlers console_cohesion+ --packages-select accelerated_image_processor_ros
Remarks
Pre-Review Checklist for the PR Author
PR Author should check the checkboxes below when creating the PR.
Checklist for the PR Reviewer
Reviewers should check the checkboxes below before approval.
Post-Review Checklist for the PR Author
PR Author should check the checkboxes below before merging.
CI Checks