[Bug 711155] wayland: add wl_drm support to wayland sink

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=711155

--- Comment #44 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
(In reply to Benjamin Gaignard from comment #43)
> No we track which allocator has been selected in pool set_config() 

I mean tracking the life time of a memory instead of a buffer. This is very
basic error in this GstBufferPool/GstAllocator world.

> Maybe we have put to much #ifdef HAVE_WAYLAND_DMABUF but at least it
> indicate us where we have adding code.

I might be in disbelief, but to me it's impossible that a modern IPC like
Wayland suggest can't let you check what is supported at run-time. It's really
unfortunate if you have to rebuild GStreamer for every compositor. I'd like
someone that know to strongly asset this before adding any kind of ifdef in the
code.

> 
> > ::: ext/wayland/gstwaylandsink.c
> > @@ +586,3 @@
> >      config = gst_buffer_pool_get_config (pool);
> >      gst_buffer_pool_config_set_params (config, caps, size, 2, 0);
> > +    gst_buffer_pool_config_set_allocator (config, NULL, &params);
> > 
> > Why bother passing allocation params if you don't have any alignment
> > requirement ?
> 
> because gst_buffer_pool_config_set_allocator() do not accept to have the
> both parameters NULL and was needed for pool initialization.

Ok, so this is a hack instead of fixing the assertion, This is a need-work
then.

> 
> > @@ +78,3 @@
> > +    memset (&destroy_arg, 0, sizeof destroy_arg);
> > +    drmPrimeFDToHandle (meta->drm_fd, prime_fd, &destroy_arg.handle);
> > +    drmIoctl (meta->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_arg);
> > 
> > What if the dmabuf was not DRM dumb ?
> 
> In your case dmabuf always means dumb buffer.
> It is a short cut we have done for your implementation but we can imagine
> have something like v4l2 plugins io-mode to select allocation method.

This is exactly the kind of implicit limitation and hacks I'm trying to move
away from.

>  
> > 
> > ::: ext/wayland/wlvideoformat.c
> > @@ +63,3 @@
> > +#ifdef HAVE_WAYLAND_DMABUF
> > +  {WL_DMABUF_FORMAT_XRGB8888, GST_VIDEO_FORMAT_BGRx},
> > +  {WL_DMABUF_FORMAT_ARGB8888, GST_VIDEO_FORMAT_BGRA},
> > 
> > And no YUV format ? Without any YUV format, it's a bit pointless to have
> > this in GStreamer.
> Of course we have YUV formats but it is an other patch and link to dmabuf
> usage.

So this patchset is partial ?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.