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

[Alexandre Janniaux]

Hi,

On Fri, Oct 25, 2019 at 01:36:35PM +0200, Steve Lhomme wrote:
> On 2019-10-24 21:33, Rémi Denis-Courmont wrote:
> > Le torstaina 24. lokakuuta 2019, 18.39.02 EEST Gabriel Luci a écrit :
> > > I think you misunderstood my question. This entire patchset was about
> > > setting it in the preferences before playback starts. There should be one
> > > *setting*, yes. But, from what I see, there are two ways to implement it.
> > > (That's what I meant by "2 options")
> > >
> > > In this patch, I defined a global setting in src/libvlc-module.c. Rémi
> > > objected to that approach and said the setting should be defined in the
> > > window manager (I assume that means in win32/window.c). But if it's defined
> > > there, the setting should be *read* in win32/window.c too.
> >
> > I've never written that it should be in the window manager. That makes no
> > sense. It has to be in the window provider, which is the only component that
> > knows how to enumerate outputs. I've already explained at least three times in
> > this thread why it cannot be in the core.
> >
> > > My objection to that is that Rémi himself had added an "id" parameter to
> > > the vout_ChangeFullscreen() last year, which currently is always NULL.
> >
> > Because the UI does not use it yet. Feel free to fix that.
> >
> > > That seems to be the place to pass the ID of the monitor to put the
> > > fullscreen window on.
> >
> > That's kind of the whole point, yes.
> >
> > > If I'm correct on that, then the setting needs to be read in
> > > src/video_output/vout_intf.c and thus it should be a global setting.
> >
> > WTF? How do you handle enumeration with a global setting? How do you handle
> > different windowing systems in a global setting? How do you not break the
> > existing XDG-shell implementation?
>
> For the record, the capture I sent with the current option in QT is listing
> the possible screens using QGuiApplication::screens(). The text presented is
> the QScreen::name(). It may not be parseable by the standalone window
> (win32/window.c in this case) and may change depending on the Qt versions.
> The documentation doesn't mention any similarity with anything existing in
> the OS.
>
> If we want a stable string we'll have to mimick what Qt does on systems
> where we use Qt (Windows and Linux). I doesn't seem to hard on Windows. I
> don't know about others.
>
> The other option would be to not use QGuiApplication::screens() but
> something else, maybe in the core. We could enumerate screen like we do for
> audio devices. Which uses "XXX-audio-device" to find the list of devices
> when the XXX output is selected. Here the XXX selection would be the GUI or
> a standalone window.
>

Yes; vout_window_ReportOutput will do this but we need:

+ to choose where to store it. Remi's implementation tells
    it should be in the window provider module where this
    id makes sense, I tell it should be in a core setting
    or the interface one for every modules, so that setting
    it has really good UI and the output is seen as
    unplugged in the rare cases where you switch window
    provider (as a reminder, meaning that you switch your
    whole environment because you can't choose the window
    module in VLC).

+ to choose when to read it. Remi's implementation tells it
    should be read in the window provider module where the
    id makes sense. I say it should be read from the
    interface or anything different from the window
    provider, as it would break multi-output pipeline.

I'm fine with merging any solution for this patchset, but
this must be discussed afterwards as I don't wan to break
neither these multi-output use cases nor the simple
preference window.

Regards,
--
Alexandre Janniaux
Videolabs

> The current selection implies that the Fullscreen Video Device is only used
> when the video is embedded in Qt.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel