Skip to content

Zerocopy gstreamer videocapture #18377

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

Jump to bottom
Draft
wants to merge 2 commits into
base: 4.x
Choose a base branch
from
Draft

Zerocopy gstreamer videocapture #18377

wants to merge 2 commits into from

Conversation

michaelgruner
Copy link
Contributor

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work: N/A
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable N/A?
  • Patch to opencv_extra has the same branch name. N/A
  • The feature is well documented and sample code can be built with the project CMake: N/A

Description

Removes an exisiting memory copy when creating a Mat from a Gstreamer buffer, when using the VideoCapture class. To do so, the pointer to the Gstreamer buffer data is shared in the Mat. To handle the pointer's lifespan, a Gstreamer custom allocator is proposed so that when the Mat's data is to be destroyed, the data is properly returned and freed by Gstreamer.

Gstreamer is the preferred multimedia framework for embedded devices. Considering this, performance is typically a priority. Currently, capturing with OpenCV is slower than capturing with plain Gstreamer. I'm keeping track of the improvements here.

Pending Issues

  • Gstreamer buffer is being mapped as READONLY, while the Mat is READ WRITE. This is currently WIP.

@asmorkalov
Copy link
Contributor

@michaelgruner Thanks for the contribution!

@mshabunin
Copy link
Contributor

@michaelgruner , does it work when GStreamer backend is built as a plug-in? -DVIDEOIO_PLUGIN_LIST=gstreamer cmake option.

@michaelgruner
Copy link
Contributor Author

I'm not sure. I'll give it a try.

@michaelgruner
Copy link
Contributor Author

Just tested. It works as with built-in.

[ INFO:0] global /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/modules/videoio/src/videoio_registry.cpp (191) VideoBackendRegistry VIDEOIO: Enabled backends(8, sorted by priority): FFMPEG(1000); GSTREAMER(990); INTEL_MFX(980); MSMF(970); V4L2(960); CV_IMAGES(950); CV_MJPEG(940); FIREWIRE(930)
[ INFO:0] global /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/modules/videoio/src/backend_plugin.cpp (359) getPluginCandidates VideoIO pluigin (GSTREAMER): glob is 'libopencv_videoio_gstreamer*.so', 1 location(s)
[ INFO:0] global /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/modules/videoio/src/backend_plugin.cpp (366) getPluginCandidates     - /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/uninstalled/lib: 1
[ INFO:0] global /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/modules/videoio/src/backend_plugin.cpp (370) getPluginCandidates Found 1 plugin(s) for GSTREAMER
[ INFO:0] global /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/modules/videoio/src/backend_plugin.cpp (175) libraryLoad load /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/uninstalled/lib/libopencv_videoio_gstreamer.so => OK
[ INFO:0] global /home/mgruner/RidgeRun/OpenCV/OpenCV/opencv/modules/videoio/src/backend_plugin.cpp (236) PluginBackend Video I/O: loaded plugin 'GStreamer OpenCV Video I/O plugin'

@asmorkalov asmorkalov requested a review from VadimLevin October 21, 2020 09:41
@vpisarev
Copy link
Contributor

vpisarev commented Dec 1, 2020

@asmorkalov, shouldn't we just merge it?

@alalek
Copy link
Member

alalek commented Dec 1, 2020

@vpisarev Consider reviewing this first. Current MatAllocator behavior is up to you - nobody understand this logic completely.

As you can see several methods are just not implemented - do we really need them in this basic interface?

@asmorkalov
Copy link
Contributor

@vpisarev . I retriggered Ci build to get status and looking the code right now. Will come back with review results today.

@alalek
Copy link
Member

alalek commented Dec 3, 2020

I retriggered Ci build to get status

Please note, that NONE of triggered builds from default "pre-commit" scope performs building/testing of GStreamer code.

@asmorkalov asmorkalov requested review from asmorkalov and removed request for VadimLevin December 9, 2020 14:35
@asmorkalov asmorkalov self-assigned this Dec 9, 2020
@michaelgruner
Copy link
Contributor Author

@alalek is this something I need to do on my side as external contributor?

@asmorkalov
Copy link
Contributor

@michaelgruner Thanks for your contribution. Alexander works on release right now. We have a bunch of changes related to multimedia and plugins support and plan to integrate your work after release with plugins API rework.

alalek
alalek reviewed Jan 15, 2021
View reviewed changes
modules/videoio/src/cap_gstreamer.cpp Outdated
Comment on lines 582 to 1061
try {
dst.getMatRef() = GStreamerMat(sample.get());
} catch (cv::Exception &e) {
CV_WARN(e.err);
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

This part introduces regression.

OutputArray is a wrapper over multiple container types, including UMat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

May I get some clarification on this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

retrieveFrame method has OuputArray as parameter. OutputArray is proxy class that wraps cv::Mat, cv::UMat, std::vector and some other containers to provide flexibility to OpenCV users. In general case dst can be UMat and the proposed solution fails. OutputArray has kind() method that reports original container type. You need to check it before assignment. Other cases can be handled with copy as it was done before.

modules/videoio/src/cap_gstreamer.cpp
{
flags += CV_MAT_TYPE(_type);
rows = _rows;
cols = _cols;
Copy link
Member

Choose a reason for hiding this comment

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

fillMat

naming is not accurate and confusing.

There are setSize() / finalizeHdr() / updateContinuityFlag() calls in the core module.
The best option is trying to reuse these calls (to keep library consistent by avoiding duplication of code).
Or at least, rename to fillMatHeader().

modules/videoio/src/cap_gstreamer.cpp
if (!sample)
return false;
if (false == gst_buffer_map (gstdata->buffer, &(gstdata->info), GST_MAP_READ)) {
CV_Error(Error::Code::StsUnsupportedFormat, ("Unable to map GstBuffer"));
Copy link
Member

Choose a reason for hiding this comment

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

memory leak is caused here (new gstdata)

modules/videoio/src/cap_gstreamer.cpp
gstdata->sample.release();

delete gstdata;
delete u;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it is better to swap order of delete operators to avoid having of dangling pointers.

modules/videoio/src/cap_gstreamer.cpp Outdated
Comment on lines 573 to 596
* \brief CvCapture_GStreamer::retrieveFrame
* \return IplImage pointer. [Transfer Full]
* Retrieve the previously grabbed buffer, and wrap it in an IPLImage structure
Copy link
Member

Choose a reason for hiding this comment

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

I know that it is copy-paste, but I believe we should not copy outdated comments anyway.

modules/videoio/src/cap_gstreamer.cpp
UMatData* u = new UMatData(this);
u->data = u->origdata = gstdata->info.data;
u->size = gstdata->info.size;
u->userdata = gstdata;
Copy link
Member

Choose a reason for hiding this comment

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

@vpisarev Is it valid usage of userdata field?

modules/videoio/src/cap_gstreamer.cpp
{
CV_Error(Error::Code::StsBadFunc, ("GStreamer allocator may not alloc raw data"));
return nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

CV_Error() throws exception (marked with "noreturn" attribute).

return nullptr; is unreachable code.

asmorkalov
asmorkalov reviewed Jan 20, 2021
View reviewed changes
modules/videoio/src/cap_gstreamer.cpp
pipeline.release();
}
CV_Error(Error::Code::StsBadFunc, ("GStreamer allocator may not alloc raw data"));
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

return is unreachable too.

@asmorkalov asmorkalov mentioned this pull request Jan 21, 2021
Merged
6 tasks
@asmorkalov
Copy link
Contributor

@michaelgruner Could you rebase you PR to current master and fix remarks?

@michaelgruner
Copy link
Contributor Author

michaelgruner commented Jan 27, 2021 via email

@asmorkalov
Copy link
Contributor

@michaelgruner Friendly reminder.

@michaelgruner michaelgruner force-pushed the zerocopy-gstreamer-videocapture branch from ff38ea7 to 2efaffd Compare February 10, 2021 15:29
@michaelgruner michaelgruner marked this pull request as draft May 19, 2022 18:04
@michaelgruner
Copy link
Contributor Author

Hey all, I'm sorry I got lost for a while there. I'm back with some time to get this through. I'm rebasing the changes to the latest master and figuring out conflicts.

@michaelgruner
Copy link
Contributor Author

Pulling #22018 first as a dependency.

@michaelgruner
Implement GStreamer allocator to keep the GstBuffer alive through the…
c8800a3 
… mat lifespan

Instead of copying the data from the GstSample into a Mat, the
allocator keeps a mapped reference of the GstBuffer pulled from
the pipeline. By implementing a custom GStreamerAllocator we instruct
OpenCV on how to properly unmap and unref the GstBuffer, even if the
data was copied to a regular Mat.
@michaelgruner michaelgruner force-pushed the zerocopy-gstreamer-videocapture branch from 2efaffd to 25df008 Compare June 29, 2022 06:50
@michaelgruner michaelgruner force-pushed the zerocopy-gstreamer-videocapture branch from 25df008 to 7a3fbbb Compare June 29, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asmorkalov asmorkalov asmorkalov left review comments

@alalek alalek alalek left review comments

Assignees

@asmorkalov asmorkalov

Labels
category: videoio RFC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@michaelgruner @asmorkalov @mshabunin @vpisarev @alalek