From 62b36495cc5dea1e5d8923fe3ea98975fdc85e2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Mon, 9 Jun 2025 08:05:02 +0100 Subject: [PATCH] Merge pull request #27153 from 03kiko:fix-videowriter-writing-colorless-images-26276 fix #26276: cv::VideoWriter fails writing colorless images #27153 When writing grayscale images (`isColor=false`), `cv::VideoWriter` failed as FFmpeg backend requires input frames to be in a grayscale format. Unlike other backends, FFmpeg doesn't perform this conversion internally and expects the input to be pre-converted. To fix this, I inserted a check in the `CvVideoWriter_FFMPEG_proxy::write` to convert input frames to grayscale when the input has more than 1 channel. If this is true, then the input is converted to a gray image using `cv::cvtColor` (with `cv::COLOR_BGR2GRAY`). Additionally, as suggested in the issue comments, I have correctly propagated the return value of `CvVideoWriter_FFMPEG::writeFrame` back to the `CvVideoWriter_FFMPEG_proxy::write`. This return value wasn't being used, and `writeFrame` was always returning false since the FFmeg's return code `AVERROR(EAGAIN)` was mistakenly being treated as an error. Now it's handled as a signal for additional input (current input was successfully written, and a new frame should be sent. [See FFmpeg documentation for `avcodec_receive_packet`](https://ffmpeg.org/doxygen/6.1/group__lavc__decoding.html)). A warning is displayed if `CvVideoWriter_FFMPEG::writeFrame` returns false. Alternatively, this could be propagated back up to the user, making cv::VideoWriter::write a boolean. Finally, I added a test case to verify if the grayscale conversion is being done correctly. Fixes #26276 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake --- modules/videoio/src/cap_ffmpeg.cpp | 3 +- modules/videoio/src/cap_ffmpeg_impl.hpp | 8 ++- modules/videoio/src/cap_gstreamer.cpp | 2 +- modules/videoio/test/test_ffmpeg.cpp | 77 +++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/modules/videoio/src/cap_ffmpeg.cpp b/modules/videoio/src/cap_ffmpeg.cpp index 62f64cb0f7..cc62920c74 100644 --- a/modules/videoio/src/cap_ffmpeg.cpp +++ b/modules/videoio/src/cap_ffmpeg.cpp @@ -194,7 +194,8 @@ public: } } - icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.channels(), 0); + if (!icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.channels(), 0)) + CV_LOG_WARNING(NULL, "FFmpeg: Failed to write frame"); } virtual bool open( const cv::String& filename, int fourcc, double fps, cv::Size frameSize, const VideoWriterParameters& params ) { diff --git a/modules/videoio/src/cap_ffmpeg_impl.hpp b/modules/videoio/src/cap_ffmpeg_impl.hpp index 4ec2137a39..9cd20fbf77 100644 --- a/modules/videoio/src/cap_ffmpeg_impl.hpp +++ b/modules/videoio/src/cap_ffmpeg_impl.hpp @@ -2525,11 +2525,13 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int // check parameters if (input_pix_fmt == AV_PIX_FMT_BGR24) { if (cn != 3) { + CV_LOG_WARNING(NULL, "write frame skipped - expected 3 channels but got " << cn); return false; } } else if (input_pix_fmt == AV_PIX_FMT_GRAY8 || input_pix_fmt == AV_PIX_FMT_GRAY16LE) { if (cn != 1) { + CV_LOG_WARNING(NULL, "write frame skipped - expected 1 channel but got " << cn); return false; } } @@ -2640,14 +2642,16 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int } hw_frame->pts = frame_idx; int ret_write = icv_av_write_frame_FFMPEG(oc, video_st, context, outbuf, outbuf_size, hw_frame, frame_idx); - ret = ret_write >= 0 ? true : false; + // AVERROR(EAGAIN): continue sending input, not an error + ret = (ret_write >= 0 || ret_write == AVERROR(EAGAIN)); av_frame_free(&hw_frame); } else #endif { picture->pts = frame_idx; int ret_write = icv_av_write_frame_FFMPEG(oc, video_st, context, outbuf, outbuf_size, picture, frame_idx); - ret = ret_write >= 0 ? true : false; + // AVERROR(EAGAIN): continue sending input, not an error + ret = (ret_write >= 0 || ret_write == AVERROR(EAGAIN)); } frame_idx++; diff --git a/modules/videoio/src/cap_gstreamer.cpp b/modules/videoio/src/cap_gstreamer.cpp index a158119d3f..ff1ff2b028 100644 --- a/modules/videoio/src/cap_gstreamer.cpp +++ b/modules/videoio/src/cap_gstreamer.cpp @@ -2699,7 +2699,7 @@ void CvVideoWriter_GStreamer::write(InputArray image) } else if (input_pix_fmt == GST_VIDEO_FORMAT_GRAY16_LE) { if (image.type() != CV_16UC1) { - CV_WARN("write frame skipped - expected CV_16UC3"); + CV_WARN("write frame skipped - expected CV_16UC1"); return; } } diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 03e41bd8f0..eeb0835078 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -953,4 +953,81 @@ inline static std::string videoio_ffmpeg_16bit_name_printer(const testing::TestP INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_16bit, testing::ValuesIn(sixteen_bit_modes), videoio_ffmpeg_16bit_name_printer); +typedef tuple ChannelMismatchTestParams; +typedef testing::TestWithParam< ChannelMismatchTestParams > videoio_ffmpeg_channel_mismatch; + +TEST_P(videoio_ffmpeg_channel_mismatch, basic) +{ + if (!videoio_registry::hasBackend(CAP_FFMPEG)) + throw SkipTestException("FFmpeg backend was not found"); + + const string filename = "mismatch_video.mp4"; + int input_type = get<0>(GetParam()); + int depth = get<1>(GetParam()); + bool is_Color = get<2>(GetParam()); + bool is_valid = get<3>(GetParam()); + const string description = get<4>(GetParam()); + + const double fps = 15.0; + const int fourcc = VideoWriter::fourcc('m', 'p', '4', 'v'); + const Mat frame(480, 640, input_type, Scalar::all(0)); + + VideoWriter writer(filename, fourcc, fps, frame.size(), + { + cv::VIDEOWRITER_PROP_DEPTH, depth, + VIDEOWRITER_PROP_IS_COLOR, is_Color + }); + + if (!writer.isOpened()) + throw SkipTestException("Failed to open video writer"); + + for (int i = 1; i <= 15; i++) + { + // In case of mismatch between input frame channels and + // expected depth/isColor configuration a warning should be printed communicating it + writer.write(frame); + } + + writer.release(); + + VideoCapture cap(filename, CAP_FFMPEG); + + if (is_valid) { + ASSERT_TRUE(cap.isOpened()) << "Can't open video for " << description; + EXPECT_EQ(cap.get(CAP_PROP_FRAME_COUNT), 15) << "All frames should be written for: " << description; + } else { + ASSERT_FALSE(cap.isOpened()) << "Video capture should fail to open for: " << description; + } + + std::remove(filename.c_str()); +} + +const ChannelMismatchTestParams mismatch_cases[] = +{ + // Testing input frame channels and expected depth/isColor combinations + + // Open VideoWriter depth/isColor combination: CV_8U/true, everything with 3 channels should be valid + make_tuple(CV_16UC1, CV_8U, true, false, "input_CV_16UC1_expected_CV_8U_isColor_true"), + make_tuple(CV_8UC1, CV_8U, true, false, "input_CV_8UC1_expected_CV_8U_isColor_true"), + make_tuple(CV_8UC3, CV_8U, true, true, "input_CV_8UC3_expected_CV_8U_isColor_true_valid"), + make_tuple(CV_16UC3, CV_8U, true, true, "input_CV_16UC3_expected_CV_8U_isColor_true_valid"), + + // Open VideoWriter depth/isColor combination: 16U,8U/false, everything with 1 channel should be valid + make_tuple(CV_8UC3, CV_8U, false, false, "input_CV_8UC3_expected_CV_8U_isColor_false"), + make_tuple(CV_16UC3, CV_8U, false, false, "input_CV_16UC3_expected_CV_8U_isColor_false"), + make_tuple(CV_8UC3, CV_16U, false, false, "input_CV_8UC3_expected_CV_16U_isColor_false"), + make_tuple(CV_16UC3, CV_16U, false, false, "input_CV_16UC3_expected_CV_16U_isColor_false"), + make_tuple(CV_8UC1, CV_16U, false, true, "input_CV_8UC1_expected_CV_16U_isColor_false_valid"), + make_tuple(CV_16UC1, CV_8U, false, true, "input_CV_16UC1_expected_CV_8U_isColor_false_valid"), +}; + +inline static std::string videoio_ffmpeg_mismatch_name_printer(const testing::TestParamInfo& info) +{ + std::ostringstream os; + os << get<4>(info.param); + return os.str(); +} + +INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_channel_mismatch, testing::ValuesIn(mismatch_cases), videoio_ffmpeg_mismatch_name_printer); + }} // namespace