Merge branch 'fix-issues-1045-1043' into 'master'

A couple of bug fixes:

See merge request openlp/openlp!444

openlp/core/lib/serviceitem.py

@@ -755,16 +755,12 @@ class ServiceItem(RegistryProperties):
             except IndexError:
                 return ''
         if self.is_image() or self.is_capable(ItemCapabilities.IsOptical):
-            path_from = frame['path']
+            frame_path = Path(frame['path'])
         elif self.is_command() and not self.has_original_file_path and self.sha256_file_hash:
-            path_from = os.path.join(frame['path'], self.stored_filename)
+            frame_path = Path(frame['path']) / self.stored_filename
         else:
-            path_from = os.path.join(frame['path'], frame['title'])
-        if isinstance(path_from, str):
-            # Handle service files prior to OpenLP 3.0
-            # Windows can handle both forward and backward slashes, so we use ntpath to get the basename
-            path_from = Path(path_from)
-        return path_from
+            frame_path = Path(frame['path']) / frame['title']
+        return frame_path
 
     def remove_frame(self, frame):
         """

openlp/core/ui/media/mediacontroller.py

@@ -351,7 +351,7 @@ class MediaController(RegistryBase, LogMixin, RegistryProperties):
         if MediaInfo.can_parse():
             media_data = MediaInfo.parse(media_path)
             # duration returns in milli seconds
-            return media_data.tracks[0].duration
+            return media_data.tracks[0].duration or 0
         return 0
 
     def media_setup_optical(self, filename, title, audio_track, subtitle_track, start, end, display, controller):

openlp/core/ui/servicemanager.py

@@ -580,19 +580,18 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
             # If the item has files, see if they exists
             if item['service_item'].uses_file():
                 for frame in item['service_item'].get_frames():
-                    path_from = item['service_item'].get_frame_path(frame=frame)
-                    path_from_path = Path(path_from)
-                    if item['service_item'].stored_filename:
-                        sha256_file_name = item['service_item'].stored_filename
-                    else:
-                        sha256_file_name = sha256_file_hash(path_from_path) + os.path.splitext(path_from)[1]
-                    path_from_tuple = (path_from_path, sha256_file_name)
-                    if path_from_tuple in write_list or str(path_from_path) in missing_list:
+                    frame_path = item['service_item'].get_frame_path(frame=frame)
+                    if not frame_path.exists():
+                        missing_list.append(str(frame_path))
                         continue
-                    if not os.path.exists(path_from):
-                        missing_list.append(str(path_from_path))
+                    if item['service_item'].stored_filename:
+                        sha256_file_name = Path(item['service_item'].stored_filename)
                     else:
-                        write_list.append(path_from_tuple)
+                        sha256_file_name = sha256_file_hash(frame_path) / frame_path.suffix
+                    bundle = (frame_path, sha256_file_name)
+                    if bundle in write_list or str(frame_path) in missing_list:
+                        continue
+                    write_list.append(bundle)
                 # For items that has thumbnails, add them to the list
                 if item['service_item'].is_capable(ItemCapabilities.HasThumbnails):
                     thumbnail_path = item['service_item'].get_thumbnail_path()

tests/openlp_core/ui/media/test_mediacontroller.py

@@ -326,7 +326,7 @@ def test_media_hide(media_env, registry):
 
 def test_media_length(media_env):
     """
-    Test the Media Info basic functionality
+    Check the duration of a few different files via MediaInfo
     """
     for test_data in TEST_MEDIA:
         # GIVEN: a media file
@@ -339,6 +339,38 @@ def test_media_length(media_env):
         assert results == test_data[1], 'The correct duration is returned for ' + test_data[0]
 
 
+@patch('openlp.core.ui.media.mediacontroller.MediaInfo.parse')
+def test_media_length_duration_none(mocked_parse, media_env):
+    """
+    Test that when MediaInfo doesn't give us a duration, we default to 0
+    """
+    # GIVEN: A fake media file and a mocked MediaInfo.parse() function
+    mocked_parse.return_value = MagicMock(tracks=[MagicMock(duration=None)])
+    file_path = 'path/to/fake/video.mkv'
+
+    # WHEN the media data is retrieved
+    duration = media_env.media_controller.media_length(file_path)
+
+    # THEN you can determine the run time
+    assert duration == 0, 'The duration should be 0'
+
+
+@patch('openlp.core.ui.media.mediacontroller.MediaInfo.can_parse')
+def test_media_length_no_can_parse(mocked_can_parse, media_env):
+    """
+    Check that 0 is returned when MediaInfo cannot parse
+    """
+    # GIVEN: A fake media file and a mocked MediaInfo.can_parse() function
+    mocked_can_parse.return_value = False
+    file_path = 'path/to/fake/video.mkv'
+
+    # WHEN the media data is retrieved
+    duration = media_env.media_controller.media_length(file_path)
+
+    # THEN you can determine the run time
+    assert duration == 0, 'The duration should be 0'
+
+
 def test_on_media_play(media_env):
     """
     Test the on_media_play method