← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~bastian-germann/openlp/depends into lp:openlp

 

Review: Needs Fixing

Please remove the video code changes as they break OpenLP and do not provide an acceptable user experience.

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-02 17:19:11 +0000
> @@ -26,13 +26,8 @@
>  import datetime
>  import logging
>  
> -try:
> -    from pymediainfo import MediaInfo
> -    pymediainfo_available = True
> -except ImportError:
> -    pymediainfo_available = False
> -
>  from subprocess import check_output
> +from pymediainfo import MediaInfo

This crashes the code with a stack trace and does not give an error message,

>  from PyQt5 import QtCore, QtWidgets
>  
>  from openlp.core.state import State
> @@ -168,11 +163,11 @@
>          self.setup()
>          self.vlc_player = VlcPlayer(self)
>          State().add_service("mediacontroller", 0)
> -        if get_vlc() and pymediainfo_available:
> +        if get_vlc():
>              State().update_pre_conditions("mediacontroller", True)
>          else:
>              State().missing_text("mediacontroller", translate('OpenLP.SlideController',
> -                                 "VLC or pymediainfo are missing, so you are unable to play any media"))

This does not give the correct message to the user about errors gracefully providing errors and allowing them to continue degraded.

> +                                 "VLC is missing, so you are unable to play any media"))
>          self._generate_extensions_lists()
>          return True
>  


-- 
https://code.launchpad.net/~bastian-germann/openlp/depends/+merge/365421
Your team OpenLP Core is subscribed to branch lp:openlp.


References