From f539c0907e2be34bea74b209b12768815c6fa0a2 Mon Sep 17 00:00:00 2001 From: Abhishek Gola Date: Mon, 25 May 2026 00:40:00 +0530 Subject: [PATCH] Merge pull request #29087 from abhishek-gola:videoio_write_issue_fix Fixed write return status in videoio module#29087 closes: https://github.com/opencv/opencv/issues/24287 ### 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 - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake --- modules/videoio/include/opencv2/videoio.hpp | 7 ++++++- modules/videoio/src/backend_plugin.cpp | 5 +++-- .../src/backend_plugin_legacy.impl.hpp | 5 +++-- modules/videoio/src/cap.cpp | 5 +++-- modules/videoio/src/cap_android_mediandk.cpp | 14 +++++++++---- modules/videoio/src/cap_avfoundation.mm | 17 ++++++++++------ modules/videoio/src/cap_avfoundation_mac.mm | 9 +++++---- modules/videoio/src/cap_ffmpeg.cpp | 10 +++++++--- modules/videoio/src/cap_gstreamer.cpp | 20 ++++++++++--------- modules/videoio/src/cap_images.cpp | 7 ++++--- modules/videoio/src/cap_interface.hpp | 2 +- modules/videoio/src/cap_mfx_writer.cpp | 4 ++-- modules/videoio/src/cap_mfx_writer.hpp | 2 +- modules/videoio/src/cap_mjpeg_encoder.cpp | 3 ++- modules/videoio/src/cap_msmf.cpp | 8 +++++--- modules/videoio/test/test_images.cpp | 19 ++++++++++++++++++ 16 files changed, 93 insertions(+), 44 deletions(-) diff --git a/modules/videoio/include/opencv2/videoio.hpp b/modules/videoio/include/opencv2/videoio.hpp index 572237c98c..752699181a 100644 --- a/modules/videoio/include/opencv2/videoio.hpp +++ b/modules/videoio/include/opencv2/videoio.hpp @@ -1190,8 +1190,13 @@ public: The function/method writes the specified image to video file. It must have the same size as has been specified when opening the video writer. + + @return `true` if the frame was written successfully by the underlying backend, + `false` otherwise (for example, on network errors when streaming, encoder failures, + or unsupported input frames). Backends that do not surface per-frame status from + their native API report `true` on best-effort success. */ - CV_WRAP virtual void write(InputArray image); + CV_WRAP virtual bool write(InputArray image); /** @brief Sets a property in the VideoWriter. diff --git a/modules/videoio/src/backend_plugin.cpp b/modules/videoio/src/backend_plugin.cpp index 500154d238..e714ac5903 100644 --- a/modules/videoio/src/backend_plugin.cpp +++ b/modules/videoio/src/backend_plugin.cpp @@ -681,7 +681,7 @@ public: { return writer_ != NULL; // TODO always true } - void write(cv::InputArray arr) CV_OVERRIDE + bool write(cv::InputArray arr) CV_OVERRIDE { cv::Mat img = arr.getMat(); CV_DbgAssert(writer_); @@ -689,8 +689,9 @@ public: if (CV_ERROR_OK != plugin_api_->v0.Writer_write(writer_, img.data, (int)img.step[0], img.cols, img.rows, img.channels())) { CV_LOG_DEBUG(NULL, "Video I/O: Can't write frame by plugin '" << plugin_api_->api_header.api_description << "'"); + return false; } - // TODO return bool result? + return true; } int getCaptureDomain() const CV_OVERRIDE { diff --git a/modules/videoio/src/backend_plugin_legacy.impl.hpp b/modules/videoio/src/backend_plugin_legacy.impl.hpp index fa9def4cc5..3848d38c33 100644 --- a/modules/videoio/src/backend_plugin_legacy.impl.hpp +++ b/modules/videoio/src/backend_plugin_legacy.impl.hpp @@ -178,7 +178,7 @@ public: { return writer_ != NULL; // TODO always true } - void write(cv::InputArray arr) CV_OVERRIDE + bool write(cv::InputArray arr) CV_OVERRIDE { cv::Mat img = arr.getMat(); CV_DbgAssert(writer_); @@ -186,8 +186,9 @@ public: if (CV_ERROR_OK != plugin_api_->v0.Writer_write(writer_, img.data, (int)img.step[0], img.cols, img.rows, img.channels())) { CV_LOG_DEBUG(NULL, "Video I/O: Can't write frame by plugin '" << plugin_api_->api_header.api_description << "'"); + return false; } - // TODO return bool result? + return true; } int getCaptureDomain() const CV_OVERRIDE { diff --git a/modules/videoio/src/cap.cpp b/modules/videoio/src/cap.cpp index 9118ac8cdb..4c2a6ce1c6 100644 --- a/modules/videoio/src/cap.cpp +++ b/modules/videoio/src/cap.cpp @@ -862,14 +862,15 @@ String VideoWriter::getBackendName() const return cv::videoio_registry::getBackendName(static_cast(api)); } -void VideoWriter::write(InputArray image) +bool VideoWriter::write(InputArray image) { CV_INSTRUMENT_REGION(); if (iwriter) { - iwriter->write(image); + return iwriter->write(image); } + return false; } VideoWriter& VideoWriter::operator << (const Mat& image) diff --git a/modules/videoio/src/cap_android_mediandk.cpp b/modules/videoio/src/cap_android_mediandk.cpp index 6dfb79843a..18b4a26614 100644 --- a/modules/videoio/src/cap_android_mediandk.cpp +++ b/modules/videoio/src/cap_android_mediandk.cpp @@ -543,11 +543,11 @@ public: virtual int getCaptureDomain() const CV_OVERRIDE { return cv::CAP_ANDROID; } - virtual void write(cv::InputArray image_ ) CV_OVERRIDE + virtual bool write(cv::InputArray image_ ) CV_OVERRIDE { if (!image_.isMat()) { LOGE("Support only Mat input"); - return; + return false; } Mat image = image_.getMat(); @@ -559,16 +559,17 @@ public: width, height, CV_8UC1, CV_8UC3, CV_8UC4, image.cols, image.rows, image.type() ); - return; + return false; } #if __ANDROID_API__ >= 26 ANativeWindow_Buffer buffer; if (0 != ANativeWindow_lock(surface, &buffer, NULL)) { LOGE("Failed to lock the surface"); - return; + return false; } + bool format_supported = true; if (AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM == buffer.format) { Mat bufferMat(image.rows, image.cols, CV_8UC4, buffer.bits, buffer.stride * 4); switch (image.type()) { @@ -578,9 +579,13 @@ public: } } else { LOGE("Unknown surface buffer format: 0x%x", buffer.format); + format_supported = false; } ANativeWindow_unlockAndPost(surface); + if (!format_supported) { + return false; + } #else //OpenCV doesn't support RGB to NV12 so we need to convert to YV12 and then manually changed it to NV12 Mat imageYV12; @@ -612,6 +617,7 @@ public: drainEncoder(false); frameIndex++; + return true; } virtual bool open( const cv::String& filename, int fourcc, double fps, cv::Size frameSize, const VideoWriterParameters& params ) diff --git a/modules/videoio/src/cap_avfoundation.mm b/modules/videoio/src/cap_avfoundation.mm index aa7ee57142..1437ebe95f 100644 --- a/modules/videoio/src/cap_avfoundation.mm +++ b/modules/videoio/src/cap_avfoundation.mm @@ -37,9 +37,14 @@ #include "cap_interface.hpp" #include #include +#include #import #import +#ifndef TARGET_OS_VISION +#define TARGET_OS_VISION 0 +#endif + #define CV_CAP_MODE_BGR CV_FOURCC_MACRO('B','G','R','3') #define CV_CAP_MODE_RGB CV_FOURCC_MACRO('R','G','B','3') #define CV_CAP_MODE_GRAY CV_FOURCC_MACRO('G','R','E','Y') @@ -185,7 +190,7 @@ class CvVideoWriter_AVFoundation : public cv::IVideoWriter{ int is_color=1); ~CvVideoWriter_AVFoundation(); bool isOpened() const CV_OVERRIDE { return mMovieWriter != NULL && mMovieWriter.status != AVAssetWriterStatusFailed; } - void write(cv::InputArray image) CV_OVERRIDE; + bool write(cv::InputArray image) CV_OVERRIDE; int getCaptureDomain() const CV_OVERRIDE { return cv::CAP_AVFOUNDATION; } private: cv::Mat argbimage; @@ -1293,20 +1298,20 @@ CvVideoWriter_AVFoundation::~CvVideoWriter_AVFoundation() { } } -void CvVideoWriter_AVFoundation::write(cv::InputArray image) { +bool CvVideoWriter_AVFoundation::write(cv::InputArray image) { @autoreleasepool { // writer status check if (![mMovieWriterInput isReadyForMoreMediaData] || mMovieWriter.status != AVAssetWriterStatusWriting) { NSLog(@"[mMovieWriterInput isReadyForMoreMediaData] Not ready for media data or ..."); NSLog(@"mMovieWriter.status: %d. Error: %@", (int)mMovieWriter.status, [mMovieWriter.error localizedDescription]); - return; + return false; } BOOL success = FALSE; if (image.size().height != movieSize.height || image.size().width != movieSize.width) { std::cout << "Frame size does not match video size." << std::endl; - return; + return false; } if (movieColor) { @@ -1351,10 +1356,10 @@ void CvVideoWriter_AVFoundation::write(cv::InputArray image) { if (success) { frameCount++; - return; + return true; } else { NSLog(@"Frame appendPixelBuffer failed."); - return; + return false; } } } diff --git a/modules/videoio/src/cap_avfoundation_mac.mm b/modules/videoio/src/cap_avfoundation_mac.mm index 3bcd0928de..aed1ddc743 100644 --- a/modules/videoio/src/cap_avfoundation_mac.mm +++ b/modules/videoio/src/cap_avfoundation_mac.mm @@ -184,7 +184,7 @@ class CvVideoWriter_AVFoundation : public cv::IVideoWriter { public: CvVideoWriter_AVFoundation(const std::string &filename, int fourcc, double fps, const cv::Size& frame_size, int is_color); ~CvVideoWriter_AVFoundation(); - void write(cv::InputArray image) CV_OVERRIDE; + bool write(cv::InputArray image) CV_OVERRIDE; int getCaptureDomain() const CV_OVERRIDE { return cv::CAP_AVFOUNDATION; } bool isOpened() const CV_OVERRIDE { @@ -1233,14 +1233,14 @@ static void releaseCallback( void *releaseRefCon, const void * ) { CFRelease((CFDataRef)releaseRefCon); } -void CvVideoWriter_AVFoundation::write(cv::InputArray image) { +bool CvVideoWriter_AVFoundation::write(cv::InputArray image) { NSAutoreleasePool* localpool = [[NSAutoreleasePool alloc] init]; // writer status check if (mMovieWriter.status != AVAssetWriterStatusWriting ) { NSLog(@"mMovieWriter.status: %d. Error: %@", (int)mMovieWriter.status, [mMovieWriter.error localizedDescription]); [localpool drain]; - return; + return false; } // Make writeFrame() a blocking call. @@ -1255,7 +1255,7 @@ void CvVideoWriter_AVFoundation::write(cv::InputArray image) { if (image.size().height!=movieSize.height || image.size().width!=movieSize.width){ fprintf(stderr, "OpenCV: Frame size does not match video size.\n"); [localpool drain]; - return; + return false; } if (movieColor) { @@ -1305,6 +1305,7 @@ void CvVideoWriter_AVFoundation::write(cv::InputArray image) { NSLog(@"Frame appendPixelBuffer failed."); } + return success; } #pragma clang diagnostic pop diff --git a/modules/videoio/src/cap_ffmpeg.cpp b/modules/videoio/src/cap_ffmpeg.cpp index c38a6289f3..ab7b544c39 100644 --- a/modules/videoio/src/cap_ffmpeg.cpp +++ b/modules/videoio/src/cap_ffmpeg.cpp @@ -201,21 +201,25 @@ public: int getCaptureDomain() const CV_OVERRIDE { return cv::CAP_FFMPEG; } - virtual void write(cv::InputArray image ) CV_OVERRIDE + virtual bool write(cv::InputArray image ) CV_OVERRIDE { if(!ffmpegWriter) - return; + return false; CV_Assert(image.depth() == CV_8U || image.depth() == CV_16U); // if UMat, try GPU to GPU copy using OpenCL extensions if (image.isUMat()) { if (ffmpegWriter->writeHWFrame(image)) { - return; + return true; } } 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"); + return false; + } + return true; } virtual bool open( const cv::String& filename, int fourcc, double fps, cv::Size frameSize, const VideoWriterParameters& params ) { diff --git a/modules/videoio/src/cap_gstreamer.cpp b/modules/videoio/src/cap_gstreamer.cpp index 042cce3c83..f2b753fbf4 100644 --- a/modules/videoio/src/cap_gstreamer.cpp +++ b/modules/videoio/src/cap_gstreamer.cpp @@ -2211,7 +2211,7 @@ public: bool open(const std::string &filename, int fourcc, double fps, const Size &frameSize, const VideoWriterParameters& params ); void close(); - void write(InputArray) CV_OVERRIDE; + bool write(InputArray) CV_OVERRIDE; int getIplDepth() const { return ipl_depth; } @@ -2683,12 +2683,13 @@ bool CvVideoWriter_GStreamer::open( const std::string &filename, int fourcc, /*! * \brief CvVideoWriter_GStreamer::writeFrame * \param image - * \return + * \return true on success, false on failure (rejected frame format or + * a non-OK GstFlowReturn from the appsrc push). * Pushes the given frame on the pipeline. * The timestamp for the buffer is generated from the framerate set in open * and ensures a smooth video */ -void CvVideoWriter_GStreamer::write(InputArray image) +bool CvVideoWriter_GStreamer::write(InputArray image) { GstClockTime duration, timestamp; GstFlowReturn ret; @@ -2698,31 +2699,31 @@ void CvVideoWriter_GStreamer::write(InputArray image) if (input_pix_fmt == GST_VIDEO_FORMAT_ENCODED) { if (image.type() != CV_8UC1 || image.size().height != 1) { CV_WARN("write frame skipped - expected CV_8UC1, height==1"); - return; + return false; } } else if(input_pix_fmt == GST_VIDEO_FORMAT_BGR) { if (image.type() != CV_8UC3) { CV_WARN("write frame skipped - expected CV_8UC3"); - return; + return false; } } else if (input_pix_fmt == GST_VIDEO_FORMAT_GRAY8) { if (image.type() != CV_8UC1) { CV_WARN("write frame skipped - expected CV_8UC1"); - return; + return false; } } else if (input_pix_fmt == GST_VIDEO_FORMAT_GRAY16_LE) { if (image.type() != CV_16UC1) { CV_WARN("write frame skipped - expected CV_16UC1"); - return; + return false; } } else { CV_WARN("write frame skipped - unsupported format"); - return; + return false; } Mat imageMat = image.getMat(); @@ -2747,12 +2748,13 @@ void CvVideoWriter_GStreamer::write(InputArray image) if (ret != GST_FLOW_OK) { CV_WARN("Error pushing buffer to GStreamer pipeline"); - return; + return false; } //GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline"); ++num_frames; + return true; } diff --git a/modules/videoio/src/cap_images.cpp b/modules/videoio/src/cap_images.cpp index 99de13889e..290099dfde 100644 --- a/modules/videoio/src/cap_images.cpp +++ b/modules/videoio/src/cap_images.cpp @@ -386,7 +386,7 @@ public: double getProperty(int) const CV_OVERRIDE { return 0; } bool setProperty( int, double ) CV_OVERRIDE; // FIXIT doesn't work: IVideoWriter interface only! bool isOpened() const CV_OVERRIDE { return !filename_pattern.empty(); } - void write( InputArray ) CV_OVERRIDE; + bool write( InputArray ) CV_OVERRIDE; int getCaptureDomain() const CV_OVERRIDE { return cv::CAP_IMAGES; } protected: std::string filename_pattern; @@ -394,15 +394,16 @@ protected: std::vector params; }; -void CvVideoWriter_Images::write(InputArray image) +bool CvVideoWriter_Images::write(InputArray image) { CV_Assert(!filename_pattern.empty()); cv::String filename = cv::format(filename_pattern.c_str(), (int)currentframe); CV_Assert(!filename.empty()); cv::Mat img = image.getMat(); - cv::imwrite(filename, img); + const bool ok = cv::imwrite(filename, img); currentframe++; + return ok; } void CvVideoWriter_Images::close() diff --git a/modules/videoio/src/cap_interface.hpp b/modules/videoio/src/cap_interface.hpp index 0d154d4779..4c9eb3e604 100644 --- a/modules/videoio/src/cap_interface.hpp +++ b/modules/videoio/src/cap_interface.hpp @@ -205,7 +205,7 @@ public: virtual double getProperty(int) const { return 0; } virtual bool setProperty(int, double) { return false; } virtual bool isOpened() const = 0; - virtual void write(InputArray) = 0; + virtual bool write(InputArray) = 0; virtual int getCaptureDomain() const { return cv::CAP_ANY; } // Return the type of the capture object: CAP_FFMPEG, etc... }; diff --git a/modules/videoio/src/cap_mfx_writer.cpp b/modules/videoio/src/cap_mfx_writer.cpp index 3006c396bd..31b8341f89 100644 --- a/modules/videoio/src/cap_mfx_writer.cpp +++ b/modules/videoio/src/cap_mfx_writer.cpp @@ -206,9 +206,9 @@ bool VideoWriter_IntelMFX::isOpened() const return good; } -void VideoWriter_IntelMFX::write(cv::InputArray input) +bool VideoWriter_IntelMFX::write(cv::InputArray input) { - write_one(input); + return write_one(input); } bool VideoWriter_IntelMFX::write_one(cv::InputArray bgr) diff --git a/modules/videoio/src/cap_mfx_writer.hpp b/modules/videoio/src/cap_mfx_writer.hpp index fff2a6387f..f78556caff 100644 --- a/modules/videoio/src/cap_mfx_writer.hpp +++ b/modules/videoio/src/cap_mfx_writer.hpp @@ -23,7 +23,7 @@ public: double getProperty(int) const CV_OVERRIDE; bool setProperty(int, double) CV_OVERRIDE; bool isOpened() const CV_OVERRIDE; - void write(cv::InputArray input) CV_OVERRIDE; + bool write(cv::InputArray input) CV_OVERRIDE; int getCaptureDomain() const CV_OVERRIDE { return cv::CAP_INTEL_MFX; } protected: bool write_one(cv::InputArray bgr); diff --git a/modules/videoio/src/cap_mjpeg_encoder.cpp b/modules/videoio/src/cap_mjpeg_encoder.cpp index 2bb01f4a81..41cbcb6c83 100644 --- a/modules/videoio/src/cap_mjpeg_encoder.cpp +++ b/modules/videoio/src/cap_mjpeg_encoder.cpp @@ -453,7 +453,7 @@ public: bool isOpened() const CV_OVERRIDE { return container.isOpenedStream(); } - void write(InputArray _img) CV_OVERRIDE + bool write(InputArray _img) CV_OVERRIDE { Mat img = _img.getMat(); size_t chunkPointer = container.getStreamPos(); @@ -504,6 +504,7 @@ public: container.pushFrameSize(tempChunkPointer - chunkPointer - 8); // Size excludes '00dc' and size field container.endWriteChunk(); // end '00dc' } + return true; } double getProperty(int propId) const CV_OVERRIDE diff --git a/modules/videoio/src/cap_msmf.cpp b/modules/videoio/src/cap_msmf.cpp index a79cb19716..1e1e0b3607 100644 --- a/modules/videoio/src/cap_msmf.cpp +++ b/modules/videoio/src/cap_msmf.cpp @@ -2483,7 +2483,7 @@ public: virtual bool open(const cv::String& filename, int fourcc, double fps, cv::Size frameSize, const cv::VideoWriterParameters& params); virtual void close(); - virtual void write(cv::InputArray); + virtual bool write(cv::InputArray); virtual double getProperty(int) const override; virtual bool setProperty(int, double) { return false; } @@ -2714,12 +2714,12 @@ void CvVideoWriter_MSMF::close() } } -void CvVideoWriter_MSMF::write(cv::InputArray img) +bool CvVideoWriter_MSMF::write(cv::InputArray img) { if (img.empty() || (img.channels() != 1 && img.channels() != 3 && img.channels() != 4) || (UINT32)img.cols() != videoWidth || (UINT32)img.rows() != videoHeight) - return; + return false; const LONG cbWidth = 4 * videoWidth; const DWORD cbBuffer = cbWidth * videoHeight; @@ -2747,8 +2747,10 @@ void CvVideoWriter_MSMF::write(cv::InputArray img) if (SUCCEEDED(sinkWriter->WriteSample(streamIndex, sample.Get()))) { rtStart += rtDuration; + return true; } } + return false; } diff --git a/modules/videoio/test/test_images.cpp b/modules/videoio/test/test_images.cpp index 255917950e..6f671341a4 100644 --- a/modules/videoio/test/test_images.cpp +++ b/modules/videoio/test/test_images.cpp @@ -137,6 +137,25 @@ TEST(videoio_images, bad) } } +TEST(videoio_images, write_returns_status) +{ + ImageCollection col; + col.generate(1); + VideoWriter wri(col.getFirstFilename(), CAP_IMAGES, 0, 0, col.getFrame(0).size()); + ASSERT_TRUE(wri.isOpened()); + + const Mat frame = col.getFrame(0); + + EXPECT_TRUE(wri.write(frame)); + wri.release(); + ASSERT_FALSE(wri.isOpened()); + EXPECT_FALSE(wri.write(frame)); + + VideoWriter empty; + EXPECT_FALSE(empty.isOpened()); + EXPECT_FALSE(empty.write(frame)); +} + TEST(videoio_images, seek) { // check files: test0005.png, ..., test0024.png