Created attachment 302174 [details] [review]
wayland: GTK+ video overlay example

Comment on attachment 302174 [details] [review]
wayland: GTK+ video overlay example

The code looks good. I guess once the waylandsink API stabilizes, we'll want to add this to the various other examples we have instead of having a separate wayland one, but it's probably too early for that.

Also, you probably want to add code to deal with changes in the window size, as this is the annoyingly non-trivial part with wayland. I'd love to be able to convince team wayland to change libwayland-client to make our lives easier, but for now, we're stuck with a complex multi-step process.

Shouldn't "gtk_window_move()" just do nothing on Wayland?

(In reply to Olivier Crête from comment #2)
> The code looks good. 

Thanks!

> I guess once the waylandsink API stabilizes, we'll want
> to add this to the various other examples we have instead of having a
> separate wayland one, but it's probably too early for that.

What other examples? As far as I see, there's a video overlay example in gst-plugin-base, but focused on X. I modified it to support waylandsink, but then I realized gl has their own video overlay example too in gst-plugin-bad, so I decided to follow its steps, since both are in gst-plugin-bad.

IMO, would be good to push tests for waylandsink as soon as possible, so others could dig in the plugin easily. 

> Also, you probably want to add code to deal with changes in the window size,
> as this is the annoyingly non-trivial part with wayland. I'd love to be able
> to convince team wayland to change libwayland-client to make our lives
> easier, but for now, we're stuck with a complex multi-step process.

Yes, I didn't work of that feature. Even the _set_render_rectangle() isn't working as expected (the videotestsrc appears displaced).

> 
> Shouldn't "gtk_window_move()" just do nothing on Wayland?

It seems so.

(In reply to Víctor Manuel Jáquez Leal from comment #3)
> (In reply to Olivier Crête from comment #2)
> > Also, you probably want to add code to deal with changes in the window size,
> > as this is the annoyingly non-trivial part with wayland. I'd love to be able
> > to convince team wayland to change libwayland-client to make our lives
> > easier, but for now, we're stuck with a complex multi-step process.
> 
> Yes, I didn't work of that feature. Even the _set_render_rectangle() isn't
> working as expected (the videotestsrc appears displaced).

Well, because you are not using it properly :)

I also have a gtk demo around: https://git.collabora.com/cgit/user/gkiagia/gst-wayland-gtk-demo.git/tree/main.c
Perhaps it can be merged into yours (yours looks more clean).

You are right that this should be in gst-plugins-bad examples, so that others can easily work with waylandsink.

commit 3eeec2836a8f1ddb5525633f4ae1c360ddc9d3ba
Author: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date:   Mon Oct 26 16:24:40 2015 +0100

    tests/examples: add a waylandsink example

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

(In reply to George Kiagiadakis from comment #4)
> (In reply to Víctor Manuel Jáquez Leal from comment #3)
> > (In reply to Olivier Crête from comment #2)
> > > Also, you probably want to add code to deal with changes in the window size,
> > > as this is the annoyingly non-trivial part with wayland. I'd love to be able
> > > to convince team wayland to change libwayland-client to make our lives
> > > easier, but for now, we're stuck with a complex multi-step process.
> > 
> > Yes, I didn't work of that feature. Even the _set_render_rectangle() isn't
> > working as expected (the videotestsrc appears displaced).
> 
> Well, because you are not using it properly :)

Hi George,

Thanks for taking care of this. It just slipped out of my mind.

I have tried the test, but I see the same problem I had with my code. Let me upload an screenshot...

Created attachment 315820 [details]
screenshot of the window displacement

Ah so it was this bug... I fixed that problem just in the previous commit:

http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=c51fe83d4228262f297382f49676c76cd2f8147e

(In reply to George Kiagiadakis from comment #8)
> Ah so it was this bug... I fixed that problem just in the previous commit:
> 
> http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/
> ?id=c51fe83d4228262f297382f49676c76cd2f8147e

It looks to me improbable, since I did a git pull, so the build has it. Perhaps it is other issue.

(In reply to Víctor Manuel Jáquez Leal from comment #9)
> (In reply to George Kiagiadakis from comment #8)
> > Ah so it was this bug... I fixed that problem just in the previous commit:
> > 
> > http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/
> > ?id=c51fe83d4228262f297382f49676c76cd2f8147e
> 
> It looks to me improbable, since I did a git pull, so the build has it.
> Perhaps it is other issue.

Yes, but did you compile and install waylandsink? The example is not related to this commit. I am pretty sure because I also saw this exact same issue and I am pretty sure I fixed it. Please make sure to recompile everything and in case you still see a problem, please open a new bug report.