[vlc-devel] [PATCH] vout: macosx: add standalone window provider
Alexandre Janniaux
ajanni at videolabs.io
Thu Nov 12 14:06:58 CET 2020
Hi,
Review does not take into account the remark you've mentioned
afterwards.
Since the window will be closed asynchronously within the main
thread, you need a lock to be able to remove the VLC window from
the window implementation and secure the call to the VLC window.
If not a lock, you could use a dispatch queue but given it's a
lock + condition + state, a lock might be a better fit.
On Fri, Oct 23, 2020 at 10:35:45AM +0200, Marvin Scholz wrote:
> ---
> modules/video_output/Makefile.am | 8 +-
> modules/video_output/window_macosx.m | 480 +++++++++++++++++++++++++++
> 2 files changed, 487 insertions(+), 1 deletion(-)
> create mode 100644 modules/video_output/window_macosx.m
>
> diff --git a/modules/video_output/Makefile.am b/modules/video_output/Makefile.am
> index c42fdc474a6..43464d8f8b1 100644
> --- a/modules/video_output/Makefile.am
> +++ b/modules/video_output/Makefile.am
> @@ -27,6 +27,12 @@ libglinterop_cvpx_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(voutdir)' \
> -Wl,-framework,Foundation,-framework,CoreVideo
>
> if HAVE_OSX
> +libwindow_macosx_plugin_la_SOURCES = video_output/window_macosx.m
> +libwindow_macosx_plugin_la_LDFLAGS = $(AM_LDFLAGS) \
> + -Wl,-framework,Cocoa
> +libwindow_macosx_plugin_la_OBJCFLAGS = $(AM_OBJCFLAGS) \
> + -fobjc-arc -fobjc-exceptions
> +
> libvout_macosx_plugin_la_SOURCES = video_output/macosx.m
> libvout_macosx_plugin_la_CFLAGS = $(AM_CFLAGS) -DHAVE_GL_CORE_SYMBOLS
> libvout_macosx_plugin_la_LIBADD = libvlc_opengl.la
> @@ -41,7 +47,7 @@ libcaopengllayer_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(voutdir)' \
>
> libglinterop_cvpx_plugin_la_LDFLAGS += -Wl,-framework,IOSurface,-framework,OpenGL
> vout_LTLIBRARIES += libvout_macosx_plugin.la libcaopengllayer_plugin.la \
> - libglinterop_cvpx_plugin.la
> + libglinterop_cvpx_plugin.la libwindow_macosx_plugin.la
> endif
> if HAVE_IOS
> libglinterop_cvpx_plugin_la_CFLAGS = $(AM_CFLAGS) -DUSE_OPENGL_ES2
> diff --git a/modules/video_output/window_macosx.m b/modules/video_output/window_macosx.m
> new file mode 100644
> index 00000000000..6a242f30df8
> --- /dev/null
> +++ b/modules/video_output/window_macosx.m
> @@ -0,0 +1,480 @@
> +/**
> + * @file window_macosx.m
> + * @brief macOS Window and View output provider
> + */
> +
> +/* Copyright (C) 2020 VLC authors and VideoLAN
> + *
> + * Authors: Marvin Scholz <epirat 07 at gmail dot com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +# include <config.h>
> +#endif
> +
> +#import <Cocoa/Cocoa.h>
> +
> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_vout_window.h>
> +
> +#define VLC_ASSERT_MAINTHREAD NSAssert([[NSThread currentThread] isMainThread], \
> + @"Must be called from the main thread!")
> +
> +/**
> + * Style mask for a decorated window
> + */
> +static const NSWindowStyleMask decoratedWindowStyleMask =
> + NSWindowStyleMaskTitled
> + | NSWindowStyleMaskClosable
> + | NSWindowStyleMaskMiniaturizable
> + | NSWindowStyleMaskResizable;
> +
> +/**
> + * Style mask for a non-decorated window
> + */
> +static const NSWindowStyleMask undecoratedWindowStyleMask =
> + NSWindowStyleMaskBorderless
> + | NSWindowStyleMaskResizable;
> +
> +
> +#pragma mark -
> +#pragma mark Obj-C Interfaces
> +
> +NS_ASSUME_NONNULL_BEGIN
> +
> +/**
> + * Video output window class
> + *
> + * Custom NSWindow subclass, mostly to overwrite that the window
> + * can become the key window even if its using the borderless
> + * (undecorated) style.
> + */
> + at interface VLCVideoWindow : NSWindow
> + at end
> +
> +
> +/**
> + * Video view class
> + *
> + * Custom NSWindow subclass, used to track resizes so that
> + * the core cen be notified about the new sizes in a timely manner.
> + */
> + at interface VLCVideoWindowContentView : NSView {
> + @private
> + vout_window_t* vlc_vout_window;
> +}
> +
> +- (instancetype)initWithVLCVoutWindow:(vout_window_t *)vout_wnd;
initWithWindow will probably not be confusing and simpler, but that's nit.
> + at end
> +
> +
> +/**
> + * Video output window controller class
> + *
> + * Controller for the VLC standalone video window (independent of the interface)
> + *
> + * Implements all interactions between the display module and the NSWindow
> + * class, except for resizes (which is handled by VLCVideoWindowContentView).
> + */
> + at interface VLCVideoStandaloneWindowController : NSWindowController <NSWindowDelegate> {
> + @private
> + vout_window_t* vlc_vout_window;
> +}
> +
> +- (instancetype)initWithVLCVoutWindow:(vout_window_t *)vout_wnd;
> +- (void)showWindowWithConfig:(const vout_window_cfg_t *restrict)config;
> +
> +/* Methods called by the callbacks to change properties of the Window */
> +- (void)setWindowDecorated:(BOOL)decorated;
> +- (void)setWindowFullscreen:(BOOL)fullscreen;
> +
> + at end
> +
> +
> +#pragma mark -
> +#pragma mark Obj-C Implementations
> +
> + at implementation VLCVideoStandaloneWindowController
> +
> +/**
> + * Initializes the window controller with the content view in the
> + * given vout_window_t.
> + */
> +- (instancetype)initWithVLCVoutWindow:(vout_window_t *)vout_wnd
> +{
> + VLC_ASSERT_MAINTHREAD;
> +
> + NSWindow *window = [[NSWindow alloc] initWithContentRect:NSZeroRect
> + styleMask:decoratedWindowStyleMask
> + backing:NSBackingStoreBuffered
> + defer:YES];
> +
> + self = [super initWithWindow:window];
Nit too, but I'd rather have if (!self) return nil; and avoid the
indentation level below.
> + if (self) {
> + vlc_vout_window = vout_wnd;
> +
> + // Set the initial vout title
> + [window setTitle:[NSString stringWithUTF8String:VOUT_TITLE " (VLC Video Output)"]];
> +
> + // The content always changes during live resize
> + [window setPreservesContentDuringLiveResize:NO];
> +
> + // Do not release on close (we might want to re-open the window later)
> + [window setReleasedWhenClosed:NO];
> +
> + // Hint that the window should become a primary fullscreen window
> + [window setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
> +
> + // Create and set custom content view for the window
> + VLCVideoWindowContentView *view =
> + [[VLCVideoWindowContentView alloc] initWithVLCVoutWindow:vout_wnd];
> + [window setContentView:view];
> +
> + [window setDelegate:self];
> +
> + // Position the window in the center
> + [window center];
> +
> + if (view == nil)
> + return nil;
> +
> + [self setWindowFrameAutosaveName:@"VLCVideoStandaloneWindow"];
> +
> + vout_wnd->type = VOUT_WINDOW_TYPE_NSOBJECT;
> + vout_wnd->handle.nsobject = (__bridge void*)view;
> + }
> +
> + return self;
> +}
> +
> +/**
> + * Applies the given config to the window and shows it.
> + */
> +- (void)showWindowWithConfig:(const vout_window_cfg_t *restrict)config
> +{
> + VLC_ASSERT_MAINTHREAD;
> +
> + // Convert from backing to window coordinates
> + NSRect backingRect = NSMakeRect(0, 0, config->width, config->height);
> + NSRect windowRect = [self.window convertRectFromBacking:backingRect];
> + [self.window setContentSize:windowRect.size];
> +
> + // Set decoration
> + [self setWindowDecorated:config->is_decorated];
> +
> + // This should always be called last, to ensure we only show the
> + // window once its fully configured. Else there could be visible
> + // changes or animations when the config is applied.
> + [self showWindow:nil];
> + [self.window makeKeyAndOrderFront:nil];
> +}
> +
> +- (BOOL)windowShouldClose:(NSWindow *)sender
> +{
> + vout_window_ReportClose(vlc_vout_window);
Need to lock and check vlc_vout_window here.
> + return YES;
> +}
> +
> +#pragma mark Helper methods
> +
> +- (BOOL)isWindowFullscreen
> +{
> + return ((self.window.styleMask & NSFullScreenWindowMask) == NSFullScreenWindowMask);
> +}
> +
> +#pragma mark Module interactions
> +
> +- (void)setWindowDecorated:(BOOL)decorated
> +{
> + NSWindowStyleMask mask =
> + (decorated) ? decoratedWindowStyleMask : undecoratedWindowStyleMask;
> +
> + [self.window setStyleMask:mask];
> +}
> +
> +- (void)setWindowFullscreen:(BOOL)fullscreen
> +{
> + if (!!fullscreen == !![self isWindowFullscreen]) {
> + // Nothing to do, just report the state to core
> + if (fullscreen) {
> + vout_window_ReportFullscreen(vlc_vout_window, NULL);
> + } else {
> + vout_window_ReportWindowed(vlc_vout_window);
> + }
Need to lock and check vlc_vout_window here.
> + return;
> + }
> +
> + [self.window toggleFullScreen:nil];
> +}
> +
> +#pragma mark Window delegate
> +
> +- (void)windowDidEnterFullScreen:(NSNotification *)notification
> +{
> + vout_window_ReportFullscreen(vlc_vout_window, NULL);
Need to lock and check vlc_vout_window here.
> +}
> +
> +- (void)windowDidExitFullScreen:(NSNotification *)notification
> +{
> + vout_window_ReportWindowed(vlc_vout_window);
Need to lock and check vlc_vout_window here.
> +}
> +
> + at end
> +
> +
> +
> + at implementation VLCVideoWindowContentView
> +
> +- (instancetype)initWithVLCVoutWindow:(vout_window_t *)vout_wnd
> +{
> + self = [super init];
> + if (self) {
> + NSAssert(vout_wnd != NULL, @"Invalid vout_window_t passed.");
> + vlc_vout_window = vout_wnd;
> + }
> + return self;
> +}
> +
> +- (void)drawRect:(NSRect)dirtyRect
> +{
> + [[NSColor blackColor] setFill];
> + NSRectFill(dirtyRect);
> +}
> +
> +/**
> + * Report the view size in the backing size dimensions to VLC core
> + */
> +- (void)reportBackingSize
> +{
> + NSRect bounds = [self convertRectToBacking:self.bounds];
> + vout_window_ReportSize(vlc_vout_window, NSWidth(bounds), NSHeight(bounds));
Need to lock and check vlc_vout_window here. Aren't they deadlock
because resize is signalled while the vout display is being started?
It's typically happening on iOS without the special handling.
> +}
> +
> +/**
> + * Handle view size changes
> + */
> +- (void)resizeSubviewsWithOldSize:(NSSize)oldSize
> +{
> + [self reportBackingSize];
> + [super resizeSubviewsWithOldSize:oldSize];
> +}
> +
> +/**
> + * Handle view backing property changes
> + */
> +- (void)viewDidChangeBackingProperties
> +{
> + // When the view backing size changes, it means the view effectively
> + // resizes from VLC core perspective, as it operates on the real
> + // backing dimensions, not the view point size.
> + [self reportBackingSize];
> + [super viewDidChangeBackingProperties];
> +}
> +
> + at end
> +
> + at implementation VLCVideoWindow
> +
> +- (BOOL)canBecomeKeyWindow
> +{
> + // A window with NSWindowStyleMaskBorderless can usually not become key
> + // window, unless we return YES here.
> + return YES;
> +}
> +
> + at end
> +
> +NS_ASSUME_NONNULL_END
> +
> +
> +#pragma mark -
> +#pragma mark VLC module
> +
> +typedef struct
> +{
> + VLCVideoStandaloneWindowController *vc;
You could store the VLCVideoStandaloneWindowController directly as sys
of the vout window object.
> +} vout_window_sys_t;
> +
> +/* Enable Window
> + */
> +static int WindowEnable(vout_window_t *wnd, const vout_window_cfg_t *restrict cfg)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + @autoreleasepool {
> +
> + VLCVideoStandaloneWindowController *vc = sys->vc;
> + dispatch_sync(dispatch_get_main_queue(), ^{
Does it need to be sync since you don't depend on any result
to return VLC_SUCCESS?
> + [vc showWindowWithConfig:cfg];
> + });
> + }
> +
> + return VLC_SUCCESS;
> +}
> +
> +/* Request to close the window */
> +static void WindowDisable(vout_window_t *wnd)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + @autoreleasepool {
> + __weak VLCVideoStandaloneWindowController *weakVc = sys->vc;
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [weakVc close];
> + });
> + }
> +}
> +
> +/* Request to resize the window */
> +static void WindowResize(vout_window_t *wnd, unsigned width, unsigned height)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + @autoreleasepool {
> + __weak VLCVideoStandaloneWindowController *weakVc = sys->vc;
> + dispatch_async(dispatch_get_main_queue(), ^{
> + VLCVideoStandaloneWindowController *vc = weakVc;
> + // Convert from backing to window coordinates
> + NSRect backingRect = NSMakeRect(0, 0, width, height);
> + NSRect windowRect = [vc.window convertRectFromBacking:backingRect];
> + [vc.window setContentSize:windowRect.size];
> +
> + // Size is reported by resizeSubviewsWithOldSize:, do not
> + // report it here, else it would get reported twice.
> + });
> + }
> +}
> +
> +/* Request to enable/disable Window decorations */
> +static void SetDecoration(vout_window_t *wnd, bool decorated)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + @autoreleasepool {
> + __weak VLCVideoStandaloneWindowController *weakVc = sys->vc;
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [weakVc setWindowDecorated:decorated];
> + });
> + }
> +}
> +
> +/* Request to enter fullscreen */
> +static void WindowSetFullscreen(vout_window_t *wnd, const char *idstr)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + @autoreleasepool {
> + __weak VLCVideoStandaloneWindowController *weakVc = sys->vc;
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [weakVc setWindowFullscreen:YES];
> + });
> + }
> +}
> +
> +/* Request to exit fullscreen */
> +static void WindowUnsetFullscreen(vout_window_t *wnd)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + @autoreleasepool {
> + __weak VLCVideoStandaloneWindowController *weakVc = sys->vc;
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [weakVc setWindowFullscreen:NO];
> + });
> + }
> +}
> +
> +static void WindowSetTitle(struct vout_window_t *wnd, const char *title)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> + @autoreleasepool {
> + __weak VLCVideoStandaloneWindowController *weakVc = sys->vc;
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [weakVc.window setTitle:[NSString stringWithUTF8String:title]];
> + });
> + }
> +}
> +
> +/*
> + * Module destruction
> + */
> +void Close(vout_window_t *wnd)
> +{
> + vout_window_sys_t *sys = wnd->sys;
> +
> + // ARC can not know when to release an object in a heap-allocated
> + // struct, so we need to explicitly set it to nil here.
> + sys->vc = nil;
Need to lock and set the vlc_vout_window to null here, to signal
that we cannot use the window callback anymore since the vout window
might be destroyed right after this function returns.
In addition sys->vc = nil seems to release the window controller in
your comment, but it seems this is the only reference to it because
other references are weak right (like above in WindowSetTitle)?
In this case, won't we have a release of the WindowController while
it's still in use within the main thread? Doesn't the reference above
need a __strong reference instead?
> +}
> +
> +/*
> + * Callbacks
> + */
> +static const struct vout_window_operations ops = {
> + .enable = WindowEnable,
> + .disable = WindowDisable,
> + .resize = WindowResize,
> + .set_state = NULL,
> + .unset_fullscreen = WindowUnsetFullscreen,
> + .set_fullscreen = WindowSetFullscreen,
> + .set_title = WindowSetTitle,
> + .destroy = Close,
> +};
> +
> +/*
> + * Module initialization
> + */
> +int Open(vout_window_t *wnd)
> +{
> + @autoreleasepool {
> + msg_Info(wnd, "using the macOS new video output window module");
> +
> + // Check if there is an NSApplication, needed for the connection
> + // to the Window Server so we can use NSWindows, NSViews, etc.
> + if (NSApp == nil) {
> + msg_Err(wnd, "cannot create video output window without NSApplication");
> + return VLC_EGENERIC;
> + }
> +
> + vout_window_sys_t *sys = vlc_obj_calloc(VLC_OBJECT(wnd), 1, sizeof(*sys));
> + if (unlikely(sys == NULL))
> + return VLC_ENOMEM;
> +
> + __block VLCVideoStandaloneWindowController *vc;
> + dispatch_sync(dispatch_get_main_queue(), ^{
> + vc = [[VLCVideoStandaloneWindowController alloc] initWithVLCVoutWindow:wnd];
> + });
> + if (unlikely(vc == nil))
> + return VLC_ENOMEM;
> + sys->vc = vc;
> +
> + wnd->ops = &ops;
> + wnd->sys = sys;
> +
> + return VLC_SUCCESS;
> + }
> +}
> +
> +/*
> + * Module declaration
> + */
> +vlc_module_begin()
> + set_description("macOS Video Output Window")
> + set_capability("vout window", 1000)
> + set_callback(Open)
> +vlc_module_end()
> --
> 2.24.3 (Apple Git-128)
>
> _______________________________________________
> 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