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 793271 - v4l2object: Check for all allocators and pools in query
v4l2object: Check for all allocators and pools in query
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 793270
Blocks: 792034
 
 
Reported: 2018-02-07 18:23 UTC by Víctor Manuel Jáquez Leal
Modified: 2018-11-03 15:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v4l2object: Check for all allocators and pools in query (2.65 KB, patch)
2018-02-07 18:23 UTC, Víctor Manuel Jáquez Leal
none Details | Review
v4l2object: Check for all allocators and pools in query (1.98 KB, patch)
2018-02-08 09:06 UTC, Víctor Manuel Jáquez Leal
needs-work Details | Review

Description Víctor Manuel Jáquez Leal 2018-02-07 18:23:20 UTC
See commit description.
Comment 1 Víctor Manuel Jáquez Leal 2018-02-07 18:23:23 UTC
Created attachment 368084 [details] [review]
v4l2object: Check for all allocators and pools in query

When the io-mode is dmabuf-import is important to look for an
allocator and pool that supports dmabuf, either a dmabuf descendant
allocator or a pool with the GST_BUFFER_POOL_OPTION_VIDEO_DMABUF
config.
Comment 2 Nicolas Dufresne (ndufresne) 2018-02-07 18:47:37 UTC
Ok, I see why you wanted this in 793270 now. There was an open question if it was ever correct to add a DMABuf allocator (for the case there is no alloc/custom) in the allocation query. I think if the answer is no, then until we have a non-custom dmabuf allocator, this is a bit speculative.

For the pool option, well, an option is supposed to be explicitly enabled by the user. So the usage is a bit of a clash if enabling it have no effect (it's already enabled).
Comment 3 Víctor Manuel Jáquez Leal 2018-02-07 18:52:51 UTC
(In reply to Nicolas Dufresne (stormer) from comment #2)
> Ok, I see why you wanted this in 793270 now. There was an open question if
> it was ever correct to add a DMABuf allocator (for the case there is no
> alloc/custom) in the allocation query. I think if the answer is no, then
> until we have a non-custom dmabuf allocator, this is a bit speculative.

Agree, a dmabuf allocator with a custom alloc, as the gstreamer-vaapi case, might no be right to share it. I can remove it from the query in gstreamer-vaapi :)

> For the pool option, well, an option is supposed to be explicitly enabled by
> the user. So the usage is a bit of a clash if enabling it have no effect
> (it's already enabled).

If enabled and downstream doesn't provide a dmabuf-enable pool or allocator, it fails.
Comment 4 Nicolas Dufresne (ndufresne) 2018-02-07 18:57:10 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #3)
> (In reply to Nicolas Dufresne (stormer) from comment #2)
> > Ok, I see why you wanted this in 793270 now. There was an open question if
> > it was ever correct to add a DMABuf allocator (for the case there is no
> > alloc/custom) in the allocation query. I think if the answer is no, then
> > until we have a non-custom dmabuf allocator, this is a bit speculative.
> 
> Agree, a dmabuf allocator with a custom alloc, as the gstreamer-vaapi case,
> might no be right to share it. I can remove it from the query in
> gstreamer-vaapi :)

And looking at the implementation, it's not used.

> 
> > For the pool option, well, an option is supposed to be explicitly enabled by
> > the user. So the usage is a bit of a clash if enabling it have no effect
> > (it's already enabled).
> 
> If enabled and downstream doesn't provide a dmabuf-enable pool or allocator,
> it fails.

Make sense. Shall we add code to enable it though ?
Comment 5 Víctor Manuel Jáquez Leal 2018-02-08 09:06:03 UTC
Created attachment 368132 [details] [review]
v4l2object: Check for all allocators and pools in query

When the io-mode is dmabuf-import is important to look for a
pool that supports dmabuf, that is, a buffer pool with
GST_BUFFER_POOL_OPTION_VIDEO_DMABUF config.
Comment 6 Víctor Manuel Jáquez Leal 2018-02-08 09:09:45 UTC
(In reply to Nicolas Dufresne (stormer) from comment #4)
> (In reply to Víctor Manuel Jáquez Leal from comment #3)
> > Agree, a dmabuf allocator with a custom alloc, as the gstreamer-vaapi case,
> > might no be right to share it. I can remove it from the query in
> > gstreamer-vaapi :)
> 
> And looking at the implementation, it's not used.

Removed. I have updated the patches for vaapi pool to ignore any non-vaapi allocator and try to reuse the already set allocator, if there's one.

> > > For the pool option, well, an option is supposed to be explicitly enabled by
> > > the user. So the usage is a bit of a clash if enabling it have no effect
> > > (it's already enabled).
> > 
> > If enabled and downstream doesn't provide a dmabuf-enable pool or allocator,
> > it fails.
> 
> Make sense. Shall we add code to enable it though ?

IMO, if the user request a specific io-mode an it's not achieved, the pipeline should fail.
Comment 7 Nicolas Dufresne (ndufresne) 2018-03-03 01:10:31 UTC
Review of attachment 368132 [details] [review]:

The main problem with this patch is that it's not backward compatible. We have kmssink that export a pool the produce DMABuf, without modifying kmssink, the following pipeline will not work any-more:

  v4l2src io-mode=dmabuf-import ! kmssink

This is because the code will no longer consider any pool that does not have the new option.
Comment 8 GStreamer system administrator 2018-11-03 15:25:59 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/437.