From 30a5ad1cee6f3d1c5bab31022926be32bb5c40f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Mon, 24 Mar 2025 22:03:02 +0000 Subject: [PATCH 01/10] fix #26276: VideoWriter fails on grayscale images The internal return values of the FFmpeg library are now correctly handled in the FFmpeg backend. The writeFrame function now considers that the FFmpeg return code AVERROR(EAGAIN) indicates the encoder needs more input, so a new frame must be sent instead of treating it as a real error. Additionally, the return value of CvVideoWriter_FFMPEG::writeFrame is now properly propagated back to the write function in the cap_ffmpeg.cpp file, which displays a warning when writeFrame returns false. Additionally, a conversion of the input to grayscale was implemented before entering FFmpeg backend, as the encoder expects grayscale frames when isColor=false. This requires converting from CV_8UC3 to grayscale (a simple color conversion is done here using the cvtColor function). Finally, a new test case has been implemented to verify that the grayscale conversion is correctly applied. --- modules/videoio/src/cap_ffmpeg.cpp | 11 ++++- modules/videoio/src/cap_ffmpeg_impl.hpp | 12 ++++- modules/videoio/test/test_ffmpeg.cpp | 59 +++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/modules/videoio/src/cap_ffmpeg.cpp b/modules/videoio/src/cap_ffmpeg.cpp index 62f64cb0f7..2a7d626311 100644 --- a/modules/videoio/src/cap_ffmpeg.cpp +++ b/modules/videoio/src/cap_ffmpeg.cpp @@ -194,7 +194,16 @@ public: } } - icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.channels(), 0); + cv::Mat imageMat = image.getMat(); + // when isColor=false, image must have 1 channel (grayscale) + if (!ffmpegWriter->isColor() && imageMat.channels() != 1) + { + // convert input image to grayscale + cv::cvtColor(imageMat, imageMat, cv::COLOR_BGR2GRAY); + } + + if(!icvWriteFrame_FFMPEG_p(ffmpegWriter, imageMat.data, (int)imageMat.step, imageMat.cols, imageMat.rows, imageMat.channels(), 0)) + CV_LOG_WARNING(NULL, "FFmpeg: Unable 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..6f0fd41dfa 100644 --- a/modules/videoio/src/cap_ffmpeg_impl.hpp +++ b/modules/videoio/src/cap_ffmpeg_impl.hpp @@ -2143,6 +2143,7 @@ struct CvVideoWriter_FFMPEG bool writeHWFrame(cv::InputArray input); double getProperty(int propId) const; bool setProperty(int, double); + bool isColor(); void init(); @@ -2640,14 +2641,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) means the encoder needs more 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) means the encoder needs more input, not an error + ret = (ret_write >= 0 || ret_write == AVERROR(EAGAIN)); } frame_idx++; @@ -2731,6 +2734,11 @@ bool CvVideoWriter_FFMPEG::setProperty(int property_id, double value) return true; } +bool CvVideoWriter_FFMPEG::isColor() +{ + return input_pix_fmt == AV_PIX_FMT_BGR24; +} + /// close video output stream and free associated memory void CvVideoWriter_FFMPEG::close() { diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 03e41bd8f0..eba82a9784 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -953,4 +953,63 @@ 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); +TEST(videoio_ffmpeg, write_colorless_images) { + if (!videoio_registry::hasBackend(CAP_FFMPEG)) + throw SkipTestException("FFmpeg backend was not found"); + + const std::string filename = "grayscale_video.mp4"; + const Mat frame(480, 640, CV_8UC3, Scalar(255, 0, 0)); // 3 channels (BGR) + const double fps = 30.0; + const bool isColor = false; // encoder will work with grayscale frames + + VideoWriter writer(filename, VideoWriter::fourcc('m', 'p', '4', 'v'), fps, frame.size(), isColor); + + ASSERT_TRUE(writer.isOpened()) << "Failed to open video writer"; + + for (int i = 1; i <= 90; i++) + { + // write converts input to grayscale internally, + // as ffmpeg doesn't handle this conversion + writer.write(frame); + } + + writer.release(); + + EXPECT_GT(getFileSize(filename), 0); + + VideoCapture cap(filename, CAP_FFMPEG); + if (!cap.isOpened()) + throw SkipTestException("Failed to open video file, stream not supported or invalid"); + + Mat readFrame; + int frameCount = 0; + while (cap.read(readFrame)) + { + // check if grayscale conversion was successful + bool isGrayscale = true; + for (int y = 0; y < readFrame.rows; y++) + { + for (int x = 0; x < readFrame.cols; x++) + { + Vec3b pixel = readFrame.at(y, x); + // all 3 channels (B, G, R) must have the same value for grayscale + if (pixel[0] != pixel[1] || pixel[1] != pixel[2]) { + isGrayscale = false; + break; + } + } + if (!isGrayscale) + break; + } + + ASSERT_TRUE(isGrayscale) << "Frame is not grayscale"; + + frameCount++; + } + + EXPECT_EQ(frameCount, 90); + + std::remove(filename.c_str()); +} + }} // namespace From e9a059255206042f1dd91392a770769677520840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Thu, 1 May 2025 21:32:16 +0100 Subject: [PATCH 02/10] fix #26276: conversion in writeFrame & test cases Aligns FFmpeg backend behavior with MS Media Foundation and Android, by moving the internal conversion logic to writeFrame(). This now handles all relevant format conversions: - CV_8UC3 -> CV_8UC1 or CV_16UC1 - CV_8UC1 -> CV_8UC3 or CV_16UC1 - CV_16UC1 -> CV_8UC1 or CV_8UC3 It still corrects AVERROR(EAGAIN) handling, which was previously misinterpreting it as an error and displays a warning when writeFrame returns false. Adds more test cases ensuring all possible conversions are properly tested. --- modules/videoio/src/cap_ffmpeg.cpp | 10 +-- modules/videoio/src/cap_ffmpeg_impl.hpp | 71 ++++++++++++-------- modules/videoio/test/test_ffmpeg.cpp | 89 +++++++++++++++---------- 3 files changed, 98 insertions(+), 72 deletions(-) diff --git a/modules/videoio/src/cap_ffmpeg.cpp b/modules/videoio/src/cap_ffmpeg.cpp index 2a7d626311..b0c28eac44 100644 --- a/modules/videoio/src/cap_ffmpeg.cpp +++ b/modules/videoio/src/cap_ffmpeg.cpp @@ -194,15 +194,7 @@ public: } } - cv::Mat imageMat = image.getMat(); - // when isColor=false, image must have 1 channel (grayscale) - if (!ffmpegWriter->isColor() && imageMat.channels() != 1) - { - // convert input image to grayscale - cv::cvtColor(imageMat, imageMat, cv::COLOR_BGR2GRAY); - } - - if(!icvWriteFrame_FFMPEG_p(ffmpegWriter, imageMat.data, (int)imageMat.step, imageMat.cols, imageMat.rows, imageMat.channels(), 0)) + if (!icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.type(), 0)) CV_LOG_WARNING(NULL, "FFmpeg: Unable 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 6f0fd41dfa..dd1e0bf88e 100644 --- a/modules/videoio/src/cap_ffmpeg_impl.hpp +++ b/modules/videoio/src/cap_ffmpeg_impl.hpp @@ -2139,11 +2139,10 @@ struct CvVideoWriter_FFMPEG bool open( const char* filename, int fourcc, double fps, int width, int height, const VideoWriterParameters& params ); void close(); - bool writeFrame( const unsigned char* data, int step, int width, int height, int cn, int origin ); + bool writeFrame( const unsigned char* data, int step, int width, int height, int type, int origin ); bool writeHWFrame(cv::InputArray input); double getProperty(int propId) const; bool setProperty(int, double); - bool isColor(); void init(); @@ -2513,34 +2512,57 @@ static int icv_av_write_frame_FFMPEG( AVFormatContext * oc, AVStream * video_st, } /// write a frame with FFMPEG -bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int width, int height, int cn, int origin ) +bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int width, int height, int type, int origin ) { if (!encode_video) { - CV_Assert(cn == 1 && ((width > 0 && height == 1) || (width == 1 && height > 0 && step == 1))); + CV_Assert((type == CV_16UC1 || type == CV_8UC1) && ((width > 0 && height == 1) || (width == 1 && height > 0 && step == 1))); const bool set_key_frame = key_frame ? key_frame : idr_period ? frame_idx % idr_period == 0 : 1; bool ret = icv_av_encapsulate_video_FFMPEG(oc, video_st, context, (uint8_t*)data, width, frame_idx, pts_index, b_frame_dts_delay, set_key_frame); frame_idx++; return ret; } - // check parameters - if (input_pix_fmt == AV_PIX_FMT_BGR24) { - if (cn != 3) { - return false; - } - } - else if (input_pix_fmt == AV_PIX_FMT_GRAY8 || input_pix_fmt == AV_PIX_FMT_GRAY16LE) { - if (cn != 1) { - return false; - } - } - else { - CV_LOG_WARNING(NULL, "Input data does not match selected pixel format: " - << av_get_pix_fmt_name(input_pix_fmt) - << ", number of channels: " << cn); - CV_Assert(false); + // check parameters and do conversions if needed + cv::Mat inputMat(height, width, type, (void*)data, step); + cv::Mat convertedMat; + switch (input_pix_fmt) { + case AV_PIX_FMT_BGR24: // expected CV_8UC3 + if (type == CV_8UC3) + convertedMat = inputMat; + else { + if (type == CV_16UC1) // CV_16UC1 -> CV_8UC1 + inputMat.convertTo(inputMat, CV_8UC1, 1.0 / 256); + cv::cvtColor(inputMat, convertedMat, cv::COLOR_GRAY2BGR); // CV_8UC1 -> CV_8UC3 + } + break; + case AV_PIX_FMT_GRAY8: // expected CV_8UC1 + if (type == CV_8UC1) + convertedMat = inputMat; + else if (type == CV_8UC3) // CV_8UC3 -> CV_8UC1 + cv::cvtColor(inputMat, convertedMat, COLOR_BGR2GRAY); + else // CV_16UC1 -> CV_8UC1 + inputMat.convertTo(convertedMat, CV_8UC1, 1.0 / 256); + break; + case AV_PIX_FMT_GRAY16LE: // expected CV_16UC1 + if (type == CV_16UC1) + convertedMat = inputMat; + else { + if (type == CV_8UC3) + cv::cvtColor(inputMat, inputMat, COLOR_BGR2GRAY); // CV_8UC3 -> CV_8UC1 + inputMat.convertTo(convertedMat, CV_16UC1, 256.0); // CV_8UC1 -> CV_16UC1 + } + break; + + default: + CV_LOG_WARNING(NULL, "Unknown pixel format: " << av_get_pix_fmt_name(input_pix_fmt)); + CV_Assert(false); + break; } + data = convertedMat.data; + step = (int)convertedMat.step; + type = convertedMat.type(); + if( (width & -2) != frame_width || (height & -2) != frame_height || !data ) return false; width = frame_width; @@ -2734,11 +2756,6 @@ bool CvVideoWriter_FFMPEG::setProperty(int property_id, double value) return true; } -bool CvVideoWriter_FFMPEG::isColor() -{ - return input_pix_fmt == AV_PIX_FMT_BGR24; -} - /// close video output stream and free associated memory void CvVideoWriter_FFMPEG::close() { @@ -3464,7 +3481,7 @@ void cvReleaseVideoWriter_FFMPEG( CvVideoWriter_FFMPEG** writer ) int cvWriteFrame_FFMPEG( CvVideoWriter_FFMPEG* writer, const unsigned char* data, int step, - int width, int height, int cn, int origin) + int width, int height, int type, int origin) { - return writer->writeFrame(data, step, width, height, cn, origin); + return writer->writeFrame(data, step, width, height, type, origin); } diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index eba82a9784..00247fca93 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -953,63 +953,80 @@ 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); -TEST(videoio_ffmpeg, write_colorless_images) { +typedef tuple ConversionTestParams; +typedef testing::TestWithParam< ConversionTestParams > videoio_ffmpeg_conversion; + +TEST_P(videoio_ffmpeg_conversion, handle_conversion) +{ if (!videoio_registry::hasBackend(CAP_FFMPEG)) throw SkipTestException("FFmpeg backend was not found"); - - const std::string filename = "grayscale_video.mp4"; - const Mat frame(480, 640, CV_8UC3, Scalar(255, 0, 0)); // 3 channels (BGR) + + const string filename = "conversion_video.mp4"; + int input_type = get<0>(GetParam()); + int target_type = get<1>(GetParam()); + bool isColor = get<2>(GetParam()); + string description = get<3>(GetParam()); + const double fps = 30.0; - const bool isColor = false; // encoder will work with grayscale frames + 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(), isColor); + writer.set(VIDEOWRITER_PROP_DEPTH, CV_MAT_DEPTH(target_type)); - VideoWriter writer(filename, VideoWriter::fourcc('m', 'p', '4', 'v'), fps, frame.size(), isColor); - - ASSERT_TRUE(writer.isOpened()) << "Failed to open video writer"; + ASSERT_TRUE(writer.isOpened()) << "Failed to open video writer for: " << description; for (int i = 1; i <= 90; i++) { - // write converts input to grayscale internally, - // as ffmpeg doesn't handle this conversion + // Conversion happens internally + // as ffmpeg doesn't handle itself. + // If conversion fails, FFmpeg won't write any frames writer.write(frame); } writer.release(); - EXPECT_GT(getFileSize(filename), 0); + EXPECT_GT(getFileSize(filename), 0) << "Output file is empty for: " << description; VideoCapture cap(filename, CAP_FFMPEG); if (!cap.isOpened()) - throw SkipTestException("Failed to open video file, stream not supported or invalid"); - + throw SkipTestException("Failed to open written video"); + Mat readFrame; int frameCount = 0; while (cap.read(readFrame)) - { - // check if grayscale conversion was successful - bool isGrayscale = true; - for (int y = 0; y < readFrame.rows; y++) - { - for (int x = 0; x < readFrame.cols; x++) - { - Vec3b pixel = readFrame.at(y, x); - // all 3 channels (B, G, R) must have the same value for grayscale - if (pixel[0] != pixel[1] || pixel[1] != pixel[2]) { - isGrayscale = false; - break; - } - } - if (!isGrayscale) - break; - } - - ASSERT_TRUE(isGrayscale) << "Frame is not grayscale"; - frameCount++; - } - - EXPECT_EQ(frameCount, 90); + + // Simply checking if all 90 frames can be read is enough. + // If no conversion is done 0 frames will be written. + EXPECT_EQ(frameCount, 90) << "Not all frames were read for: " << description; std::remove(filename.c_str()); } +const ConversionTestParams conversion_cases[] = +{ + // converting to 8UC3 + make_tuple(CV_8UC1, CV_8UC3, true, "CV_8UC1_to_CV_8UC3"), + make_tuple(CV_16UC1, CV_8UC3, true, "CV_16UC1_to_CV_8UC3"), + + // converting to 8UC1 + make_tuple(CV_8UC3, CV_8UC1, false, "CV_8UC3_to_CV_8UC1"), + make_tuple(CV_16UC1, CV_8UC1, false, "CV_16UC1_to_CV_8UC1"), + + // converting to 16UC1 + make_tuple(CV_8UC3, CV_16UC1, false, "CV_8UC3_to_CV_16UC1"), + make_tuple(CV_8UC1, CV_16UC1, false, "CV_8UC1_to_CV_16UC1"), +}; + +inline static std::string videoio_ffmpeg_conversion_name_printer(const testing::TestParamInfo& info) +{ + std::ostringstream os; + os << get<3>(info.param) << "_" + << "isColor_" << (get<2>(info.param) ? "true" : "false"); + return os.str(); +} + +INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_conversion, testing::ValuesIn(conversion_cases), videoio_ffmpeg_conversion_name_printer); + }} // namespace From 62d6fa72d5677a487021c7b12fd44cf8dc64f084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Thu, 1 May 2025 22:19:59 +0100 Subject: [PATCH 03/10] fix #26276: removes trailing whitespaces --- modules/videoio/src/cap_ffmpeg_impl.hpp | 2 +- modules/videoio/test/test_ffmpeg.cpp | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/modules/videoio/src/cap_ffmpeg_impl.hpp b/modules/videoio/src/cap_ffmpeg_impl.hpp index dd1e0bf88e..60b5f53469 100644 --- a/modules/videoio/src/cap_ffmpeg_impl.hpp +++ b/modules/videoio/src/cap_ffmpeg_impl.hpp @@ -2552,7 +2552,7 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int inputMat.convertTo(convertedMat, CV_16UC1, 256.0); // CV_8UC1 -> CV_16UC1 } break; - + default: CV_LOG_WARNING(NULL, "Unknown pixel format: " << av_get_pix_fmt_name(input_pix_fmt)); CV_Assert(false); diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 00247fca93..59e387aa5d 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -960,17 +960,17 @@ TEST_P(videoio_ffmpeg_conversion, handle_conversion) { if (!videoio_registry::hasBackend(CAP_FFMPEG)) throw SkipTestException("FFmpeg backend was not found"); - + const string filename = "conversion_video.mp4"; int input_type = get<0>(GetParam()); int target_type = get<1>(GetParam()); bool isColor = get<2>(GetParam()); string description = get<3>(GetParam()); - + const double fps = 30.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(), isColor); writer.set(VIDEOWRITER_PROP_DEPTH, CV_MAT_DEPTH(target_type)); @@ -991,15 +991,15 @@ TEST_P(videoio_ffmpeg_conversion, handle_conversion) VideoCapture cap(filename, CAP_FFMPEG); if (!cap.isOpened()) throw SkipTestException("Failed to open written video"); - + Mat readFrame; int frameCount = 0; while (cap.read(readFrame)) frameCount++; - + // Simply checking if all 90 frames can be read is enough. // If no conversion is done 0 frames will be written. - EXPECT_EQ(frameCount, 90) << "Not all frames were read for: " << description; + EXPECT_EQ(frameCount, 90) << "Not all frames were read for: " << description; std::remove(filename.c_str()); } @@ -1009,11 +1009,11 @@ const ConversionTestParams conversion_cases[] = // converting to 8UC3 make_tuple(CV_8UC1, CV_8UC3, true, "CV_8UC1_to_CV_8UC3"), make_tuple(CV_16UC1, CV_8UC3, true, "CV_16UC1_to_CV_8UC3"), - + // converting to 8UC1 make_tuple(CV_8UC3, CV_8UC1, false, "CV_8UC3_to_CV_8UC1"), make_tuple(CV_16UC1, CV_8UC1, false, "CV_16UC1_to_CV_8UC1"), - + // converting to 16UC1 make_tuple(CV_8UC3, CV_16UC1, false, "CV_8UC3_to_CV_16UC1"), make_tuple(CV_8UC1, CV_16UC1, false, "CV_8UC1_to_CV_16UC1"), From 5ea3f51fcb7e6d1539024b60bf957c8ac0ee1e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Fri, 2 May 2025 15:46:28 +0100 Subject: [PATCH 04/10] fix #26276: adds more descriptive error messages --- modules/videoio/test/test_ffmpeg.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 59e387aa5d..47a15cc1b4 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -965,7 +965,7 @@ TEST_P(videoio_ffmpeg_conversion, handle_conversion) int input_type = get<0>(GetParam()); int target_type = get<1>(GetParam()); bool isColor = get<2>(GetParam()); - string description = get<3>(GetParam()); + const string description = get<3>(GetParam()); const double fps = 30.0; const int fourcc = VideoWriter::fourcc('m', 'p', '4', 'v'); @@ -990,16 +990,15 @@ TEST_P(videoio_ffmpeg_conversion, handle_conversion) VideoCapture cap(filename, CAP_FFMPEG); if (!cap.isOpened()) - throw SkipTestException("Failed to open written video"); + throw SkipTestException("Failed to open video file, stream not supported or invalid"); Mat readFrame; int frameCount = 0; while (cap.read(readFrame)) frameCount++; - // Simply checking if all 90 frames can be read is enough. - // If no conversion is done 0 frames will be written. - EXPECT_EQ(frameCount, 90) << "Not all frames were read for: " << description; + // Checking if all 90 frames can be read is enough, if no conversion is done 0 frames will be written. + EXPECT_EQ(frameCount, 90) << "Not all frames were read for: " << description << "(expected 90, got " << frameCount << ")"; std::remove(filename.c_str()); } From 832f2ab2f0b17213c5f380180e9a54a69ee6bca9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Sun, 18 May 2025 01:02:31 +0100 Subject: [PATCH 05/10] fix #26276: fix typo gstreamer Fixes a minor typo in the warning message for type mismatches in the GStreamer backend. ("... expected CV_16UC3" -> "... expected CV_16UC1") This change is unrelated to the FFmpeg logic but improves consistency across backends. --- modules/videoio/src/cap_gstreamer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/videoio/src/cap_gstreamer.cpp b/modules/videoio/src/cap_gstreamer.cpp index e4a325041c..a17972cb95 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; } } From e4515048ac81fabb6e0902516f774c53e2921b66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Sun, 18 May 2025 01:49:04 +0100 Subject: [PATCH 06/10] fix #26276: changes upon review Replaces implicit data manipulation with a warning-based approach. Following reviewer feedback, data conversion is now explicitly left to the user, and no automatic transformations are performed in writeFrame(). --- modules/videoio/src/cap_ffmpeg_impl.hpp | 65 ++++++++++--------------- modules/videoio/test/test_ffmpeg.cpp | 47 ++++++++---------- 2 files changed, 45 insertions(+), 67 deletions(-) diff --git a/modules/videoio/src/cap_ffmpeg_impl.hpp b/modules/videoio/src/cap_ffmpeg_impl.hpp index 60b5f53469..7ac704a377 100644 --- a/modules/videoio/src/cap_ffmpeg_impl.hpp +++ b/modules/videoio/src/cap_ffmpeg_impl.hpp @@ -2515,53 +2515,38 @@ static int icv_av_write_frame_FFMPEG( AVFormatContext * oc, AVStream * video_st, bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int width, int height, int type, int origin ) { if (!encode_video) { - CV_Assert((type == CV_16UC1 || type == CV_8UC1) && ((width > 0 && height == 1) || (width == 1 && height > 0 && step == 1))); + CV_Assert(type == CV_8UC1 && ((width > 0 && height == 1) || (width == 1 && height > 0 && step == 1))); const bool set_key_frame = key_frame ? key_frame : idr_period ? frame_idx % idr_period == 0 : 1; bool ret = icv_av_encapsulate_video_FFMPEG(oc, video_st, context, (uint8_t*)data, width, frame_idx, pts_index, b_frame_dts_delay, set_key_frame); frame_idx++; return ret; } - // check parameters and do conversions if needed - cv::Mat inputMat(height, width, type, (void*)data, step); - cv::Mat convertedMat; - switch (input_pix_fmt) { - case AV_PIX_FMT_BGR24: // expected CV_8UC3 - if (type == CV_8UC3) - convertedMat = inputMat; - else { - if (type == CV_16UC1) // CV_16UC1 -> CV_8UC1 - inputMat.convertTo(inputMat, CV_8UC1, 1.0 / 256); - cv::cvtColor(inputMat, convertedMat, cv::COLOR_GRAY2BGR); // CV_8UC1 -> CV_8UC3 - } - break; - case AV_PIX_FMT_GRAY8: // expected CV_8UC1 - if (type == CV_8UC1) - convertedMat = inputMat; - else if (type == CV_8UC3) // CV_8UC3 -> CV_8UC1 - cv::cvtColor(inputMat, convertedMat, COLOR_BGR2GRAY); - else // CV_16UC1 -> CV_8UC1 - inputMat.convertTo(convertedMat, CV_8UC1, 1.0 / 256); - break; - case AV_PIX_FMT_GRAY16LE: // expected CV_16UC1 - if (type == CV_16UC1) - convertedMat = inputMat; - else { - if (type == CV_8UC3) - cv::cvtColor(inputMat, inputMat, COLOR_BGR2GRAY); // CV_8UC3 -> CV_8UC1 - inputMat.convertTo(convertedMat, CV_16UC1, 256.0); // CV_8UC1 -> CV_16UC1 - } - break; - - default: - CV_LOG_WARNING(NULL, "Unknown pixel format: " << av_get_pix_fmt_name(input_pix_fmt)); - CV_Assert(false); - break; + // check parameters + if (input_pix_fmt == AV_PIX_FMT_BGR24) { + if (type != CV_8UC3) { + CV_LOG_WARNING(NULL, "write frame skipped - expected frame with depth of CV_8UC3"); + return false; + } + } + else if (input_pix_fmt == AV_PIX_FMT_GRAY8) { + if (type != CV_8UC1) { + CV_LOG_WARNING(NULL, "write frame skipped - expected frame with depth of CV_8UC1"); + return false; + } + } + else if (input_pix_fmt == AV_PIX_FMT_GRAY16LE) { + if (type != CV_16UC1) { + CV_LOG_WARNING(NULL, "write frame skipped - expected frame with depth of CV_16UC1"); + return false; + } + } + else { + CV_LOG_WARNING(NULL, "Input data does not match selected pixel format: " + << av_get_pix_fmt_name(input_pix_fmt) + << ", only CV_8UC1, CV_8UC3, and CV_16UC1 are supported"); + CV_Assert(false); } - - data = convertedMat.data; - step = (int)convertedMat.step; - type = convertedMat.type(); if( (width & -2) != frame_width || (height & -2) != frame_height || !data ) return false; diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 47a15cc1b4..1f670e4b44 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -953,41 +953,37 @@ 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 ConversionTestParams; -typedef testing::TestWithParam< ConversionTestParams > videoio_ffmpeg_conversion; +typedef tuple MismatchTestParams; +typedef testing::TestWithParam< MismatchTestParams > videoio_ffmpeg_type_mismatch; -TEST_P(videoio_ffmpeg_conversion, handle_conversion) +TEST_P(videoio_ffmpeg_type_mismatch, type_mismatch) { if (!videoio_registry::hasBackend(CAP_FFMPEG)) throw SkipTestException("FFmpeg backend was not found"); - const string filename = "conversion_video.mp4"; + const string filename = "type_mismatch_video.mp4"; int input_type = get<0>(GetParam()); - int target_type = get<1>(GetParam()); - bool isColor = get<2>(GetParam()); + int expected_type = get<1>(GetParam()); + bool is_Color = get<2>(GetParam()); const string description = get<3>(GetParam()); const double fps = 30.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(), isColor); - writer.set(VIDEOWRITER_PROP_DEPTH, CV_MAT_DEPTH(target_type)); + VideoWriter writer(filename, fourcc, fps, frame.size(), {cv::VIDEOWRITER_PROP_DEPTH, CV_MAT_DEPTH(expected_type), VIDEOWRITER_PROP_IS_COLOR, is_Color}); ASSERT_TRUE(writer.isOpened()) << "Failed to open video writer for: " << description; for (int i = 1; i <= 90; i++) { - // Conversion happens internally - // as ffmpeg doesn't handle itself. - // If conversion fails, FFmpeg won't write any frames + // There's a mismatch between the given and expected types. Writer + // should produce a warning communicating it to the user. writer.write(frame); } writer.release(); - EXPECT_GT(getFileSize(filename), 0) << "Output file is empty for: " << description; - VideoCapture cap(filename, CAP_FFMPEG); if (!cap.isOpened()) throw SkipTestException("Failed to open video file, stream not supported or invalid"); @@ -997,28 +993,25 @@ TEST_P(videoio_ffmpeg_conversion, handle_conversion) while (cap.read(readFrame)) frameCount++; - // Checking if all 90 frames can be read is enough, if no conversion is done 0 frames will be written. - EXPECT_EQ(frameCount, 90) << "Not all frames were read for: " << description << "(expected 90, got " << frameCount << ")"; + // Since there's a mismatch, no frames should have been written. + EXPECT_EQ(frameCount, 0) << "Although there was a mismatch between given and expected types, " << frameCount << " frames were written for: " << description; std::remove(filename.c_str()); } -const ConversionTestParams conversion_cases[] = +const MismatchTestParams mismatch_cases[] = { - // converting to 8UC3 - make_tuple(CV_8UC1, CV_8UC3, true, "CV_8UC1_to_CV_8UC3"), - make_tuple(CV_16UC1, CV_8UC3, true, "CV_16UC1_to_CV_8UC3"), + // expected type CV_8UC3 + make_tuple(CV_16UC1, CV_8UC3, true, "input_CV_16UC1_expected_CV_8UC3"), - // converting to 8UC1 - make_tuple(CV_8UC3, CV_8UC1, false, "CV_8UC3_to_CV_8UC1"), - make_tuple(CV_16UC1, CV_8UC1, false, "CV_16UC1_to_CV_8UC1"), + // expected type 8UC1 + make_tuple(CV_8UC3, CV_8UC1, false, "input_CV_8UC3_expected_CV_8UC1"), - // converting to 16UC1 - make_tuple(CV_8UC3, CV_16UC1, false, "CV_8UC3_to_CV_16UC1"), - make_tuple(CV_8UC1, CV_16UC1, false, "CV_8UC1_to_CV_16UC1"), + // expected type 16UC1 + make_tuple(CV_8UC1, CV_16UC1, false, "input_CV_8UC1_expected_CV_16UC1"), }; -inline static std::string videoio_ffmpeg_conversion_name_printer(const testing::TestParamInfo& info) +inline static std::string videoio_ffmpeg_mismatch_name_printer(const testing::TestParamInfo& info) { std::ostringstream os; os << get<3>(info.param) << "_" @@ -1026,6 +1019,6 @@ inline static std::string videoio_ffmpeg_conversion_name_printer(const testing:: return os.str(); } -INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_conversion, testing::ValuesIn(conversion_cases), videoio_ffmpeg_conversion_name_printer); +INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_type_mismatch, testing::ValuesIn(mismatch_cases), videoio_ffmpeg_mismatch_name_printer); }} // namespace From 67bf5e604d4776d95ee68d1f4eebc2488c07d3e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Sun, 18 May 2025 16:11:53 +0100 Subject: [PATCH 07/10] fix #26276: simplify test case(s) Since no frames should be written, simply checking if video capture fails to open is enough. Also reduced the frames to be written from 90 to 30 --- modules/videoio/test/test_ffmpeg.cpp | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 1f670e4b44..a817978133 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -975,7 +975,7 @@ TEST_P(videoio_ffmpeg_type_mismatch, type_mismatch) ASSERT_TRUE(writer.isOpened()) << "Failed to open video writer for: " << description; - for (int i = 1; i <= 90; i++) + for (int i = 1; i <= 30; i++) { // There's a mismatch between the given and expected types. Writer // should produce a warning communicating it to the user. @@ -985,17 +985,8 @@ TEST_P(videoio_ffmpeg_type_mismatch, type_mismatch) writer.release(); VideoCapture cap(filename, CAP_FFMPEG); - if (!cap.isOpened()) - throw SkipTestException("Failed to open video file, stream not supported or invalid"); - - Mat readFrame; - int frameCount = 0; - while (cap.read(readFrame)) - frameCount++; - - // Since there's a mismatch, no frames should have been written. - EXPECT_EQ(frameCount, 0) << "Although there was a mismatch between given and expected types, " << frameCount << " frames were written for: " << description; - + ASSERT_FALSE(cap.isOpened()) << "Video capture should fail to open for: " << description + << " as no frames should have been written due to the existent mismatch"; std::remove(filename.c_str()); } From 159b5ce9d10e4e1acdaeb1d1da886a4400488ae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Sun, 18 May 2025 17:44:08 +0100 Subject: [PATCH 08/10] fix #26276: skip test if writer won't open Test cases are skipped when writer fails to open. Trailing whitespaces were also removed --- modules/videoio/test/test_ffmpeg.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index a817978133..4fc7743ad4 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -973,7 +973,8 @@ TEST_P(videoio_ffmpeg_type_mismatch, type_mismatch) VideoWriter writer(filename, fourcc, fps, frame.size(), {cv::VIDEOWRITER_PROP_DEPTH, CV_MAT_DEPTH(expected_type), VIDEOWRITER_PROP_IS_COLOR, is_Color}); - ASSERT_TRUE(writer.isOpened()) << "Failed to open video writer for: " << description; + if (!writer.isOpened()) + throw SkipTestException("Failed to open video writer"); for (int i = 1; i <= 30; i++) { @@ -985,7 +986,7 @@ TEST_P(videoio_ffmpeg_type_mismatch, type_mismatch) writer.release(); VideoCapture cap(filename, CAP_FFMPEG); - ASSERT_FALSE(cap.isOpened()) << "Video capture should fail to open for: " << description + ASSERT_FALSE(cap.isOpened()) << "Video capture should fail to open for: " << description << " as no frames should have been written due to the existent mismatch"; std::remove(filename.c_str()); } From 1a4bf9d1089c1a27b84df91df1c369b33987084e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Sat, 24 May 2025 01:56:05 +0100 Subject: [PATCH 09/10] fix #26276: changes upon review Revert the commit where parameter int cn (number of channels) was replaced with int type (e.g: CV_8UC1) to preserve external API compatibility. All realted logging and tests were updated accordingly. --- modules/videoio/src/cap_ffmpeg.cpp | 4 +- modules/videoio/src/cap_ffmpeg_impl.hpp | 32 +++++------- modules/videoio/test/test_ffmpeg.cpp | 67 ++++++++++++++++--------- 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/modules/videoio/src/cap_ffmpeg.cpp b/modules/videoio/src/cap_ffmpeg.cpp index b0c28eac44..cc62920c74 100644 --- a/modules/videoio/src/cap_ffmpeg.cpp +++ b/modules/videoio/src/cap_ffmpeg.cpp @@ -194,8 +194,8 @@ public: } } - if (!icvWriteFrame_FFMPEG_p(ffmpegWriter, (const uchar*)image.getMat().ptr(), (int)image.step(), image.cols(), image.rows(), image.type(), 0)) - CV_LOG_WARNING(NULL, "FFmpeg: Unable to write frame"); + 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 7ac704a377..9cd20fbf77 100644 --- a/modules/videoio/src/cap_ffmpeg_impl.hpp +++ b/modules/videoio/src/cap_ffmpeg_impl.hpp @@ -2139,7 +2139,7 @@ struct CvVideoWriter_FFMPEG bool open( const char* filename, int fourcc, double fps, int width, int height, const VideoWriterParameters& params ); void close(); - bool writeFrame( const unsigned char* data, int step, int width, int height, int type, int origin ); + bool writeFrame( const unsigned char* data, int step, int width, int height, int cn, int origin ); bool writeHWFrame(cv::InputArray input); double getProperty(int propId) const; bool setProperty(int, double); @@ -2512,10 +2512,10 @@ static int icv_av_write_frame_FFMPEG( AVFormatContext * oc, AVStream * video_st, } /// write a frame with FFMPEG -bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int width, int height, int type, int origin ) +bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int width, int height, int cn, int origin ) { if (!encode_video) { - CV_Assert(type == CV_8UC1 && ((width > 0 && height == 1) || (width == 1 && height > 0 && step == 1))); + CV_Assert(cn == 1 && ((width > 0 && height == 1) || (width == 1 && height > 0 && step == 1))); const bool set_key_frame = key_frame ? key_frame : idr_period ? frame_idx % idr_period == 0 : 1; bool ret = icv_av_encapsulate_video_FFMPEG(oc, video_st, context, (uint8_t*)data, width, frame_idx, pts_index, b_frame_dts_delay, set_key_frame); frame_idx++; @@ -2524,27 +2524,21 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int // check parameters if (input_pix_fmt == AV_PIX_FMT_BGR24) { - if (type != CV_8UC3) { - CV_LOG_WARNING(NULL, "write frame skipped - expected frame with depth of CV_8UC3"); + 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) { - if (type != CV_8UC1) { - CV_LOG_WARNING(NULL, "write frame skipped - expected frame with depth of CV_8UC1"); - return false; - } - } - else if (input_pix_fmt == AV_PIX_FMT_GRAY16LE) { - if (type != CV_16UC1) { - CV_LOG_WARNING(NULL, "write frame skipped - expected frame with depth of CV_16UC1"); + 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; } } else { CV_LOG_WARNING(NULL, "Input data does not match selected pixel format: " << av_get_pix_fmt_name(input_pix_fmt) - << ", only CV_8UC1, CV_8UC3, and CV_16UC1 are supported"); + << ", number of channels: " << cn); CV_Assert(false); } @@ -2648,7 +2642,7 @@ 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); - // AVERROR(EAGAIN) means the encoder needs more input, not an error + // AVERROR(EAGAIN): continue sending input, not an error ret = (ret_write >= 0 || ret_write == AVERROR(EAGAIN)); av_frame_free(&hw_frame); } else @@ -2656,7 +2650,7 @@ bool CvVideoWriter_FFMPEG::writeFrame( const unsigned char* data, int step, int { picture->pts = frame_idx; int ret_write = icv_av_write_frame_FFMPEG(oc, video_st, context, outbuf, outbuf_size, picture, frame_idx); - // AVERROR(EAGAIN) means the encoder needs more input, not an error + // AVERROR(EAGAIN): continue sending input, not an error ret = (ret_write >= 0 || ret_write == AVERROR(EAGAIN)); } @@ -3466,7 +3460,7 @@ void cvReleaseVideoWriter_FFMPEG( CvVideoWriter_FFMPEG** writer ) int cvWriteFrame_FFMPEG( CvVideoWriter_FFMPEG* writer, const unsigned char* data, int step, - int width, int height, int type, int origin) + int width, int height, int cn, int origin) { - return writer->writeFrame(data, step, width, height, type, origin); + return writer->writeFrame(data, step, width, height, cn, origin); } diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 4fc7743ad4..59f41784dd 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -953,64 +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 MismatchTestParams; -typedef testing::TestWithParam< MismatchTestParams > videoio_ffmpeg_type_mismatch; +typedef tuple ChannelMismatchTestParams; +typedef testing::TestWithParam< ChannelMismatchTestParams > videoio_ffmpeg_channel_mismatch; -TEST_P(videoio_ffmpeg_type_mismatch, type_mismatch) +TEST_P(videoio_ffmpeg_channel_mismatch, basic) { if (!videoio_registry::hasBackend(CAP_FFMPEG)) throw SkipTestException("FFmpeg backend was not found"); - const string filename = "type_mismatch_video.mp4"; + const string filename = "mismatch_video.mp4"; int input_type = get<0>(GetParam()); - int expected_type = get<1>(GetParam()); + int depth = get<1>(GetParam()); bool is_Color = get<2>(GetParam()); - const string description = get<3>(GetParam()); + bool is_valid = get<3>(GetParam()); + const string description = get<4>(GetParam()); - const double fps = 30.0; + 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, CV_MAT_DEPTH(expected_type), VIDEOWRITER_PROP_IS_COLOR, is_Color}); + 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 <= 30; i++) + for (int i = 1; i <= 15; i++) { - // There's a mismatch between the given and expected types. Writer - // should produce a warning communicating it to the user. + // 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); - ASSERT_FALSE(cap.isOpened()) << "Video capture should fail to open for: " << description - << " as no frames should have been written due to the existent mismatch"; + VideoCapture cap(filename, CAP_FFMPEG); + + if (is_valid) { + ASSERT_TRUE(cap.isOpened()) << "Can't open written video " << description; + EXPECT_EQ(cap.get(CAP_PROP_FRAME_COUNT), 15) << "Video capture should be able to read all frames for: " << description; + } else { + ASSERT_FALSE(cap.isOpened()) << "Video capture should not be able to read any frames for: " << description; + } + std::remove(filename.c_str()); } -const MismatchTestParams mismatch_cases[] = +const ChannelMismatchTestParams mismatch_cases[] = { - // expected type CV_8UC3 - make_tuple(CV_16UC1, CV_8UC3, true, "input_CV_16UC1_expected_CV_8UC3"), + // Testing input frame channels and expected depth/isColor combinations - // expected type 8UC1 - make_tuple(CV_8UC3, CV_8UC1, false, "input_CV_8UC3_expected_CV_8UC1"), + // 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"), - // expected type 16UC1 - make_tuple(CV_8UC1, CV_16UC1, false, "input_CV_8UC1_expected_CV_16UC1"), + // 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) +inline static std::string videoio_ffmpeg_mismatch_name_printer(const testing::TestParamInfo& info) { std::ostringstream os; - os << get<3>(info.param) << "_" - << "isColor_" << (get<2>(info.param) ? "true" : "false"); + os << get<4>(info.param); return os.str(); } -INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_type_mismatch, testing::ValuesIn(mismatch_cases), videoio_ffmpeg_mismatch_name_printer); +INSTANTIATE_TEST_CASE_P(/**/, videoio_ffmpeg_channel_mismatch, testing::ValuesIn(mismatch_cases), videoio_ffmpeg_mismatch_name_printer); }} // namespace From 2f7011ca6e53ea7edeae4daa8f7b7345be009f69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20M=C3=B3nica?= Date: Sat, 24 May 2025 02:53:37 +0100 Subject: [PATCH 10/10] fix opencv#26276: removes trailing whitespaces --- modules/videoio/test/test_ffmpeg.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/videoio/test/test_ffmpeg.cpp b/modules/videoio/test/test_ffmpeg.cpp index 59f41784dd..eeb0835078 100644 --- a/modules/videoio/test/test_ffmpeg.cpp +++ b/modules/videoio/test/test_ffmpeg.cpp @@ -990,13 +990,13 @@ TEST_P(videoio_ffmpeg_channel_mismatch, basic) writer.release(); - VideoCapture cap(filename, CAP_FFMPEG); + VideoCapture cap(filename, CAP_FFMPEG); if (is_valid) { - ASSERT_TRUE(cap.isOpened()) << "Can't open written video " << description; - EXPECT_EQ(cap.get(CAP_PROP_FRAME_COUNT), 15) << "Video capture should be able to read all frames for: " << description; + 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 not be able to read any frames for: " << description; + ASSERT_FALSE(cap.isOpened()) << "Video capture should fail to open for: " << description; } std::remove(filename.c_str());