Add `QImage` and `CV::Mat` conversion functions into `Frame` for better handling alpha channel handling #998

aperture147 · 2025-03-21T03:32:50Z

This pull request introduces enhanced image effect capabilities for the Frame class by leveraging the OpenCV library. Direct manipulation of QImage raw data for complex effects proves challenging, and the performance of QGraphicsEffect can be suboptimal. To address these limitations, this PR proposes the integration of OpenCV for more efficient and versatile effect creation.

While existing conversion functions between QImage and cv::Mat in Frame.cpp lack alpha channel support, several effects, including the currently implemented Outline and the proposed Shadow, require it for proper rendering. This pull request introduces four new methods to facilitate seamless conversion between QImage and cv::Mat while preserving the alpha channel:

QImage2BGRACvMat: Converts a QImage to a cv::Mat with BGRA pixel format.
BGRACvMat2QImage: Converts a cv::Mat with BGRA pixel format back to a QImage.
GetBGRACvMat: Provides access to the underlying cv::Mat (in BGRA format) of a Frame object.
SetBGRACvMat: Allows setting the cv::Mat (in BGRA format) to a Frame object.

These conversion functions were previously located within the Outline effect code and have been migrated to the Frame class for broader reusability. Your review and feedback on these additions would be greatly appreciated.

Note: The imagecv variable, which appears to be a potentially unused cached cv::Mat, is currently shared across the code. While investigating its purpose, no clear or active usage was identified. The line intended for caching was also commented out. This aspect might warrant further review or potential removal in a subsequent cleanup.

Now switched to brga_image_cv variable.

…r related code

…ss for better encapsulation

…class

… for alpha support

codecov · 2025-03-22T09:45:24Z

Codecov Report

❌ Patch coverage is 87.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.68%. Comparing base (47b3081) to head (02637fc).

Files with missing lines	Patch %	Lines
src/Frame.cpp	80.00%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #998      +/-   ##
===========================================
+ Coverage    59.60%   59.68%   +0.08%     
===========================================
  Files          206      206              
  Lines        20374    20397      +23     
===========================================
+ Hits         12143    12174      +31     
+ Misses        8231     8223       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This pull request adds alpha channel support to Frame's OpenCV conversion functions by introducing four new methods: QImage2BGRACvMat, BGRACvMat2QImage, GetBGRACvMat, and SetBGRACvMat. These functions are migrated from the Outline effect to make them reusable across multiple effects that require alpha channel handling.

Key changes:

Added new member variable brga_image_cv and four conversion methods to Frame class for BGRA format with alpha channel
Refactored Outline effect to use the new Frame methods instead of local conversion functions
Added unit test to verify 4-channel cv::Mat conversion

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/Frame.h	Declares new `brga_image_cv` member variable and four new methods for QImage ↔ cv::Mat conversion with alpha channel support
src/Frame.cpp	Implements the conversion methods and adds cleanup for the new member variable in destructor
src/effects/Outline.h	Removes local conversion method declarations, now using Frame class methods
src/effects/Outline.cpp	Refactored to use `Frame::GetBGRACvMat()` and `Frame::SetBGRACvMat()` instead of local conversion functions
tests/Frame.cpp	Adds test case verifying GetBGRACvMat returns a 4-channel cv::Mat

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T08:04:30Z

src/Frame.cpp

+	if (!image)
+		// Fill with black
+		AddColor(width, height, color);
+	brga_image_cv = QImage2BGRACvMat(image);


Storing the result in brga_image_cv member variable is problematic because the cv::Mat shares memory with the QImage (no deep copy). If the QImage changes, brga_image_cv becomes invalid. This is different from imagecv which is a deep copy. Consider either: (1) not storing it as a member variable and returning a new cv::Mat each time, or (2) making it a deep copy with .clone() if caching is desired. The current approach creates a dangling reference risk.

Suggested change

brga_image_cv = QImage2BGRACvMat(image);

// Create a cv::Mat that references the QImage, then store a deep copy

brga_image_cv = QImage2BGRACvMat(image).clone();

Copilot · 2025-12-23T08:04:30Z

src/Frame.cpp

+std::shared_ptr<QImage> Frame::BGRACvMat2QImage(cv::Mat img) {
+	cv::Mat final_img;
+	cv::cvtColor(img, final_img, cv::COLOR_BGRA2RGBA);
+	QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_ARGB32);


After converting the cv::Mat to RGBA format using cv::COLOR_BGRA2RGBA, the code creates a QImage with QImage::Format_ARGB32. However, Format_ARGB32 expects ARGB in native endianness (which is BGRA in memory on little-endian systems), but the data is in RGBA format. This creates a format mismatch that requires an additional costly conversion on line 935. Consider creating the QImage directly with QImage::Format_RGBA8888 to match the actual data layout and avoid the unnecessary conversion.

Suggested change

QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_ARGB32);

QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_RGBA8888);

Copilot · 2025-12-23T08:04:30Z

src/Frame.cpp

+	cv::Mat cv_img(
+		qimage->height(), qimage->width(),
+		CV_8UC4, (uchar*)qimage->constBits(),
+		qimage->bytesPerLine()
+	);
+	return cv_img;


The function name suggests it returns a cv::Mat in BGRA format, but it actually wraps the QImage data without any color conversion. Since QImages in this codebase use Format_RGBA8888_Premultiplied (RGBA in memory), the returned cv::Mat contains RGBA data, not BGRA. This is inconsistent with the existing Qimage2mat function (line 873) which converts RGB to BGR. OpenCV operations typically expect BGR/BGRA format. The function should either convert RGBA to BGRA, or be renamed to accurately reflect that it returns RGBA data.

Suggested change

cv::Mat cv_img(

qimage->height(), qimage->width(),

CV_8UC4, (uchar*)qimage->constBits(),

qimage->bytesPerLine()

);

return cv_img;

// QImage::Format_RGBA8888_Premultiplied stores data in RGBA order.

// First, wrap the existing bits as an RGBA cv::Mat (no copy).

cv::Mat rgba_img(

qimage->height(), qimage->width(),

CV_8UC4, const_cast<uchar*>(qimage->constBits()),

qimage->bytesPerLine()

);

// Then convert channel order from RGBA to BGRA as expected by OpenCV.

cv::Mat bgra_img;

cv::cvtColor(rgba_img, bgra_img, cv::COLOR_RGBA2BGRA);

return bgra_img;

Copilot · 2025-12-23T08:04:31Z

src/Frame.h


 #ifdef USE_OPENCV
 		cv::Mat imagecv; ///< OpenCV image. It will always be in BGR format
+		cv::Mat brga_image_cv; ///< OpenCV image. It will always be in BGR format


The variable name has a typo: brga_image_cv should be bgra_image_cv. BGRA is the correct order: Blue, Green, Red, Alpha.

Suggested change

cv::Mat brga_image_cv; ///< OpenCV image. It will always be in BGR format

cv::Mat bgra_image_cv; ///< OpenCV image. It will always be in BGR format

Copilot · 2025-12-23T08:04:31Z

tests/Frame.cpp

+TEST_CASE( "Convert_Image_Alpha", "[libopenshot][opencv][frame]" )
+{
+	// Create a video clip
+	std::stringstream path;
+	path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4";
+	Clip c1(path.str());
+	c1.Open();
+
+	// Get first frame
+	auto f1 = c1.GetFrame(1);
+
+	// Get first Mat image
+	cv::Mat cvimage = f1->GetBGRACvMat();
+
+	CHECK_FALSE(cvimage.empty());
+
+	CHECK(f1->number == 1);
+	CHECK(f1->GetWidth() == cvimage.cols);
+	CHECK(f1->GetHeight() == cvimage.rows);
+	CHECK(cvimage.channels() == 4);
+}


The test only verifies that GetBGRACvMat returns a 4-channel cv::Mat but doesn't validate that the color channels are in the correct order or that the color values are accurate. Consider adding assertions that check specific pixel values or perform a round-trip conversion test (QImage → cv::Mat → QImage) to ensure data integrity and correct channel ordering.

Copilot · 2025-12-23T08:04:31Z

src/Frame.h


 #ifdef USE_OPENCV
 		cv::Mat imagecv; ///< OpenCV image. It will always be in BGR format
+		cv::Mat brga_image_cv; ///< OpenCV image. It will always be in BGR format


The comment incorrectly states "BGR format" but this variable stores images in BGRA format (4 channels including alpha). Update the comment to accurately reflect this.

Suggested change

cv::Mat brga_image_cv; ///< OpenCV image. It will always be in BGR format

cv::Mat brga_image_cv; ///< OpenCV image. It will always be in BGRA format (with alpha channel)

Copilot · 2025-12-23T08:04:32Z

src/Frame.cpp

 	audio.reset();
 	#ifdef USE_OPENCV
 	imagecv.release();
+	brga_image_cv.release();


The variable name has a typo: brga_image_cv should be bgra_image_cv to correctly represent the BGRA color format.

Suggested change

brga_image_cv.release();

bgra_image_cv.release();

Copilot · 2025-12-23T08:04:32Z

src/Frame.cpp

+	brga_image_cv = QImage2BGRACvMat(image);
+	return brga_image_cv;
+}
+
+void Frame::SetBGRACvMat(cv::Mat _image) {
+	brga_image_cv = _image;


The variable name has a typo: brga_image_cv should be bgra_image_cv to correctly represent the BGRA color format.

Suggested change

brga_image_cv = QImage2BGRACvMat(image);

return brga_image_cv;

}

void Frame::SetBGRACvMat(cv::Mat _image) {

brga_image_cv = _image;

bgra_image_cv = QImage2BGRACvMat(image);

return bgra_image_cv;

}

void Frame::SetBGRACvMat(cv::Mat _image) {

bgra_image_cv = _image;

Copilot · 2025-12-23T08:04:32Z

src/Frame.cpp

+	brga_image_cv = QImage2BGRACvMat(image);
+	return brga_image_cv;
+}
+
+void Frame::SetBGRACvMat(cv::Mat _image) {
+	brga_image_cv = _image;


The variable name has a typo: brga_image_cv should be bgra_image_cv to correctly represent the BGRA color format.

Suggested change

brga_image_cv = QImage2BGRACvMat(image);

return brga_image_cv;

}

void Frame::SetBGRACvMat(cv::Mat _image) {

brga_image_cv = _image;

bgra_image_cv = QImage2BGRACvMat(image);

return bgra_image_cv;

}

void Frame::SetBGRACvMat(cv::Mat _image) {

bgra_image_cv = _image;

Copilot · 2025-12-23T08:04:33Z

src/Frame.cpp

+		CV_8UC4, (uchar*)qimage->constBits(),
+		qimage->bytesPerLine()
+	);
+	return cv_img;


The QImage2BGRACvMat function creates a cv::Mat that shares memory with the QImage (shallow copy) without calling .clone(). This is inconsistent with the Qimage2mat function (line 875) which explicitly clones the data. While this works for the current Outline effect usage, it could lead to memory safety issues if the QImage is modified or destroyed while the returned cv::Mat is still in use. Consider adding .clone() for consistency and safety, similar to how Qimage2mat handles it.

Suggested change

return cv_img;

// Return a deep copy to avoid depending on QImage lifetime / mutability

return cv_img.clone();

aperture147 added 8 commits March 21, 2025 03:14

Add support for alpha channel in image conversion methods and refacto…

a11c9b2

…r related code

Refactor image conversion methods to be member functions of Frame cla…

4551154

…ss for better encapsulation

Add unit test for image conversion with alpha channel support

b020108

Rename imagecv to brga_image_cv for clarity and consistency in Frame …

ed33b6d

…class

Fix author attribution and remove unrelated note

11f8f78

Update author attribution and modify image channel check in unit test…

2520c68

… for alpha support

Update author attribution in Outline effect files for clarity

be5e2e9

Release resources for brga_image_cv in Frame destructor

f2f76e0

aperture147 mentioned this pull request Mar 25, 2025

Add shadow effect to libopenshot #999

Open

aperture147 added 5 commits May 25, 2025 15:48

Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter

b3c171f

Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter

b8f6bd4

Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter

a7cd61c

Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter

985a9df

Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter

02637fc

Copilot AI review requested due to automatic review settings December 23, 2025 07:57

Copilot started reviewing on behalf of aperture147 December 23, 2025 07:58 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `QImage` and `CV::Mat` conversion functions into `Frame` for better handling alpha channel handling #998

Add `QImage` and `CV::Mat` conversion functions into `Frame` for better handling alpha channel handling #998

aperture147 commented Mar 21, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 22, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	brga_image_cv = QImage2BGRACvMat(image);
	// Create a cv::Mat that references the QImage, then store a deep copy
	brga_image_cv = QImage2BGRACvMat(image).clone();

	QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_ARGB32);
	QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_RGBA8888);

-	cv::Mat cv_img(
-		qimage->height(), qimage->width(),
-		CV_8UC4, (uchar*)qimage->constBits(),
-		qimage->bytesPerLine()
-	);
-	return cv_img;
+	// QImage::Format_RGBA8888_Premultiplied stores data in RGBA order.
+	// First, wrap the existing bits as an RGBA cv::Mat (no copy).
+	cv::Mat rgba_img(
+		qimage->height(), qimage->width(),
+		CV_8UC4, const_cast<uchar*>(qimage->constBits()),
+		qimage->bytesPerLine()
+	);
+	// Then convert channel order from RGBA to BGRA as expected by OpenCV.
+	cv::Mat bgra_img;
+	cv::cvtColor(rgba_img, bgra_img, cv::COLOR_RGBA2BGRA);
+	return bgra_img;

	cv::Mat brga_image_cv; ///< OpenCV image. It will always be in BGR format
	cv::Mat bgra_image_cv; ///< OpenCV image. It will always be in BGR format

	return cv_img;
	// Return a deep copy to avoid depending on QImage lifetime / mutability
	return cv_img.clone();

Add QImage and CV::Mat conversion functions into Frame for better handling alpha channel handling #998

Are you sure you want to change the base?

Add QImage and CV::Mat conversion functions into Frame for better handling alpha channel handling #998

Conversation

aperture147 commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Add `QImage` and `CV::Mat` conversion functions into `Frame` for better handling alpha channel handling #998

Add `QImage` and `CV::Mat` conversion functions into `Frame` for better handling alpha channel handling #998

aperture147 commented Mar 21, 2025 •

edited

Loading

codecov bot commented Mar 22, 2025 •

edited

Loading