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 739332 - Failing to connect vaapisink directly to decodebin
Failing to connect vaapisink directly to decodebin
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal major
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 739031 740110
 
 
Reported: 2014-10-29 06:47 UTC by sreerenj
Modified: 2016-05-28 18:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vaapidecode: Increase the rank to GST_RANK_PRIMARY + 1 (1.17 KB, patch)
2014-11-18 12:16 UTC, sreerenj
committed Details | Review

Description sreerenj 2014-10-29 06:47:56 UTC
The following pipeline is failing all the time:
gst-launch-1.0 filesrc location=sample.mp4 ! decodebin ! vaapisink

Error message:
:00:00.120910229 17238      0x1e15f20 ERROR       vaapivideomemory gstvaapivideomemory.c:197:gst_video_meta_map_vaapi_memory: unsupported map flags (0x3)
0:00:00.120950594 17238      0x1e15f20 ERROR                default video-frame.c:141:gst_video_frame_map_id: failed to map video frame plane 0

** (gst-launch-1.0:17238): CRITICAL **: gst_vaapi_image_get_plane: assertion 'image != NULL' failed

** (gst-launch-1.0:17238): CRITICAL **: gst_vaapi_image_get_pitch: assertion 'image != NULL' failed
Comment 1 Gwenole Beauchesne 2014-10-29 06:51:16 UTC
I got by this recently. It seems to occur with GStreamer 1.4 but not 1.2. Do you confirm on your end?
Comment 2 sreerenj 2014-10-29 06:55:38 UTC
R(In reply to comment #1)
> I got by this recently. It seems to occur with GStreamer 1.4 but not 1.2. Do
> you confirm on your end?

No, it is reproducible in 1.2 too!
I think itz a regression after gstreamer-vaapi-0.5.8...just an assumption based on https://bugzilla.gnome.org/show_bug.cgi?id=739031
Comment 3 Gwenole Beauchesne 2014-10-29 07:04:36 UTC
One reason is that decodebin is not selecting vaapidecode, thus defaulting to a SW decoder that uses GST_MAP_READWRITE map flags to decode into.
Comment 4 sreerenj 2014-10-29 07:21:10 UTC
The autoplugging of h/w accelerated elements in a pipeline in implemented only in playbin (http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=a8d1b4549184f4aec88132b3650fa2b8fe1ca970) based on the features of a combination of decoder+sink. 
So as you said, a manual pipeline with decodebin will still select a SW decoder most probably. 
Do we need to re-assign the bug to gst-plugins-base ? Or fix the vaapisink+SW_decoder combination in someway?
Comment 5 Gwenole Beauchesne 2014-10-29 07:27:44 UTC
I think decodebin should autoplug hw decoders too. That's more user friendly IMHO. I really thought it did before. Otherwise, what would be the point of setting a vaapisink while still using a SW decoder?

Besides, in the future with no hwaccel specific sink, it would also be more natural to always pick a HW decoder. i.e. the pipeline needs to be constructed in a way that best uses HW resources, thus selecting an adequate HW decoder when available.

At some point, I also wanted a specific vaapidecodebin, but that was meant as an interim solution. I really think we should autoplug HW elements.
Comment 6 sreerenj 2014-10-29 07:34:31 UTC
Okay lets reassign it to gst-plugins-base first, so that we can gather some opinion from upstream guys too.
Comment 7 Tim-Philipp Müller 2014-10-29 12:42:20 UTC
fwiw, I think there's a bug in gstreamer-vaapi that makes normal gst_buffer_map() fail (it works only when mapping via the video api), which may or may not be related.
Comment 8 Gwenole Beauchesne 2014-10-29 14:30:00 UTC
(In reply to comment #7)
> fwiw, I think there's a bug in gstreamer-vaapi that makes normal
> gst_buffer_map() fail (it works only when mapping via the video api), which may
> or may not be related.

Yes, I just don't want to support R/W mappings, so I let it crash. :)
See bug #704083 for a refresh on that. It's hard to achieve read/write mappings with HW buffers, unless you actually use userptr underneath for very particular use cases.

Besides, this is an interesting way to realize that HW accelerated elements did not get auto-plugged in.
Comment 9 sreerenj 2014-11-18 11:06:06 UTC
As per the discussion in IRC(#gstreamer), the better solution to enable the
autoplugging of HW decoder in decodebin is increasing the HW decoder rank:
If no other objection, I will provide a patch for that.

This is the IRC log:

<sree_> slomo: actully vaapidecode can output video/x-raw(sysmem) ,,vaapidecode
! xvimagesink is a working scenario 
<slomo> sree_: why is vaapidecode not chosen then? does it have a lower rank
than e.g. avdec_h264?
<sree_> slomo: no, both have rank primary
<slomo> well, A comes before V :)
* twi_ has quit (Ping timeout: 250 seconds)
<sree_> slomo: aha :)
<sree_> slomo: but if upstream element query vaapisink, it will return
video/x-raw(VASurface) as first preference...so decdoebin should select a
decoder which is matching with that, right?
<slomo> there is no upstream yet
<slomo> decodebin does not have any srcpads at that time
* darkbasic has quit (Read error: Connection reset by peer)
* gandhi_m has quit (Read error: Connection reset by peer)
* darkbasic (~quassel@niko.linuxsystems.it) has joined #gstreamer
<slomo> without overriding the autoplug-* signals it has no way to select
elements other than their ranks and their caps
<sree_> okay, that I asked with out looking into the code !
* Tarnyko (~mbachmann@46.18.96.46) has left #gstreamer
<sree_> may be to reorder elements with more number of capsfeatures in src_pad
as first in the list?
<slomo> this also means that in a standard decodebin scenario, vaapidecode only
knows that it can output video/x-raw(sysmem) without videometa btw
<slomo> no, i don't think there is a reason why more capsfeatures in general
should be better than less :)
<sree_> if ranks are the same, 
<sree_> the one with more capsfeatures supports more memory types,,
<sree_> so it is superior,,
<slomo> i don't think that's generally true
<sree_> we are using similar (bit different of course) logic in playbin...more
numb of common capsfeatreus between dec and sink... 
<slomo> yes, because there you can compare those capsfeatures with an actual
sink
<sree_> All HW decoders are supposed to support more memtypes than SW decoder
...so it is a clear superiority IMO
<sree_> we will get a benefit of hw decoding..is it a convincing explanation
for adding something like i proposed ? 
<sree_> may be i can create a new bug to track this?? WDT
<slomo> i don't think i agree with your assumptions
<sree_> so, you are concluding that there is no way to autoplug the HW decoder
in decodebin?
<sree_> hm,,there are multiple bugs waiting to get a fix for this
* lmr_ (~lmr@179.110.43.152) has joined #gstreamer
<sree_> eg: #740110, #739031 
* Guest57714 is now known as prasu
<slomo> sree_: just give it a higher rank
<sree_> I hope giving a higher rank won't affect the use case of SW decoder
fallback if a particular codec profile is not supported by the HW decoder...
(It shouldn't , just making sure)
* desti_T2 (~desti@dslb-092-072-214-047.092.072.pools.vodafone-ip.de) has
joined #gstreamer
<slomo> sree_: no, that makes no difference assuming that vaapidecode properly
declares which profiles it supports on its sinkpad
<slomo> but that's an independent issue in any case, if it doesn't do ↑ it will
cause problems in any case
* gandhi_m (~gandhi_m@pd95bcf90.dip0.t-ipconnect.de) has joined #gstreamer
<sree_> slomo: k thanks
Comment 10 sreerenj 2014-11-18 12:16:40 UTC
Created attachment 290902 [details] [review]
vaapidecode: Increase the rank to GST_RANK_PRIMARY + 1
Comment 11 Gwenole Beauchesne 2014-11-18 12:28:45 UTC
(In reply to comment #10)
> Created an attachment (id=290902) [details] [review]
> vaapidecode: Increase the rank to GST_RANK_PRIMARY + 1

I am fine with that, I just want to be sure that's OK for other GStreamer people too and that fallback to SW decoders still works. Maybe make this #if GST_CHECK_VERSION(1,5,0) where profiles are explicitly propagated downstream?
Comment 12 sreerenj 2014-11-18 12:32:54 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > Created an attachment (id=290902) [details] [review] [details] [review]
> > vaapidecode: Increase the rank to GST_RANK_PRIMARY + 1
> 
> I am fine with that, I just want to be sure that's OK for other GStreamer
> people too and that fallback to SW decoders still works. Maybe make this #if
> GST_CHECK_VERSION(1,5,0) where profiles are explicitly propagated downstream?

Fallback to SW decoders still works, tested with h264-high-10 profile samples.
Why we need a GST_CHECK_VERSION() here?, it is not going to break anything I guess..
Comment 13 Sebastian Dröge (slomo) 2014-11-18 12:49:56 UTC
You only need to ensure that it works properly with video/x-raw(sysmem) and also works properly without GstVideoMeta. That is, it should be possible to read-write map the memory!
Comment 14 Gwenole Beauchesne 2014-11-18 17:53:19 UTC
(In reply to comment #13)
> You only need to ensure that it works properly with video/x-raw(sysmem) and
> also works properly without GstVideoMeta. That is, it should be possible to
> read-write map the memory!

Does this mean we might need to perform in-place postprocessing of decoded surfaces? wrt. to comment about R/W map the memory.

FYI, I have additionally tested the following pipelines:
$ ... decodebin ! xvimagesink
$ ... decodebin ! videoconvert ! ximagesink
$ ... decodebin ! vaapipostproc ! ximagesink
$ ... decodebin ! glimagesink
$ ... decodebin ! deinterlace ! vaapisink
$ ... decodebin ! deinterlace ! xvimagesink
$ ... decodebin ! videoscale ! video/x-raw,width=1280,height=720 ! xvimagesink

That worked, so I have pushed the patch. :)
Comment 15 Gwenole Beauchesne 2014-11-18 17:54:47 UTC
Forgot to mention, "that worked", in the sense that vaapidecode got picked and worked in that condition, as I could assess by GST_DEBUG=vaapi:5 that printed out a huge amount of vaapi related logs.
Comment 16 Sebastian Dröge (slomo) 2014-11-18 17:59:38 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > You only need to ensure that it works properly with video/x-raw(sysmem) and
> > also works properly without GstVideoMeta. That is, it should be possible to
> > read-write map the memory!
> 
> Does this mean we might need to perform in-place postprocessing of decoded
> surfaces? wrt. to comment about R/W map the memory.

It means that downstream elements must be able to read and/or write to the memory, without vaapidecode interfering with that... so that the modified data arrives further downstream.

What do you mean with in-place postprocessing?
Comment 17 Gwenole Beauchesne 2014-11-18 18:02:49 UTC
(In reply to comment #16)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > You only need to ensure that it works properly with video/x-raw(sysmem) and
> > > also works properly without GstVideoMeta. That is, it should be possible to
> > > read-write map the memory!
> > 
> > Does this mean we might need to perform in-place postprocessing of decoded
> > surfaces? wrt. to comment about R/W map the memory.
> 
> It means that downstream elements must be able to read and/or write to the
> memory, without vaapidecode interfering with that... so that the modified data
> arrives further downstream.
> 
> What do you mean with in-place postprocessing?

IMHO, read *and* write to the memory could only occur for "in-place postprocessing", i.e. modifying the same input buffer. Otherwise, most use cases would be either reading from the memory, or writing to it.

Put it differently, in which cases, do we actually need to read and write to the same memory handed over by the upstream element?
Comment 18 Sebastian Dröge (slomo) 2014-11-18 18:04:34 UTC
Any downstream element that does in-place processing, yes. Like some video filter that adds some fancy effect to the video :)

Or textoverlay would actually be another case, as it blends the overlay in-place into the video frames.
Comment 19 Gwenole Beauchesne 2014-11-19 06:22:42 UTC
(In reply to comment #18)
> Any downstream element that does in-place processing, yes. Like some video
> filter that adds some fancy effect to the video :)
> 
> Or textoverlay would actually be another case, as it blends the overlay
> in-place into the video frames.

That was indeed not tested with those. :)

For R/W mappings, I still believe in approach described in bug #704083 at the expense of updating a few plugins along the way. Meanwhile, we could offer R/W mapping functionality at the gst-vaapi level, but with some more round-trips.
Comment 20 Gwenole Beauchesne 2014-11-19 06:23:22 UTC
Review of attachment 290902 [details] [review]:

OK
Comment 21 Sebastian Dröge (slomo) 2014-11-19 08:52:59 UTC
Well, if you don't support read-write mapping properly you're not going to break all applications that use decodebin :)
Comment 22 Sebastian Dröge (slomo) 2014-11-19 09:11:36 UTC
(In reply to comment #21)
> ... you're not going ...

The "not" should obviously be "now" :)
Comment 23 Gwenole Beauchesne 2014-11-19 09:31:33 UTC
(In reply to comment #22)
> (In reply to comment #21)
> > ... you're not going ...
> 
> The "not" should obviously be "now" :)

Well, OK. The gst-vaapi level workaround is actually a two liner. :)

However, trying to optimize somewhat more the process is possible but less trivial. Read: more changes to track more states and commit back to VA surface when needed.
Comment 24 Tim-Philipp Müller 2016-05-28 18:20:00 UTC
This (the original issue at least) seems to work fine these days.

Please re-open if there's still an issue.