Merge pull request #27153 from 03kiko:fix-videowriter-writing-colorless-images-26276

03kiko · web-flow · commit 62b36495cc5d · 2025-06-09T10:05:02.000+03:00
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
diff --git a/modules/videoio/src/cap_ffmpeg.cpp b/modules/videoio/src/cap_ffmpeg.cpp
@@ -194,7 +194,8 @@ class CvVideoWriter_FFMPEG_proxy CV_FINAL :
             }
         }
 
-        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
@@ -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
@@ -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
@@ -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<int /*inputType*/, int /*Depth*/, bool /*isColor*/, bool /*isValid*/, string /*description*/> 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<videoio_ffmpeg_channel_mismatch::ParamType>& 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

Original file line number	Diff line number	Diff line change
`@@ -194,7 +194,8 @@ class CvVideoWriter_FFMPEG_proxy CV_FINAL :`
`194`	`194`	`}`
`195`	`195`	`}`
`196`	`196`
`197`		`- icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.channels(), 0);`
	`197`	`+ if (!icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.channels(), 0))`
	`198`	`+ CV_LOG_WARNING(NULL, "FFmpeg: Failed to write frame");`
`198`	`199`	`}`
`199`	`200`	`virtual bool open( const cv::String& filename, int fourcc, double fps, cv::Size frameSize, const VideoWriterParameters& params )`
`200`	`201`	`{`
Original file line number	Diff line number	Diff line change
`@@ -2525,11 +2525,13 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int`
`2525`	`2525`	`// check parameters`
`2526`	`2526`	`if (input_pix_fmt == AV_PIX_FMT_BGR24) {`
`2527`	`2527`	`if (cn != 3) {`
	`2528`	`+ CV_LOG_WARNING(NULL, "write frame skipped - expected 3 channels but got " << cn);`
`2528`	`2529`	`return false;`
`2529`	`2530`	`}`
`2530`	`2531`	`}`
`2531`	`2532`	`else if (input_pix_fmt == AV_PIX_FMT_GRAY8 \|\| input_pix_fmt == AV_PIX_FMT_GRAY16LE) {`
`2532`	`2533`	`if (cn != 1) {`
	`2534`	`+ CV_LOG_WARNING(NULL, "write frame skipped - expected 1 channel but got " << cn);`
`2533`	`2535`	`return false;`
`2534`	`2536`	`}`
`2535`	`2537`	`}`
`@@ -2640,14 +2642,16 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int`
`2640`	`2642`	`}`
`2641`	`2643`	`hw_frame->pts = frame_idx;`
`2642`	`2644`	`int ret_write = icv_av_write_frame_FFMPEG(oc, video_st, context, outbuf, outbuf_size, hw_frame, frame_idx);`
`2643`		`- ret = ret_write >= 0 ? true : false;`
	`2645`	`+ // AVERROR(EAGAIN): continue sending input, not an error`
	`2646`	`+ ret = (ret_write >= 0 \|\| ret_write == AVERROR(EAGAIN));`
`2644`	`2647`	`av_frame_free(&hw_frame);`
`2645`	`2648`	`} else`
`2646`	`2649`	`#endif`
`2647`	`2650`	`{`
`2648`	`2651`	`picture->pts = frame_idx;`
`2649`	`2652`	`int ret_write = icv_av_write_frame_FFMPEG(oc, video_st, context, outbuf, outbuf_size, picture, frame_idx);`
`2650`		`- ret = ret_write >= 0 ? true : false;`
	`2653`	`+ // AVERROR(EAGAIN): continue sending input, not an error`
	`2654`	`+ ret = (ret_write >= 0 \|\| ret_write == AVERROR(EAGAIN));`
`2651`	`2655`	`}`
`2652`	`2656`
`2653`	`2657`	`frame_idx++;`
Original file line number	Diff line number	Diff line change
`@@ -2699,7 +2699,7 @@ void CvVideoWriter_GStreamer::write(InputArray image)`
`2699`	`2699`	`}`
`2700`	`2700`	`else if (input_pix_fmt == GST_VIDEO_FORMAT_GRAY16_LE) {`
`2701`	`2701`	`if (image.type() != CV_16UC1) {`
`2702`		`- CV_WARN("write frame skipped - expected CV_16UC3");`
	`2702`	`+ CV_WARN("write frame skipped - expected CV_16UC1");`
`2703`	`2703`	`return;`
`2704`	`2704`	`}`
`2705`	`2705`	`}`