[vlc-devel] [PATCH 2/4] vout: Use "fullscreen-monitor" setting

[Alexandre Janniaux]

Hi,

I don't really see how that break the semantics as the
screen variable can be null. Do you have something else
in mind for this use case? You seem to disapproving the
fact that the window could be set to fullscreen on a
predefined monitor, which would be weird so I'm probably
misunderstanding.

I have other questions related to your others review, so
I'll post there.

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Oct 14, 2019 at 10:02:06AM +0200, Thomas Guillem wrote:
>
>
> On Sun, Oct 13, 2019, at 09:18, Rémi Denis-Courmont wrote:
> > Le sunnuntaina 13. lokakuuta 2019, 6.58.29 EEST Gabriel Luci a écrit :
> > > Pass the setting as the id parameter to vout_ChangeFullscreen()
> > > ---
> > >  src/video_output/vout_intf.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/video_output/vout_intf.c b/src/video_output/vout_intf.c
> > > index ce50616fc9..9a0120fb4d 100644
> > > --- a/src/video_output/vout_intf.c
> > > +++ b/src/video_output/vout_intf.c
> > > @@ -566,7 +566,11 @@ static int FullscreenCallback( vlc_object_t *p_this,
> > > char const *psz_cmd, (void)psz_cmd; (void) oldval; (void)p_data;
> > >
> > >      if( newval.b_bool )
> > > -        vout_ChangeFullscreen(p_vout, NULL);
> > > +    {
> > > +        char *psz_fullscreen_monitor = var_InheritString( p_vout,
> > > "fullscreen-monitor" ); +        vout_ChangeFullscreen(p_vout,
> > > psz_fullscreen_monitor);
> > > +        free( psz_fullscreen_monitor );
> > > +    }
> > >      else
> > >          vout_ChangeWindowed(p_vout);
> > >      return VLC_SUCCESS;
> >
> > This breaks the API definition and intended semantics of passing NULL there -
> > let the window manager pick the most suitable default.
>
> There is no documentation at all about this id parameter.
> Personally, I would have do like Gabriel here.
>
>
> >
> > --
> > 雷米‧德尼-库尔蒙
> > 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