openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

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.