Review of attachment 369886 [details] [review]:

I cannot make any sense of this commit message. Can you rewrite this, and add a longer explanation ?

::: ext/wayland/gstwaylandsink.c
@@ +585,3 @@
+    config = gst_buffer_pool_get_config (sink->pool);
+    gst_buffer_pool_config_get_params (config, NULL, &size, &min_bufs,
+        &max_bufs);

That make no sense. The pool does not know the caps, how could it know it's buffer size ?

@@ +589,1 @@
+    gst_query_add_allocation_pool (query, sink->pool, size, min_bufs, max_bufs);

sink->pool is private and internal. The only use for this pool is to copy incoming buffers when they are not suitable for the Wayland protocol (not from wayland pool, FD base, or dmabuf). Sharing this pool with upstream is unsafe.

@@ +589,2 @@
+    gst_query_add_allocation_pool (query, sink->pool, size, min_bufs, max_bufs);
+    gst_query_add_allocation_param (query, gst_tizen_wl_shm_allocator_get (),

Right, gst_tizen .... I guess you should submit your patch to the related downstream fork ?

I'm closing this bug report as the attached patch is not valid and no bug description has been provided. When you file bugs, you should first describe the issue you are facing so that your reviewer understand what you are trying to fix. This patch indicate that you might be fixing an issue that only exist in a downstream fork of GStreamer. The patch itself is as describe fully incorrect.

please check if waylandsink can make gstbuffer in the gst_wayland_sink_propose_allocation().
The current master version only make gstbuffer poll int gst_wayland_sink_propose_allocation() and create gstbuffer in the gst_wayland_sink_show_frame() because 'if(!wbuf)' is true and always do gst_vido_frame_copy().

if waylandsink does not want to make gstbuffer everytime, waylandsink use gstbuffer which is made by gst_wayland_sink_propose_allocation().

waylandsink can gst buffer poll in set_caps() so this patch can run.

If wayland sink use wl_buffer widthout creating and gst_vido_frame_copy, the master version has no issue.
but master code can not reach that below code is true.

  if (G_LIKELY (wlbuffer && wlbuffer->display == sink->display)) {
    GST_LOG_OBJECT (sink, "buffer %p has a wl_buffer from our display, "
        "writing directly", buffer);
    to_render = buffer;
    goto render;
  }

This code is reached if upstream uses the proposed pool. To demonstrate this you can run:

Copy Path (not using the pool):
$ GST_DEBUG="GST_PERFORMANCE:5" gst-launch-1.0 videotestsrc ! identity drop-allocation=1 ! waylandsink
...
0:00:01.851560175 15888      0x2178450 DEBUG        GST_PERFORMANCE video-frame.c:372:gst_video_frame_copy_plane: copy plane 0, w:1280 h:240 

Zero-Copy:
GST_DEBUG="GST_PERFORMANCE:5" gst-launch-1.0 videotestsrc ! identity drop-allocation=0 ! waylandsink
...
0:00:02.584972806 15987       0xc0c050 LOG              waylandsink gstwaylandsink.c:680:gst_wayland_sink_show_frame:<waylandsink0> buffer 0x7f6eb4013390 has a wl_buffer from our display, writing directly

Errata: Zero-copy: GST_DEBUG="GST_PERFORMANCE:5,waylandsink:7"

Is below command non-zero-copy, right?
gst-launch-1.0 videotestsrc ! waylandsink

(In reply to Hyunil Park from comment #7)
> Is below command non-zero-copy, right?
> gst-launch-1.0 videotestsrc ! waylandsink

It is zero-copy in GStreamer, videotestsrc uses wayland SHM based proposed allocator and write into these buffers. The SHM is passed zero-copy to the compositor, which of course will likely have to copy to GL, but this is what wayland offers without custom extensions (like Tizen seems to do).

Thank you!!