openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Tim Bentley has proposed merging lp:~trb143/openlp/media2018a into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1589675 in OpenLP: "OpenLP on Linux crashes on startup when VLC not installed"
  https://bugs.launchpad.net/openlp/+bug/1589675

For more details, see:
https://code.launchpad.net/~trb143/openlp/media2018a/+merge/341592

Fix issues with VLC

lp:~trb143/openlp/media2018a (revision 2838)
https://ci.openlp.io/job/Branch-01-Pull/2481/                          [SUCCESS]
https://ci.openlp.io/job/Branch-02a-Linux-Tests/2382/                  [SUCCESS]
https://ci.openlp.io/job/Branch-02b-macOS-Tests/169/                   [FAILURE]
https://ci.openlp.io/job/Branch-03a-Build-Source/91/                   [SUCCESS]
https://ci.openlp.io/job/Branch-03b-Build-macOS/84/                    [SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code-Analysis/1553/                [SUCCESS]
https://ci.openlp.io/job/Branch-04b-Test-Coverage/1366/                [SUCCESS]

-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~trb143/openlp/media2018a into lp:openlp.

=== modified file 'openlp/core/ui/media/mediacontroller.py'
--- openlp/core/ui/media/mediacontroller.py	2018-02-23 15:54:25 +0000
+++ openlp/core/ui/media/mediacontroller.py	2018-03-18 18:27:55 +0000
@@ -459,26 +459,16 @@
         log.debug('use %s controller' % self.current_media_players[controller.controller_type].display_name)
         return True
 
-    def media_length(self, service_item):
+    @staticmethod
+    def media_length(service_item):
         """
-        Loads and starts a media item to obtain the media length
+        Uses Media Info to obtain the media length
 
         :param service_item: The ServiceItem containing the details to be played.
         """
         media_info = MediaInfo()
         media_info.volume = 0
         media_info.file_info = QtCore.QFileInfo(service_item.get_frame_path())
-        # display = controller.preview_display
-        suffix = '*.%s' % media_info.file_info.suffix().lower()
-        used_players = get_media_players()[0]
-        player = self.media_players[used_players[0]]
-        if suffix not in player.video_extensions_list and suffix not in player.audio_extensions_list:
-            # Media could not be loaded correctly
-            critical_error_message_box(
-                translate('MediaPlugin.MediaItem', 'Unsupported Media File'),
-                translate('MediaPlugin.MediaItem', 'File {file_path} not supported using player {player_name}'
-                          ).format(file_path=service_item.get_frame_path(), player_name=used_players[0]))
-            return False
         media_data = MediaInfoWrapper.parse(service_item.get_frame_path())
         # duration returns in milli seconds
         service_item.set_media_length(media_data.tracks[0].duration)
@@ -573,16 +563,6 @@
                 if not title:
                     continue
                 player = self.media_players[title]
-                # The system player may not return what files it can play so add it now
-                #  and check whether it can play the file later
-                if title == 'system':
-                    if not controller.media_info.is_background or controller.media_info.is_background and \
-                            player.can_background:
-                        self.resize(display, player)
-                        if player.load(display):
-                            self.current_media_players[controller.controller_type] = player
-                            controller.media_info.media_type = MediaType.Video
-                            return True
                 if suffix in player.video_extensions_list:
                     if not controller.media_info.is_background or controller.media_info.is_background and \
                             player.can_background:

=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2017-12-29 09:15:48 +0000
+++ openlp/core/ui/media/vlcplayer.py	2018-03-18 18:27:55 +0000
@@ -66,7 +66,15 @@
     """
     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:
+            is_vlc_available = bool(sys.modules['openlp.core.ui.media.vendor.vlc'].get_default_instance())
+        except:
+            pass
+        if is_vlc_available:
+            return sys.modules['openlp.core.ui.media.vendor.vlc']
+        else:
+            return None
     is_vlc_available = False
     try:
         if is_macosx():
@@ -89,6 +97,7 @@
     except (ImportError, NameError, NotImplementedError):
         pass
     except OSError as e:
+        # this will get raised the first time
         if is_win():
             if not isinstance(e, WindowsError) and e.winerror != 126:
                 raise

=== modified file 'tests/functional/openlp_core/api/endpoint/test_controller.py'
--- tests/functional/openlp_core/api/endpoint/test_controller.py	2018-03-09 21:58:45 +0000
+++ tests/functional/openlp_core/api/endpoint/test_controller.py	2018-03-18 18:27:55 +0000
@@ -22,8 +22,24 @@
 from unittest import TestCase
 from unittest.mock import MagicMock
 
+from PyQt5 import QtCore
+
 from openlp.core.common.registry import Registry
 from openlp.core.api.endpoint.controller import controller_text, controller_direction
+from openlp.core.display.renderer import Renderer
+from openlp.core.display.screens import ScreenList
+from openlp.core.lib import ServiceItem
+
+from tests.utils import convert_file_service_item
+from tests.utils.constants import RESOURCE_PATH
+
+TEST_PATH = str(RESOURCE_PATH / 'service')
+
+SCREEN = {
+    'primary': False,
+    'number': 1,
+    'size': QtCore.QRect(0, 0, 1024, 768)
+}
 
 
 class TestController(TestCase):
@@ -38,11 +54,18 @@
         Registry.create()
         self.registry = Registry()
         self.mocked_live_controller = MagicMock()
+        self.desktop = MagicMock()
+        self.desktop.primaryScreen.return_value = SCREEN['primary']
+        self.desktop.screenCount.return_value = SCREEN['number']
+        self.desktop.screenGeometry.return_value = SCREEN['size']
+        self.screens = ScreenList.create(self.desktop)
+        renderer = Renderer()
+        renderer.empty_height = 1000
         Registry().register('live_controller', self.mocked_live_controller)
 
-    def test_controller_text(self):
+    def test_controller_text_empty(self):
         """
-        Remote API Tests : test the controller text method can be called
+        Remote API Tests : test the controller text method can be called with empty service item
         """
         # GIVEN: A mocked service with a dummy service item
         self.mocked_live_controller.service_item = MagicMock()
@@ -53,6 +76,22 @@
         assert isinstance(results['item'], MagicMock)
         assert len(results['slides']) == 0
 
+    def test_controller_text(self):
+        """
+        Remote API Tests : test the controller text method can be called with a real service item
+        """
+        # GIVEN: A mocked service with a dummy service item
+        line = convert_file_service_item(TEST_PATH, 'serviceitem_custom_1.osj')
+        self.mocked_live_controller.service_item = ServiceItem(None)
+        self.mocked_live_controller.service_item.set_from_service(line)
+        self.mocked_live_controller.service_item.render(True)
+        # WHEN: I trigger the method
+        ret = controller_text("SomeText")
+        # THEN: I get a basic set of results
+        results = ret['results']
+        assert isinstance(ret, dict)
+        assert len(results['slides']) == 2
+
     def test_controller_direction_next(self):
         """
         Text the live next method is triggered