Review: Needs Information Overall this looks good. There is some commented code that I think either need to be fixed or removed? Also, I noticed there's some code that deals with streaming on Windows and Linux, which I presume you'll need the macOS equivalent for. Is there any other code that you need the macOS equivalent for?
Diff comments: > > === modified file 'openlp/core/ui/media/mediacontroller.py' > --- openlp/core/ui/media/mediacontroller.py 2019-02-14 15:09:09 +0000 > +++ openlp/core/ui/media/mediacontroller.py 2019-04-11 20:30:04 +0000 > @@ -287,14 +198,13 @@ > :param display: Display on which the output is to be played > :param preview: Whether the display is a main or preview display > """ > - # clean up possible running old media files > - self.finalise() > + display.media_info = ItemMediaInfo() > display.has_audio = True > - if display.is_live and preview: > - return > + # if display.is_live and preview: > + # return Can we remove these comments if the code is no longer used? > if preview: > display.has_audio = False > - self.vlc_player.setup(display) > + self.vlc_player.setup(display, preview) > > def set_controls_visible(self, controller, value): > """ > @@ -305,9 +215,9 @@ > """ > # Generic controls > controller.mediabar.setVisible(value) > - if controller.is_live and controller.display: > - if self.current_media_players and value: > - controller.display.set_transparency(False) > + # if controller.is_live and controller.display: > + # if self.current_media_players and value: > + # controller.display.set_transparency(False) Can we remove these comments if the code is no longer used? > > @staticmethod > def resize(display, player): > @@ -354,8 +264,8 @@ > log.debug('video is not optical and live') > controller.media_info.length = service_item.media_length > is_valid = self._check_file_type(controller, display) > - display.override['theme'] = '' > - display.override['video'] = True > + # display.override['theme'] = '' > + # display.override['video'] = True Can we remove these comments if the code is no longer used? > if controller.media_info.is_background: > # ignore start/end time > controller.media_info.start_time = 0 > @@ -379,10 +289,10 @@ > critical_error_message_box(translate('MediaPlugin.MediaItem', > 'Unsupported File'), > translate('MediaPlugin.MediaItem', > 'Unsupported File')) > return False > - log.debug('video mediatype: ' + > str(controller.media_info.media_type)) > + log.debug('video media type: ' + > str(controller.media_info.media_type)) > # dont care about actual theme, set a black background > - if controller.is_live and not controller.media_info.is_background: > - display.frame.runJavaScript('show_video("setBackBoard", null, > null,"visible");') > + # if controller.is_live and not controller.media_info.is_background: > + # display.frame.runJavaScript('show_video("setBackBoard", null, > null,"visible");') Can we remove these comments if the code is no longer used? > # now start playing - Preview is autoplay! > autoplay = False > # Preview requested > @@ -533,8 +439,8 @@ > else: > self.media_volume(controller, controller.media_info.volume) > if first_time: > - if not controller.media_info.is_background: > - display.frame.runJavaScript('show_blank("desktop");') > + # if not controller.media_info.is_background: > + # display.frame.runJavaScript('show_blank("desktop");') Can we remove these comments if the code is no longer used? > > self.current_media_players[controller.controller_type].set_visible(display, > True) > controller.mediabar.actions['playbackPlay'].setVisible(False) > controller.mediabar.actions['playbackPause'].setVisible(True) > @@ -653,8 +555,8 @@ > """ > display = self._define_display(controller) > if controller.controller_type in self.current_media_players: > - if not looping_background: > - display.frame.runJavaScript('show_blank("black");') > + # if not looping_background: > + # display.frame.runJavaScript('show_blank("black");') Can we remove these comments if the code is no longer used? > > self.current_media_players[controller.controller_type].stop(display) > > self.current_media_players[controller.controller_type].set_visible(display, > False) > controller.seek_slider.setSliderPosition(0) > @@ -725,7 +627,7 @@ > display.override = {} > > self.current_media_players[controller.controller_type].reset(display) > > self.current_media_players[controller.controller_type].set_visible(display, > False) > - display.frame.runJavaScript('show_video("setBackBoard", null, > null, "hidden");') > + # display.frame.runJavaScript('show_video("setBackBoard", null, > null, "hidden");') Can we remove these comments if the code is no longer used? > del self.current_media_players[controller.controller_type] > > def media_hide(self, msg): -- https://code.launchpad.net/~trb143/openlp/media_state/+merge/365879 Your team OpenLP Core is subscribed to branch lp:openlp. _______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp