openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/fix-vlc2.2-osx into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/fix-vlc2.2-osx/+merge/257498

Fix a problem with VLC 2.2 on Mac OS X, where VLC could not find it's plugins, and would bomb out.
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/fix-vlc2.2-osx into lp:openlp.

=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2015-03-18 22:04:30 +0000
+++ openlp/core/ui/media/vlcplayer.py	2015-04-27 09:42:20 +0000
@@ -38,40 +38,6 @@
 
 log = logging.getLogger(__name__)
 
-VLC_AVAILABLE = False
-try:
-    from openlp.core.ui.media.vendor import vlc
-    VLC_AVAILABLE = bool(vlc.get_default_instance())
-except (ImportError, NameError, NotImplementedError):
-    pass
-except OSError as e:
-    if is_win():
-        if not isinstance(e, WindowsError) and e.winerror != 126:
-            raise
-    elif is_macosx():
-        pass
-    else:
-        raise
-
-if VLC_AVAILABLE:
-    try:
-        VERSION = vlc.libvlc_get_version().decode('UTF-8')
-    except:
-        VERSION = '0.0.0'
-    # LooseVersion does not work when a string contains letter and digits (e. g. 2.0.5 Twoflower).
-    # http://bugs.python.org/issue14894
-    if LooseVersion(VERSION.split()[0]) < LooseVersion('1.1.0'):
-        VLC_AVAILABLE = False
-        log.debug('VLC could not be loaded, because the vlc version is too old: %s' % VERSION)
-    # On linux we need to initialise X threads, but not when running tests.
-    if VLC_AVAILABLE and is_linux() and 'nose' not in sys.argv[0]:
-        import ctypes
-        try:
-            x11 = ctypes.cdll.LoadLibrary('libX11.so')
-            x11.XInitThreads()
-        except:
-            log.exception('Failed to run XInitThreads(), VLC might not work properly!')
-
 # Audio and video extensions copied from 'include/vlc_interface.h' from vlc 2.2.0 source
 AUDIO_EXT = ['*.3ga', '*.669', '*.a52', '*.aac', '*.ac3', '*.adt', '*.adts', '*.aif', '*.aifc', '*.aiff', '*.amr',
              '*.aob', '*.ape', '*.awb', '*.caf', '*.dts', '*.flac', '*.it', '*.kar', '*.m4a', '*.m4b', '*.m4p', '*.m5p',
@@ -89,6 +55,61 @@
              '*.nut', '*.rv', '*.xvid']
 
 
+def get_vlc():
+    """
+    In order to make this module more testable, we have to wrap the VLC import inside a method. We do this so that we
+    can mock out the VLC module entirely.
+
+    :return: The "vlc" module, or None
+    """
+    if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
+        # If VLC has already been imported, no need to do all the stuff below again
+        return sys.modules['openlp.core.ui.media.vendor.vlc']
+
+    is_vlc_available = False
+    try:
+        if is_macosx():
+            # Newer versions of VLC on OS X need this. See https://forum.videolan.org/viewtopic.php?t=124521
+            os.environ['VLC_PLUGIN_PATH'] = '/Applications/VLC.app/Contents/MacOS/plugins'
+        from openlp.core.ui.media.vendor import vlc
+
+        is_vlc_available = bool(vlc.get_default_instance())
+    except (ImportError, NameError, NotImplementedError):
+        pass
+    except OSError as e:
+        if is_win():
+            if not isinstance(e, WindowsError) and e.winerror != 126:
+                raise
+        elif is_macosx():
+            pass
+        else:
+            raise
+
+    if is_vlc_available:
+        try:
+            VERSION = vlc.libvlc_get_version().decode('UTF-8')
+        except:
+            VERSION = '0.0.0'
+        # LooseVersion does not work when a string contains letter and digits (e. g. 2.0.5 Twoflower).
+        # http://bugs.python.org/issue14894
+        if LooseVersion(VERSION.split()[0]) < LooseVersion('1.1.0'):
+            is_vlc_available = False
+            log.debug('VLC could not be loaded, because the vlc version is too old: %s' % VERSION)
+        # On linux we need to initialise X threads, but not when running tests.
+        if is_vlc_available and is_linux() and 'nose' not in sys.argv[0]:
+            import ctypes
+            try:
+                x11 = ctypes.cdll.LoadLibrary('libX11.so')
+                x11.XInitThreads()
+            except:
+                log.exception('Failed to run XInitThreads(), VLC might not work properly!')
+
+    if is_vlc_available:
+        return vlc
+    else:
+        return None
+
+
 class VlcPlayer(MediaPlayer):
     """
     A specialised version of the MediaPlayer class, which provides a VLC display.
@@ -110,6 +131,7 @@
         """
         Set up the media player
         """
+        vlc = get_vlc()
         display.vlc_widget = QtGui.QFrame(display)
         display.vlc_widget.setFrameStyle(QtGui.QFrame.NoFrame)
         # creating a basic vlc instance
@@ -145,12 +167,13 @@
         """
         Return the availability of VLC
         """
-        return VLC_AVAILABLE
+        return get_vlc() is not None
 
     def load(self, display):
         """
         Load a video into VLC
         """
+        vlc = get_vlc()
         log.debug('load vid in Vlc Controller')
         controller = display.controller
         volume = controller.media_info.volume
@@ -190,6 +213,7 @@
         Wait for the video to change its state
         Wait no longer than 60 seconds. (loading an iso file needs a long time)
         """
+        vlc = get_vlc()
         start = datetime.now()
         while not media_state == display.vlc_media.get_state():
             if display.vlc_media.get_state() == vlc.State.Error:
@@ -209,6 +233,7 @@
         """
         Play the current item
         """
+        vlc = get_vlc()
         controller = display.controller
         start_time = 0
         log.debug('vlc play')
@@ -254,6 +279,7 @@
         """
         Pause the current item
         """
+        vlc = get_vlc()
         if display.vlc_media.get_state() != vlc.State.Playing:
             return
         display.vlc_media_player.pause()
@@ -303,6 +329,7 @@
         """
         Update the UI
         """
+        vlc = get_vlc()
         # Stop video if playback is finished.
         if display.vlc_media.get_state() == vlc.State.Ended:
             self.stop(display)

=== modified file 'openlp/plugins/media/lib/mediaitem.py'
--- openlp/plugins/media/lib/mediaitem.py	2015-04-21 21:49:22 +0000
+++ openlp/plugins/media/lib/mediaitem.py	2015-04-27 09:42:20 +0000
@@ -33,8 +33,8 @@
 from openlp.core.ui import DisplayController, Display, DisplayControllerType
 from openlp.core.ui.media import get_media_players, set_media_players, parse_optical_path, format_milliseconds
 from openlp.core.utils import get_locale_key
-from openlp.core.ui.media.vlcplayer import VLC_AVAILABLE
-if VLC_AVAILABLE:
+from openlp.core.ui.media.vlcplayer import get_vlc
+if get_vlc() is not None:
     from openlp.plugins.media.forms.mediaclipselectorform import MediaClipSelectorForm
 
 

=== added file 'tests/functional/openlp_core_ui_media/test_vlcplayer.py'
--- tests/functional/openlp_core_ui_media/test_vlcplayer.py	1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_core_ui_media/test_vlcplayer.py	2015-04-27 09:42:20 +0000
@@ -0,0 +1,77 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2015 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# 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 General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+"""
+Package to test the openlp.core.ui.media.vlcplayer package.
+"""
+import os
+import sys
+
+from unittest import TestCase
+from tests.functional import patch
+
+from openlp.core.ui.media.vlcplayer import get_vlc
+
+
+class TestVLCPlayer(TestCase):
+    """
+    Test the functions in the :mod:`vlcplayer` module.
+    """
+
+    @patch('openlp.core.ui.media.vlcplayer.is_macosx')
+    def fix_vlc_22_plugin_path_test(self, mocked_is_macosx):
+        """
+        Test that on OS X we set the VLC plugin path to fix a bug in the VLC module
+        """
+        # GIVEN: We're on OS X and we don't have the VLC plugin path set
+        mocked_is_macosx.return_value = True
+        if 'VLC_PLUGIN_PATH' in os.environ:
+            del os.environ['VLC_PLUGIN_PATH']
+        if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
+            del sys.modules['openlp.core.ui.media.vendor.vlc']
+
+        # WHEN: An checking if the player is available
+        get_vlc()
+
+        # THEN: The extra environment variable should be there
+        self.assertIn('VLC_PLUGIN_PATH', os.environ,
+                      'The plugin path should be in the environment variables')
+        self.assertEqual('/Applications/VLC.app/Contents/MacOS/plugins', os.environ['VLC_PLUGIN_PATH'])
+
+    @patch.dict(os.environ)
+    @patch('openlp.core.ui.media.vlcplayer.is_macosx')
+    def not_osx_fix_vlc_22_plugin_path_test(self, mocked_is_macosx):
+        """
+        Test that on Linux or some other non-OS X we do not set the VLC plugin path
+        """
+        # GIVEN: We're not on OS X and we don't have the VLC plugin path set
+        mocked_is_macosx.return_value = False
+        if 'VLC_PLUGIN_PATH' in os.environ:
+            del os.environ['VLC_PLUGIN_PATH']
+        if 'openlp.core.ui.media.vendor.vlc' in sys.modules:
+            del sys.modules['openlp.core.ui.media.vendor.vlc']
+
+        # WHEN: An checking if the player is available
+        get_vlc()
+
+        # THEN: The extra environment variable should NOT be there
+        self.assertNotIn('VLC_PLUGIN_PATH', os.environ,
+                         'The plugin path should NOT be in the environment variables')
\ No newline at end of file