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 704078 - plugins: enable memory map for read & write
plugins: enable memory map for read & write
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
: 733242 (view as bug list)
Depends on: 704083
Blocks: 731852 735360
 
 
Reported: 2013-07-12 10:38 UTC by Víctor Manuel Jáquez Leal
Modified: 2015-06-30 12:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
plugins: enable memory map for read & write (1.41 KB, patch)
2013-07-12 10:38 UTC, Víctor Manuel Jáquez Leal
none Details | Review

Description Víctor Manuel Jáquez Leal 2013-07-12 10:38:43 UTC
vaapisink can display raw buffers decoded by gst-libav or any other decoder.
Comment 1 Víctor Manuel Jáquez Leal 2013-07-12 10:38:46 UTC
Created attachment 248997 [details] [review]
plugins: enable memory map for read & write

Hence vaapisink can display buffers decoded by gst-libav.
Comment 2 Gwenole Beauchesne 2013-07-12 11:19:10 UTC
Hi, that's indeed an issue but I have not found any solution for the broader one: if we allow R/W mappings, we have to detect cases when the user maps the GstBuffer and expects to read data from it. So, we would have to download the VA surface contents first.

That's why, supporting read-only or write-only mappings are easy and efficient. read/write mappings are too generic. This could mean:
- reading, so downloading will be necessary ;
- writing, so only upload will be needed ;
- or read-modify-write, which means VA surface download first would be needed.

IMHO, it would be best if we had an additional flag that would tell "hey, I asked RW mapping but this is because I guarantee you that I will be writing to the whole buffer and possible read from it for intermediate operations".

What about GST_MAP_RESET, thus telling that the caller would be touching to the whole GstBuffer during the mapping? Or, alternatively, a flag that would mean "hey, I really expect data to be valid and the first operation to the buffer would be a read". e.g. GST_MAP_READ_FIRST?

That way, we would have the following cases:
- GST_MAP_READ: map with VA surface download first ;
- GST_MAP_WRITE: map, and VA surface upload on un-map ;
- GST_MAP_READWRITE: map, and VA surface upload on un-map;
- GST_MAP_READWRITE|GST_MAP_READ_FIRST: map, download VA surface, and commit it back on un-map;
Comment 3 Víctor Manuel Jáquez Leal 2013-07-12 11:48:24 UTC
(In reply to comment #2)
> Hi, that's indeed an issue but I have not found any solution for the broader
> one: if we allow R/W mappings, we have to detect cases when the user maps the
> GstBuffer and expects to read data from it. So, we would have to download the
> VA surface contents first.

Damn! I knew it wasn't that simple :/

IIUC, gst-libav maps the buffer to read some metadata on it and then it is used for write the frame data, just like GST_MAP_READ_FIRST that you propose.
Comment 4 Gwenole Beauchesne 2013-10-09 08:28:23 UTC
Even if #704083 is fixed, we cannot expect people to upgrade to a newer GStreamer stack. So, I will allow read/write mappings only if direct-rendering mode is supported. That's the minimum thing to do. Though, this limits the usage to VA drivers that support vaDeriveImage() [Intel].
Comment 5 Gwenole Beauchesne 2013-10-09 16:33:13 UTC
commit 77b2d884520f8d5de7f5d77545f2373da002fd30
Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Date:   Wed Oct 9 10:33:55 2013 +0200

    plugins: enable memory maps for read & write with direct-rendering.
    
    Enable read and write mappings only if direct-rendering is supported.
    Otherwise, this means that we may need to download data from the VA
    surface first for correctness, even if the VA surface doesn't need to
    be read at all. i.e. sometimes, READWRITE mappings are meant for
    surfaces that are written to first, and read afterwards for further
    processing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=704078
Comment 6 Gwenole Beauchesne 2013-12-12 10:38:47 UTC
Victor, do you still have issues with the mentioned patch that was pushed to git master branch, now released as 0.5.7? At least, it works for me on Intel platforms.

The general case (vaGetImage() / vaPutImage()) looks complex, in terms of efforts needed to upgrade all the existing plugins, to get the expected performance level on platforms that do not support vaDeriveImage().
Comment 7 Víctor Manuel Jáquez Leal 2013-12-20 15:53:15 UTC
Hey!

I finally sat on my pants to test this.

With videotestsrc it works great, but if using gst-libav, as the decoder wants to use the sink's buffer-pool, when ask to write in them, the pipeline fails. But placing a videoconvert between the decoder and the sink (I guess because of the bugs when handling the features) the pipeline works.

I thinks the fix should be in gst-libav, the decode should create its own buffer-pool if the sink doesn't provide a proper one.
Comment 8 Gwenole Beauchesne 2014-07-23 17:31:39 UTC
*** Bug 733242 has been marked as a duplicate of this bug. ***
Comment 9 Gwenole Beauchesne 2014-11-19 10:14:18 UTC
(In reply to comment #7)
> Hey!
> 
> I finally sat on my pants to test this.
> 
> With videotestsrc it works great, but if using gst-libav, as the decoder wants
> to use the sink's buffer-pool, when ask to write in them, the pipeline fails.
> But placing a videoconvert between the decoder and the sink (I guess because of
> the bugs when handling the features) the pipeline works.
> 
> I thinks the fix should be in gst-libav, the decode should create its own
> buffer-pool if the sink doesn't provide a proper one.

The other issue with avdec I believe is that in some use cases, the video frame is not going to be unmaped, thus not committed back to the underlying VA surface. So, we will get jerkiness.
Comment 10 Gwenole Beauchesne 2014-11-19 10:15:20 UTC
commit 0e87abc574427c6f1d702492fd71d7723a06ff4e
Author: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>
Date:   Fri Jul 12 06:34:15 2013 -0400

    plugins: enable memory maps for read & write.
    
    Hence vaapisink can display buffers decoded by gst-libav, or HW decoded
    buffers can be further processed in-place, e.g. with a textoverlay.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=704078
    
    [ported to current git master branch, amended commit message]
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Comment 11 Víctor Manuel Jáquez Leal 2015-06-30 12:57:42 UTC
*** Bug 751711 has been marked as a duplicate of this bug. ***