[RFC] Add wayland support for MPlayer2

[Alexander Preisinger]

Sorry for the late answer.

2012/8/24 Uoti Urpala <uoti.urpala at pp1.inet.fi>:
> On Fri, 2012-08-24 at 08:08 +0200, Alexander Preisinger wrote:
>> This patch series will enable support for wayland via EGL.
>> Input and output works correctly. But there is no window decoration.
>>
>> Please have look and tell what should be changed or improved.
>
> This was already very briefly discussed on IRC, but I'll write a reply
> here now in a bit more detail.
>
> The first main issue is that I can't easily test Wayland code, and I
> haven't heard of others using it either. Adding code like this in
> gl_common.c will cause some problems if there's nobody who's doing
> player development/testing otherwise and would compile/run it. What are
> the actual benefits? Which users would benefit and how?

The actual benefit is that we have a native media player. I wanted to know
how hard or easy it is to port a program to wayland and I choose
mplayer2, because I use it daily.

>
>
> Here are some issues about the patch I noticed (but it's probably not
> worth spending too much time on the particulars now, if it's not clear
> if it'd be applied anyway):
>
> Having a separate configure check for "Wayland headers presence" seems
> completely pointless. Its result isn't used for anything separate.
>
> Does Wayland presence somehow guarantee libxkb presence? Should that not
> be tested separately?
>
> You have "pkg_config_add" but then a comment "#pkg_config_add doesn't
> work" and add some libs manually. What does the .pc file on your system
> add then?

I screwed up the configure check that's why pkg_config_add didn't work.
I also added xkbcommmon to the check and removed the separate check.

>
> Adding "wayland" to the "vomodules" configure variable is wrong, as
> there is no vo_wayland; it's only a backend for vo_gl.
>

I accidentally left it there, because for testing I added another vo module
with shm buffers.

> res_comment="check if the dev(el) packages are installed" shouldn't be
> there; that'd apply equally to almost any library, and Wayland is
> currently one of the things most people don't need to check even if not
> enabled.
>
> +static struct wl_priv wl = {NULL, NULL, NULL, NULL};
> +    /* New wl_common requires to preset it
> +        to zero, but that is a bit too hackish for my taste.
>
> I'm not sure what that comment is about; does it refer to the "NULL"
> initialization above? That initialization does not actually do anything;
> the variable would be zero-initialized by default anyway. Adding a
> static variable to gl_common is quite ugly.
>
> You add a second copy of the appendstr() function.
>
> wm_common.c is compiled if GL_WAYLAND is enabled, but still pointlessly
> tests for "#ifdef CONFIG_GL_WAYLAND".
>

I left it there because it is possible to use different rendering backends
with wayland aside from EGL. But I think it is better to remove the #ifdef
because the GL output will be the only one worth using.

> The vo_wl_check_events() function seems to be unused; instead
> vo_wl_priv_check_events() is called.
>
> What's the deal with the repeat.timer_fd stuff? Does wayland input not
> have native key repeat, so you emulate that manually with one
> arbitrarily picked repeat rate?
>
> Handling of window resize events seems to be missing.
>
>
> I didn't look at most of the actual Wayland code.
>

Thanks for your input. I will try to fix the patches according to your
comments. As for the timer_fd stuff, it was used in weston toytoolkit and
I found no other way to implement repeating keys. I should take more time
to look into xkbcommon. Maybe there is already some native way of doing this.
But as of now xkbcommon, had no official release.


Thanks,

Alexander Preisinger

>
> _______________________________________________
> mplayer2-dev mailing list
> mplayer2-dev at mplayer2.org
> http://lists.mplayer2.org/mailman/listinfo/mplayer2-dev