[vlc-devel] [PATCH] Send fullscreen video to specific monitor

Alexandre Janniaux ajanni at videolabs.io
Thu Oct 3 21:49:05 CEST 2019


Hi,

Thank you very much for fixing this !

Actually, your patch is changing too much things at once:
+ Qt configuration
+ new variable added
+ windows implementation of SetFullscreen
We like to have atomic patches for those.

The previous qt-fullscreen-screennumber is a relic of xinerama's and Qt
implementation which were using numbers. Since then, xinerama has been replaced
by XRandr and Qt has support for QScreen, and both are able to use names which
have the convenient property of being user-friendly and predictable.

You should first add the new variable fullscreen-monitor (or
fullscreen-monitor-name but it seemed a bit long for me).

Then expose and implement the setting in Qt, using Qt's implementation and the
compatible names.

Finally, as a side patch, implement the fullscreen control for windows. This
time, you have to rely on the `id` instead of relying on your own mechanism,
because the window implementation is UI-independant and it is the duty of the
interface or other parent users of the window to place it adequately.

Coming from the X11/Wayland world, I find the fullscreen implement a bit odd
though, it seems to me that it would be the wrong way to fix this and that the
window module isn't correctly resizing, or on the correct screen, or properly
calling vout_window_ReportSize when being resize. I believe you still need the
previous implementation in case you just want the window to become fullscreen on
the current screen it is attached to, which is the expected behaviour when
calling vout_window_SetFullscreen with a null monitor id.

I've done some cleaning for the previous variable which introduced a
fullscreen-monitor like variable with correct monitor settings in Qt, but it's
not as clean as I would like to so feel free to clean yours. I'll rebase on top
of it if I have additional modifications. Your window.c implemention for
fullscreen can be sent as soon as it is ready, because it is a different matter
than configuring it for the end-user.

I hope I didn't get you lost into those small details.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Oct 03, 2019 at 01:19:12PM -0400, Gabriel Luci wrote:
> (Windows only) This adds a new configuration setting to specify which monitor
> to display non-embedded fullscreen video on. This allows a "presentation" type
> setup, where the controls remain on the primary monitor, and the video is
> displayed fullscreen on a secondary monitor (for example, to an audience).
>
> This functionality was availble only in the DirectDraw module until it was
> removed. However, even then, it did not work properly when a second monitor
> had a different resolution (https://trac.videolan.org/vlc/ticket/4659), so if
> this is accepted, I'd like to backport this to 3.0.9.
>
> This implementation is agnostic to the output module used (Direct3D, OpenGL,
> etc.) since it just moves the window.
>
> Currently (without this patch), the "Fullscreen video device" setting in the
> simple preferences only changes the "qt-fullscreen-screennumber" setting, which
> only affects embedded video and has no effect when not embedded. This
> piggy-backs off of that same "Fullscreen video device" setting in the simple
> prefs, saving the display device name to the new "fullscreen-monitor-name"
> setting. Then, when changing to fullscreen mode, the monitor is looked up and
> used. If the monitor cannot be found (has been disconnected, or the setting is
> just set wrong) it falls back to the monitor that the window is already on.
>
> This currently only applies to Windows (#ifdef _WIN32), but the possibility is
> there to expand it to the other OS's. The SetFullscreen functions would just
> need to be updated to make use of the new "fullscreen-monitor-name" setting.
>
> ---
>  .../gui/qt/components/simple_preferences.cpp  |  9 +++
>  modules/video_output/win32/window.c           | 74 +++++++++++++++++--
>  src/libvlc-module.c                           | 10 +++
>  3 files changed, 87 insertions(+), 6 deletions(-)
>
> diff --git a/modules/gui/qt/components/simple_preferences.cpp b/modules/gui/qt/components/simple_preferences.cpp
> index f80c48cc1b..284b144e16 100644
> --- a/modules/gui/qt/components/simple_preferences.cpp
> +++ b/modules/gui/qt/components/simple_preferences.cpp
> @@ -1095,6 +1095,15 @@ void SPrefsPanel::apply()
>      {
>          int i_fullscreenScreen =  qobject_cast<QComboBox *>(optionWidgets["fullscreenScreenB"])->currentData().toInt();
>          config_PutInt( "qt-fullscreen-screennumber", i_fullscreenScreen );
> +#ifdef _WIN32
> +        if (i_fullscreenScreen >= 0)
> +        {
> +            QByteArray s_fullscreenScreen = qobject_cast<QComboBox *>(optionWidgets["fullscreenScreenB"])->currentText().toUtf8();
> +            config_PutPsz( "fullscreen-monitor-name", s_fullscreenScreen);
> +        } else {
> +            config_PutPsz( "fullscreen-monitor-name", "");
> +        }
> +#endif
>          break;
>      }
>
> diff --git a/modules/video_output/win32/window.c b/modules/video_output/win32/window.c
> index 50e172b9b4..4af120f0dd 100644
> --- a/modules/video_output/win32/window.c
> +++ b/modules/video_output/win32/window.c
> @@ -37,6 +37,7 @@
>  #include <vlc_actions.h>
>
>  #include <shellapi.h>                                         /* ExtractIcon */
> +#include <vlc_charset.h>                                      /* FromWide */
>
>  #define RECTWidth(r)   (LONG)((r).right - (r).left)
>  #define RECTHeight(r)  (LONG)((r).bottom - (r).top)
> @@ -153,6 +154,35 @@ static void SetTitle(vout_window_t *wnd, const char *title)
>      PostMessage( sys->hwnd, WM_VLC_CHANGE_TEXT, 0, 0 );
>  }
>
> +typedef struct enum_monitors_callback_params_t {
> +    bool is_monitor_found;
> +    char *psz_name_to_find;
> +    MONITORINFOEX *found_monitor_info;
> +} enum_monitors_callback_params_t;
> +
> +static BOOL CALLBACK EnumDisplayMonitorsCallback(HMONITOR hMonitor, HDC hdcMonitor, LPRECT lprcMonitor, LPARAM dwData)
> +{
> +    /* this is called for each active monitor */
> +
> +    VLC_UNUSED(hdcMonitor);
> +    VLC_UNUSED(lprcMonitor);
> +
> +    MONITORINFOEX mi;
> +    mi.cbSize = sizeof(MONITORINFOEX);
> +    if (!GetMonitorInfo(hMonitor, (LPMONITORINFO) &mi)) return true;
> +
> +    enum_monitors_callback_params_t *params = (enum_monitors_callback_params_t*) dwData;
> +    char *psz_monitor_name = FromWide(mi.szDevice);
> +    if (!strcmp(psz_monitor_name, params->psz_name_to_find))
> +    {
> +        /* the monitor name matches what we're looking for */
> +        params->is_monitor_found = true;
> +        params->found_monitor_info = &mi;
> +    }
> +    free(psz_monitor_name);
> +    return !params->is_monitor_found;
> +}
> +
>  static void SetFullscreen(vout_window_t *wnd, const char *id)
>  {
>      VLC_UNUSED(id);
> @@ -167,18 +197,50 @@ static void SetFullscreen(vout_window_t *wnd, const char *id)
>      /* Change window style, no borders and no title bar */
>      SetWindowLong(sys->hwnd, GWL_STYLE, WS_CLIPCHILDREN | WS_VISIBLE);
>
> -    /* Retrieve current window position so fullscreen will happen
> -     * on the right screen */
> -    HMONITOR hmon = MonitorFromWindow(sys->hwnd, MONITOR_DEFAULTTONEAREST);
> -    MONITORINFO mi;
> -    mi.cbSize = sizeof(MONITORINFO);
> -    if (GetMonitorInfo(hmon, &mi))
> +    /* Figure out which monitor we should be using */
> +    MONITORINFOEX mi;
> +    bool is_monitor_found = false;
> +
> +    char *psz_monitor_name = var_InheritString( wnd, "fullscreen-monitor-name" );
> +    if (psz_monitor_name)
> +    {
> +        /* Config tells us to use a specific monitor, so see if we can find an
> +         * active monitor with the same name */
> +        enum_monitors_callback_params_t params = {
> +            .is_monitor_found = false,
> +            .psz_name_to_find = psz_monitor_name
> +        };
> +        EnumDisplayMonitors(NULL, NULL, &EnumDisplayMonitorsCallback, (LPARAM) &params);
> +
> +        if (params.is_monitor_found)
> +        {
> +            is_monitor_found = true;
> +            mi = *(params.found_monitor_info);
> +        }
> +        else
> +        {
> +            msg_Dbg(wnd, "display device with name \"%s\" was requested by config, but no such device exists", psz_monitor_name);
> +        }
> +    }
> +
> +    if (!is_monitor_found)
> +    {
> +        /* fall back to using the screen the window is currently mostly on */
> +        HMONITOR hmon;
> +        hmon = MonitorFromWindow(sys->hwnd, MONITOR_DEFAULTTONEAREST);
> +        mi.cbSize = sizeof(MONITORINFOEX);
> +        is_monitor_found = GetMonitorInfo(hmon, (LPMONITORINFO) &mi);
> +    }
> +
> +    if (is_monitor_found)
>          SetWindowPos(sys->hwnd, 0,
>                       mi.rcMonitor.left,
>                       mi.rcMonitor.top,
>                       RECTWidth(mi.rcMonitor),
>                       RECTHeight(mi.rcMonitor),
>                       SWP_NOZORDER|SWP_FRAMECHANGED);
> +
> +    free(psz_monitor_name);
>  }
>
>  static void UnsetFullscreen(vout_window_t *wnd)
> diff --git a/src/libvlc-module.c b/src/libvlc-module.c
> index d3da4e5fb8..38564f958e 100644
> --- a/src/libvlc-module.c
> +++ b/src/libvlc-module.c
> @@ -314,6 +314,13 @@ static const char *const ppsz_align_descriptions[] =
>  #define FULLSCREEN_LONGTEXT N_( \
>      "Start video in fullscreen mode" )
>
> +#ifdef _WIN32
> +#define FULLSCREEN_MONITOR_NAME_TEXT N_("Display device for fullscreen video")
> +#define FULLSCREEN_MONITOR_NAME_LONGTEXT N_( \
> +    "In a multiple monitor configuration, you can specify the device name " \
> +    "of the display that you want the fullscreen video window to open on." )
> +#endif
> +
>  #define VIDEO_ON_TOP_TEXT N_("Always on top")
>  #define VIDEO_ON_TOP_LONGTEXT N_( \
>      "Always place the video window on top of other windows." )
> @@ -1641,6 +1648,9 @@ vlc_module_begin ()
>      add_bool( "fullscreen", false, FULLSCREEN_TEXT, FULLSCREEN_LONGTEXT, false )
>          change_short('f')
>          change_safe ()
> +#ifdef _WIN32
> +    add_string( "fullscreen-monitor-name", "", FULLSCREEN_MONITOR_NAME_TEXT, FULLSCREEN_MONITOR_NAME_LONGTEXT, false )
> +#endif
>      add_bool( "embedded-video", 1, EMBEDDED_TEXT, EMBEDDED_LONGTEXT,
>                true )
>      add_bool( "xlib", true, "", "", true )
> --
> 2.23.0.windows.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list