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

[GStreamer (GNOME Bugzilla)]

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

--- Comment #88 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
(In reply to Fabien Dessenne from comment #87)
> Nicolas, I have added/fix few things in v3. In top of that, I have two
> remarks on v3:
> - in show_frame, since you removed the "if (sink->pool)" check in
> show_frame, the patch is unconsistent. Indeed set_caps does not ALWAYS
> propose a pool (as described in the commit message: no buffer pool for
> dmabuf). If you really want to remove "if (sink->pool)", then you need some
> rework. I suggest you keep this check, and add a "fixme" or something like
> this

Sorry, I'm not sure I follow you, I believed I have checked the conditions for
the pool checks properly. Can you clarify ?

> - in gst_wayland_sink_set_caps, you start with updating sink->video_info /
> sink->video_info_changed. Then you check for parameters support, which may
> result in "return false": in that case, you have updated video_info while
> you probably should not have. I suggest you keep the initial implementation.

Failing SET_CAPS is fatal, the state of your video-info is no longer important.

> 
> Another remark, we talked about, regarding additional check for
> dmabuf_format support: I think there is no need to update anything, and you
> probably got confused with your hack patch "HACK: Just hackup a format list
> to be able to test". Indeed, before adding a new format to dmabuf_formats
> you shall first check if that format can actually be converted to a known
> GST video format. Just like it is done in dmabuf_format() (wldisplay.c),
> otherwise you will get some assert error (which you talked me about)
> Example (this format will be filtered out);
>   format = DRM_FORMAT_YUV420;
>   if (gst_wl_dmabuf_format_to_video_format (format) !=
> GST_VIDEO_FORMAT_UNKNOWN)
>     g_array_append_val (self->dmabuf_formats, format);

You miss-understood. GStreamer do have a match for all main DRM_FORMAT_*, I'd
like the static map to be complete. The assertion was cause indeed by the hack,
but would occure if the remote compositor sent a FOURCC that is unsupported.
That because gst_video_format_to_string() does not accept UNKNOWN.

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