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 774213 - vaapivideomemory: lock at mapping and unmapping
vaapivideomemory: lock at mapping and unmapping
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 772151
Blocks:
 
 
Reported: 2016-11-10 14:04 UTC by Víctor Manuel Jáquez Leal
Modified: 2016-11-11 11:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vaapivideomemory: refactor vaapi memory unmapping (3.69 KB, patch)
2016-11-10 14:26 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapivideomemory: lock map and unmap operations (5.70 KB, patch)
2016-11-10 14:26 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapivideomemory: fail if frame map can't get plane (1.02 KB, patch)
2016-11-10 14:26 UTC, Víctor Manuel Jáquez Leal
committed Details | Review

Description Víctor Manuel Jáquez Leal 2016-11-10 14:04:29 UTC
As Jan told me a few days ago, we should add a lock when mapping and unmapping to avoid race conditions when two thread want to map - unmap a frame from vaapi

For example, this pipeline:

gst-launch-1.0 videotestsrc ! vaapipostproc ! tee name=t \
        ! queue ! xvimagesink sync=false 
     t. ! queue ! xvimagesink sync=false
Comment 1 Víctor Manuel Jáquez Leal 2016-11-10 14:26:09 UTC
Created attachment 339491 [details] [review]
vaapivideomemory: refactor vaapi memory unmapping

There were duplicated code in gst_video_meta_unmap_vaapi_memory() and
gst_vaapi_video_memory_unmap() when unmapping.

This patch refactors both methods adding the common function
unmap_vaapi_memory(). This also ensures, if direct rendering is enabled, it
is correctly reset.

Additionally, only when mapping flag has the WRITE bit, it set the image as
current, which was done in gst_video_meta_map_vaapi_memory() but no in
gst_vaapi_video_memory_map().

In order to make this, the mapping flags were required, so instead of
overloading mem_unmap() virtual function, mem_unmap_full() is overloaded.
Comment 2 Víctor Manuel Jáquez Leal 2016-11-10 14:26:14 UTC
Created attachment 339492 [details] [review]
vaapivideomemory: lock map and unmap operations

In order to avoid race condition when two threads call map/unmap the same
VA surface, this patch mutex these operations.
Comment 3 Víctor Manuel Jáquez Leal 2016-11-10 14:26:19 UTC
Created attachment 339493 [details] [review]
vaapivideomemory: fail if frame map can't get plane

If map() vmethod in GstVideMeta cannot get the plane data, return false,
thus the caller will not try to read invalid memory.
Comment 4 Víctor Manuel Jáquez Leal 2016-11-11 11:58:30 UTC
Attachment 339491 [details] pushed as 4ae9428 - vaapivideomemory: refactor vaapi memory unmapping
Attachment 339492 [details] pushed as 3fa9af2 - vaapivideomemory: lock map and unmap operations
Attachment 339493 [details] pushed as a02c86b - vaapivideomemory: fail if frame map can't get plane