[vlc-devel] [PATCH 1/4] libvlc: Add "fullscreen-monitor" setting

[Alexandre Janniaux]

Hi,

I'm sorry for late answer, I couldn't find an adequate time
to write it.

On Wed, Oct 16, 2019 at 10:58:29PM +0300, Rémi Denis-Courmont wrote:
> Le keskiviikkona 16. lokakuuta 2019, 22.27.30 EEST Alexandre Janniaux a écrit
> :
> > On Wed, Oct 16, 2019 at 10:11:33PM +0300, Rémi Denis-Courmont wrote:
> > > Le keskiviikkona 16. lokakuuta 2019, 21.46.26 EEST Alexandre Janniaux a
> > > écrit
> > >
> > > It's not a UX patch. This specific patch out of the series is touching the
> > > core, in a way that contradicts the existing Wayland code (XDG-shell), the
> > > reference model (i.e. audio output device) and the agreed upon design of
> > > the affected feature.
> >
> > It adds a parameter that the user can directly provide, I
> > hardly see how this can be closer to UX.
>
> No it does not. As already noted, there are no ways to implement that
> parameter correctly in the core. The parameter should and already does exist
> in the window provider, where we can have enumeration callbacks and address
> any (other) platform-specific aspect.

I don't see how enumeration and libvlc core variable are
incompatible, but again, in case you do that, how do you
use this variable from outside of the module, meaning here
vout_SetFullscreen or the Qt UI ?

> > > Just like audio output devices. Also did I mention it's just like audio
> > > output devices?
> >
> > I brought examples on why it wasn't compatible, because you
> > don't select the fullscreen device from a menu during the
> > playback
>
> Uh, I don't know how your player does it. But VLC has a fullscreen option in
> the video menu. And FWIW, the system menu on my WM (KWin) has a "send to
> screen" list which contains exactly the list of monitors, not at all unlike
> the list of audio output devices in VLC.
>

Then you probably have a customized version of VLC because
mine doesn't have a menu for choosing a fullscreen device,
neither from right click nor in the main menubar.

> I certainly would not want to have to type in the identifier of the monitor in
> the VLC preferences - which is what this patch proposes.

The fact it is a VLC variable doesn't mean you must type
the identifier of the monitor by hand. You can have the
interface gather the different vout_window_ReportOutput and
display them to the user via a dropdown list.

Typing the device is -- by the way -- the current way for
setting the fullscreen device on the window module which
support it. Other modules are not configurable, except Qt
one which has its own parameter and is the source of a lot
of confusion when the video is started non-embedded.

It is the same for audio devices, you can't set anything in
the normal preferences and even pulseaudio don't display
anything.

Also, while you can set alsa device while running
mainly a pulseaudio audio output, you cannot select a
wayland output if your window provider is an X11 one, so
you can't even make use of the configuration variable per
module and can only configure the one from your window
provider, which you cannot choose -- in opposition with the
audio device that you can choose.

> > and the underlying audio multiplexer already has
> > the correct preference most of the time,
>
> Based on what metric?
>
> Most audio HALs only have a system-wide default. Only MMDevice and PulseAudio
> track the device preference per session.

I thought that, by mentioning the main audio output used by
users is probably Pulseaudio (or MMDevice if you want for
windows, although I wrongly mentioned wasapi instead), it
would be clear that I spoke about pulseaudio-like
multiplexers. Others are for most of them not an audio
multiplexer, except jack but it is out of subject.

I' sorry it wasn't clear to you, but my main point is that
handling video fullscreen device is different from handling
audio devices. Even the VLC interface tells you that by
exposing a right-click menu for audio devices but not for
fullscreen devices like I said.

> > while the video compositor usually doesn't
>
> [Citation needed]
>
> Some window managers remember which application was on which output. Some
> don't. But most if not all have a default output. Indeed, VLC has relied on
> the notion of default output for 20 years.

Yeah, that's what I wrote, but your interface goes on the
default output too in this case, and so you completely
loose the benefit of being able to choose your fullscreen
output. I think I'm not wrong saying that users generally
have multiple screen at the same time but only one audio
output in this very situation.

I agree that some WM like yours -- KDE -- are able to store
preferences for application. I don't know how it works with
multiple window programs (non-embed window) but it it
probably correct for this use case.

However, I'm pretty sure the behaviour is wrong when
multiple video track are used, or in general multiple
windows. And the main difference is that you can put
multiple audio stream to the same multiplexed audio device
but you can't put multiple fullscreen window to the same
fullscreen output without having usability issues (or
please share your window manager, it's probably really
useful).

I've been using the splitter a lot for demo and used an
hack to define an instance name per window and assign them
to predefined output with my WM (i3wm) but in this case I'm
fine with the setting, and the situation will be better
after the vout_window api would be implemented completely
by every window modules. So it is mostly about the case
above for your argument.

>
> > and you set the setting for the most of the time usage.
>
> Same as audio output.

Sure, but sometimes your change because with audio you have
headphones. You don't usually plug/unplug screens in a
normal scenario usage because it is just not convenient at
all. I'm not expert in UX though, I don't know if you are.
But I hope my common sense is good enough to distinguish
between both use cases.

>
> > You didn't bring any argument against this,
>
> Oh really?
>

I'm spending time to take your arguments into account and
write detailed answers about every points. In return, you
didn't answer any of my question about __your__
implementation, which is the blocking point for the author
of this patch. If you answer it and it works, we stop
loosing time debating on everyone's DE and start merging
some code which fixes the usability issues. But if you
can't answer, I don't see why we would go your way.

I've shown use case examples, detailed the implementation
and I'm ready to taking time to explain any details or
provide work on any missing points that anybody can found
on it to make progress on this, even incrementally.

> "The values depend on the output plugin,"

That's why vout_window_ReportOutput reports the value to
the core. If the window provider change, either the names
are compatible or it's just like an unplugged output.

> "this can't be a core setting since the value, and especially the enumeration
> callback for possible values, depends on the window provider plugin"

That's why we have vout_window_ReportOutput too.

> "This specific patch (...) contradicts the existing Wayland code (XDG-shell)"

It probably does, but anyway your vlc variable in xdg-shell
cannot be set from the terminal in a user-friendly way, and
is an integer while SetFullscreen takes screen id as string.
At the same time, your SetFullscreen converts the string id
into an integer, which makes it a pretty unfriendly UX as
you never pass useful information for the UI settings,
despite wl_output exposing model and vendor infos.

I guess it should move to xdg-output soon too, so as to
give ids and informations actually usable by the users.

Also, as I wrote above, user preference shouldn't be handle
by window providers. It's a platform layer and the function
vout_window_SetFullscreen(wnd, NULL) should do the most
natural thing for setting the window to fullscreen. If I
had to give an heuristic, I would say that it would be
setting the window state as fullscreen on the current
output it overlaps the most. Not depending on user
preferences. If you really want that, I believe we need an
additional layer for this, like window vs vout_window, but
currently a window IS a vout_window already.

> Meanwhile, you did not explain how a core setting is going to work. You can't
> seriously be suggesting that users type in the monitor identifier in the VLC
> preferences... so much for being "compatible with a correct UX".

It's right, I hope I've fullfilled this part with my two
posts. Let me know if something else is missing for you to
understand my point or if you don't understand what is
asked about your points.

Regards,
--
Alexandre Janniaux
Videolabs