Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aperture147 wants to merge 13 commits into
base: develop
Choose a base branch
from
+105 −33
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a11c9b2
Add support for alpha channel in image conversion methods and refacto…
aperture147 Mar 21, 2025
4551154
Refactor image conversion methods to be member functions of Frame cla…
aperture147 Mar 21, 2025
b020108
Add unit test for image conversion with alpha channel support
aperture147 Mar 21, 2025
ed33b6d
Rename imagecv to brga_image_cv for clarity and consistency in Frame …
aperture147 Mar 21, 2025
11f8f78
Fix author attribution and remove unrelated note
aperture147 Mar 22, 2025
2520c68
Update author attribution and modify image channel check in unit test…
aperture147 Mar 22, 2025
be5e2e9
Update author attribution in Outline effect files for clarity
aperture147 Mar 22, 2025
f2f76e0
Release resources for brga_image_cv in Frame destructor
aperture147 Mar 22, 2025
b3c171f
Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter
aperture147 May 25, 2025
b8f6bd4
Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter
aperture147 May 29, 2025
a7cd61c
Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter
aperture147 Jun 4, 2025
985a9df
Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter
aperture147 Jul 15, 2025
02637fc
Merge branch 'OpenShot:develop' into add-bgra-cvmat-converter
aperture147 Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 42 additions & 5 deletions src/Frame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @file
* @brief Source file for Frame class
* @author Jonathan Thomas <jonathan@openshot.org>
* @author HaiVQ <me@haivq.com>
*
* @ref License
*/
Expand Down Expand Up @@ -113,6 +114,7 @@ Frame::~Frame() {
audio.reset();
#ifdef USE_OPENCV
imagecv.release();
brga_image_cv.release();
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
#endif
}

Expand Down Expand Up @@ -893,6 +895,13 @@ cv::Mat Frame::GetImageCV()
return imagecv;
}

// Set pointer to OpenCV image object
void Frame::SetImageCV(cv::Mat _image)
{
imagecv = _image;
image = Mat2Qimage(_image);
}

std::shared_ptr<QImage> Frame::Mat2Qimage(cv::Mat img){
cv::cvtColor(img, img, cv::COLOR_BGR2RGB);
QImage qimg((uchar*) img.data, img.cols, img.rows, img.step, QImage::Format_RGB888);
Expand All @@ -906,11 +915,39 @@ std::shared_ptr<QImage> Frame::Mat2Qimage(cv::Mat img){
return imgIn;
}

// Set pointer to OpenCV image object
void Frame::SetImageCV(cv::Mat _image)
{
imagecv = _image;
image = Mat2Qimage(_image);
// Convert QImage to cv::Mat and vice versa
// Frame class has GetImageCV, but it does not include alpha channel
// so we need a separate methods which preserve alpha channel
cv::Mat Frame::QImage2BGRACvMat(std::shared_ptr<QImage>& qimage) {
cv::Mat cv_img(
qimage->height(), qimage->width(),
CV_8UC4, (uchar*)qimage->constBits(),
qimage->bytesPerLine()
);
return cv_img;
Comment on lines +922 to +927
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copilot uses AI. Check for mistakes.
}

// Convert cv::Mat back to QImage
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);
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
std::shared_ptr<QImage> imgIn = std::make_shared<QImage>(qimage.convertToFormat(QImage::Format_RGBA8888_Premultiplied));
return imgIn;
}

// Get BGRA
cv::Mat Frame::GetBGRACvMat() {
if (!image)
// Fill with black
AddColor(width, height, color);
brga_image_cv = QImage2BGRACvMat(image);
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
return brga_image_cv;
}

void Frame::SetBGRACvMat(cv::Mat _image) {
brga_image_cv = _image;
Comment on lines +944 to +949
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
Comment on lines +944 to +949
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
image = BGRACvMat2QImage(_image);
}
#endif

Expand Down
14 changes: 14 additions & 0 deletions src/Frame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @file
* @brief Header file for Frame class
* @author Jonathan Thomas <jonathan@openshot.org>
* @author HaiVQ <me@haivq.com>
*
* @ref License
*/
Expand Down Expand Up @@ -106,6 +107,7 @@ namespace openshot

#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
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
#endif

/// Constrain a color value from 0 to 255
Expand Down Expand Up @@ -276,6 +278,18 @@ namespace openshot

/// Set pointer to OpenCV image object
void SetImageCV(cv::Mat _image);

/// Convert QImage to OpenCV Mat (alpha channel included)
cv::Mat QImage2BGRACvMat(std::shared_ptr<QImage>& qimage);

/// Convert OpenCV Mat to QImage (alpha channel included)
std::shared_ptr<QImage> BGRACvMat2QImage(cv::Mat img);

/// Get pointer to OpenCV Mat image object (with alpha channel)
cv::Mat GetBGRACvMat();

/// Set pointer to OpenCV image object (with alpha channel)
void SetBGRACvMat(cv::Mat _image);
#endif
};

Expand Down
41 changes: 20 additions & 21 deletions src/effects/Outline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/**
* @file
* @brief Source file for Outline effect class
* @author Jonathan Thomas <jonathan@openshot.org>, HaiVQ <me@haivq.com>
*
* @author Jonathan Thomas <jonathan@openshot.org>
* @author HaiVQ <me@haivq.com>
*
* @ref License
*/

Expand Down Expand Up @@ -60,12 +61,11 @@ std::shared_ptr<openshot::Frame> Outline::GetFrame(std::shared_ptr<openshot::Fra
}

// Get the frame's image
std::shared_ptr<QImage> frame_image = frame->GetImage();

cv::Mat cv_image = frame->GetBGRACvMat();
float sigmaValue = widthValue / 3.0;
if (sigmaValue <= 0.0)
sigmaValue = 0.01;
cv::Mat cv_image = QImageToBGRACvMat(frame_image);

// Extract alpha channel for the mask
std::vector<cv::Mat> channels(4);
Expand Down Expand Up @@ -95,25 +95,24 @@ std::shared_ptr<openshot::Frame> Outline::GetFrame(std::shared_ptr<openshot::Fra
solid_color_mat.copyTo(final_image, outline_mask);
cv_image.copyTo(final_image, alpha_mask);

std::shared_ptr<QImage> new_frame_image = BGRACvMatToQImage(final_image);

// FIXME: The shared_ptr::swap does not work somehow
*frame_image = *new_frame_image;
frame->SetBGRACvMat(final_image);

return frame;
}

cv::Mat Outline::QImageToBGRACvMat(std::shared_ptr<QImage>& qimage) {
cv::Mat cv_img(qimage->height(), qimage->width(), CV_8UC4, (uchar*)qimage->constBits(), qimage->bytesPerLine());
return cv_img;
}

std::shared_ptr<QImage> Outline::BGRACvMatToQImage(cv::Mat img) {
cv::Mat final_img;
cv::cvtColor(img, final_img, cv::COLOR_RGBA2BGRA);
QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_ARGB32);
std::shared_ptr<QImage> imgIn = std::make_shared<QImage>(qimage.convertToFormat(QImage::Format_RGBA8888_Premultiplied));
return imgIn;
}
// Moved to Frame.cpp
// cv::Mat Outline::QImageToBGRACvMat(std::shared_ptr<QImage>& qimage) {
// cv::Mat cv_img(qimage->height(), qimage->width(), CV_8UC4, (uchar*)qimage->constBits(), qimage->bytesPerLine());
// return cv_img;
// }

// std::shared_ptr<QImage> Outline::BGRACvMatToQImage(cv::Mat img) {
// cv::Mat final_img;
// cv::cvtColor(img, final_img, cv::COLOR_RGBA2BGRA);
// QImage qimage(final_img.data, final_img.cols, final_img.rows, final_img.step, QImage::Format_ARGB32);
// std::shared_ptr<QImage> imgIn = std::make_shared<QImage>(qimage.convertToFormat(QImage::Format_RGBA8888_Premultiplied));
// return imgIn;
// }

// Generate JSON string of this object
std::string Outline::Json() const {
Expand Down
13 changes: 6 additions & 7 deletions src/effects/Outline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/**
* @file
* @brief Header file for Outline effect class
* @author Jonathan Thomas <jonathan@openshot.org>, HaiVQ <me@haivq.com>
* @author Jonathan Thomas <jonathan@openshot.org>
* @author HaiVQ <me@haivq.com>
*
* @ref License
*/
Expand Down Expand Up @@ -33,19 +34,17 @@ namespace openshot
* with openshot::Keyframe curves over time.
*
* Outlines can be added around any image or text, and animated over time.
* Idea from: https://stackoverflow.com/a/78480103
*/
class Outline : public EffectBase
{
private:
/// Init effect settings
void init_effect_details();

// Convert QImage to cv::Mat and vice versa
// Although Frame class has GetImageCV, but it does not include alpha channel
// so we need a separate methods which preserve alpha channel
// Idea from: https://stackoverflow.com/a/78480103
cv::Mat QImageToBGRACvMat(std::shared_ptr<QImage>& qimage);
std::shared_ptr<QImage> BGRACvMatToQImage(cv::Mat img);
// Moved to Frame.h
// cv::Mat QImageToBGRACvMat(std::shared_ptr<QImage>& qimage);
// std::shared_ptr<QImage> BGRACvMatToQImage(cv::Mat img);

public:
Keyframe width; ///< Width of the outline
Expand Down
23 changes: 23 additions & 0 deletions tests/Frame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* @brief Unit tests for openshot::Frame
* @author Jonathan Thomas <jonathan@openshot.org>
* @author FeRD (Frank Dana) <ferdnyc@gmail.com>
* @author HaiVQ <me@haivq.com>
*
* @ref License
*/
Expand Down Expand Up @@ -160,4 +161,26 @@ TEST_CASE( "Convert_Image", "[libopenshot][opencv][frame]" )
CHECK(f1->GetHeight() == cvimage.rows);
CHECK(cvimage.channels() == 3);
}

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);
}
Comment on lines +165 to +185
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
#endif