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

[Gabriel Luci]

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.

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. That
seems to be the place to pass the ID of the monitor to put the fullscreen
window on. 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.

And that's where my confusion is. If it really should be a
window-provider-specific setting, then why does vout_ChangeFullscreen()
have an "id" parameter?

If it should be in the window provider, that's fine, and I'm happy to do
it. There just doesn't seem to be consensus here, so I don't know which
direction to go.

On Thu, Oct 24, 2019 at 11:13 AM Steve Lhomme <robux4 at ycbcr.xyz> wrote:

> On 2019-10-24 16:57, Gabriel Luci wrote:
> > Following up on this: am I correct in my understanding that those are
> > our only 2 options?
>
> IMO there should be only one option. Either you set it in preferences
> before you start playback or you can change dynamically it from the UI.
> The second part it not handled for the moment AFAIK.
>
> > On Thu, Oct 17, 2019 at 5:26 PM Gabriel Luci <github at luci.ca
> > <mailto:github at luci.ca>> wrote:
> >
> >     I do understand the difference between the two, but they are
> >     dependent on each other, no?
> >
> >      From my inexperienced perspective, I see two options:
> >
> >     1. Use a global setting, read it in FullscreenCallback(), and pass
> >     it to vout_ChangeFullscreen() (which is what I did in this patchset)
> >     or,
> >     2. Define the setting in the window provider plugin, but then that
> >     means (correct me if I'm wrong) the window provider plugin also has
> >     the job of reading that setting in its SetFullscreen function. But
> >     then (I think) that means that there is no purpose for the id
> >     parameter in vout_ChangeFullscreen(), vout_window_SetFullScreen(),
> >     and the window provider SetFullscreen functions.
> >
> >     That's why I went with option 1, because I saw the id parameter
> >     there doing nothing, so I figured that's what it was supposed to be
> >     for, which led me to believe a global setting was the only way to go.
> >
> >     On Thu, Oct 17, 2019 at 3:44 PM Rémi Denis-Courmont <remi at remlab.net
> >     <mailto:remi at remlab.net>> wrote:
> >
> >         Le torstaina 17. lokakuuta 2019, 19.26.56 EEST Gabriel Luci a
> >         écrit :
> >          > I'm happy to make that change. However, then I'm not sure how
> >         to use the
> >          > setting. vout_ChangeFullscreen accepts an id parameter, which
> >         is supposed
> >          > to be the "output identifier", which I assume means the ID of
> >         the monitor?
> >          > (correct me if I'm wrong) So how do we find that value when
> >         calling
> >          > vout_ChangeFullscreen if only the window provider knows? Or
> am I
> >          > misunderstanding something there?
> >
> >         I think you're mixing up two things here - changing the
> >         fullscreen output used
> >         by an active window which uses vout_ChangeFullscreen(), versus
> >         changing the
> >         default fullscreen output for future windows which uses window
> >         provider-
> >         dependent mechanism - a setting in Qt and XDG-shell cases.
> >
> >         --
> >         Rémi Denis-Courmont
> >         http://www.remlab.net/
> >
> >
> >
> >         _______________________________________________
> >         vlc-devel mailing list
> >         To unsubscribe or modify your subscription options:
> >         https://mailman.videolan.org/listinfo/vlc-devel
> >
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20191024/390886e9/attachment.html>