After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752937 - glupload: Raw upload is doing an extra copy
glupload: Raw upload is doing an extra copy
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-27 19:20 UTC by Nicolas Dufresne (ndufresne)
Modified: 2015-08-16 13:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Revert "glupload: memcpy on raw data upload" (1.85 KB, patch)
2015-07-27 21:01 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
glupload: Keep input frame mapped as long as needed (5.58 KB, patch)
2015-07-27 21:01 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2015-07-27 19:20:48 UTC
Since:

commit 82c0189b2842e8729e82a4e73491dffc977bc7c2
Author: Matthew Waters <matthew@centricular.com>
Date:   Tue Jul 14 17:40:32 2015 +1000

    glupload: memcpy on raw data upload

The raw uploader is doing an extra copy. This means that we copy the full frame in GST and as we know glTexSubImage2D will also to a copy. This patch also does not add a trace in the performance category even though this has serious performance impact.
Comment 1 Nicolas Dufresne (ndufresne) 2015-07-27 21:01:38 UTC
Created attachment 308248 [details] [review]
Revert "glupload: memcpy on raw data upload"

This reverts commit 82c0189b2842e8729e82a4e73491dffc977bc7c2.
Comment 2 Nicolas Dufresne (ndufresne) 2015-07-27 21:01:42 UTC
Created attachment 308249 [details] [review]
glupload: Keep input frame mapped as long as needed

When performing a raw upload, we need to keep the raw data mapped as
long as needed.
Comment 3 Matthew Waters (ystreet00) 2015-07-28 11:36:50 UTC
Review of attachment 308248 [details] [review]:

.
Comment 4 Matthew Waters (ystreet00) 2015-07-28 11:37:03 UTC
Review of attachment 308249 [details] [review]:

Looks good.
Comment 5 Nicolas Dufresne (ndufresne) 2015-07-28 12:51:55 UTC
Great, let's do that for now, I'll re-add the FIXME about eventually adding a buffer pool, as I think it was a good idea.

Note that if we go for pool, we'll need a different way to attach this reference. In V4L2 pool, I do so using gst_mini_object_set_qdata(). We could do so on each GstGLMemory, which has a GDestroyNotify, and that could be called when memory is destroyed, and then the pool could systematically drop that quark from the memory in the pool reset() function (which also calls the destroy notify function). I'll file an enhancement bug for that.
Comment 6 Nicolas Dufresne (ndufresne) 2015-07-28 13:10:34 UTC
Attachment 308248 [details] pushed as eb4d3c3 - Revert "glupload: memcpy on raw data upload"
Attachment 308249 [details] pushed as adbd9d3 - glupload: Keep input frame mapped as long as needed
Comment 7 Nicolas Dufresne (ndufresne) 2015-07-28 13:11:02 UTC
commit f79cad55e2a9b7c5057318aa04362bb4277e009a
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Tue Jul 28 08:59:24 2015 -0400

    glupload: Add fixme about using bufferpool for raw
    
    http://bugzilla.gnome.org/show_bug.cgi?id=752937