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

[Alexandre Janniaux]

Hi, just so you don't get me wrong, I don't care if we
implement a right click menu for the interface like on
MacOSX. However, I do care that it gets a simple setting in
the location where Qt already exposes the list of
fullscreen devices, and that it gets a sane behaviour for
fullscreen in situation where it's not controled by the
user, but only the SetFullscreen API.

I hope this is clear,

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Oct 24, 2019 at 11:19:20PM +0200, Alexandre Janniaux wrote:
> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel