Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve subtitle behaviour using PlayerMonitor #183

Merged
merged 2 commits into from
Jul 9, 2020

Conversation

mediaminister
Copy link
Contributor

@mediaminister mediaminister commented May 25, 2020

This includes:

  • Implement PlayerMonitor service
  • Check subtitles onAVStarted and onAVChange:
    • Select internal InputStream Adaptive subtitles if available
    • Add external recalculated subtitle if no subtitles available
    • Enable subtitles if showsubtitles add-on setting is enabled

There are still bugs in InputStream Adaptive or Kodi that cause the following problems:

@mediaminister mediaminister force-pushed the subtitles branch 4 times, most recently from a7237ea to c5e3955 Compare May 25, 2020 15:03
@mediaminister mediaminister mentioned this pull request May 26, 2020
@dagwieers
Copy link
Collaborator

Personally I don't like it that add-ons need a service and a player monitor (running all the time) just to ensure that e.g. the subtitles work correctly. So it is unfortunate that we have to work around it like this.

That said, if this is a temporary measure (and we ensure things get fixed properly upstream at some point in the future) at least things work better for end-users, which is paramount.

@mediaminister mediaminister marked this pull request as draft May 29, 2020 10:51
@mediaminister mediaminister force-pushed the subtitles branch 7 times, most recently from c252902 to e2c21fd Compare June 5, 2020 10:58
@mediaminister mediaminister marked this pull request as ready for review June 5, 2020 11:25
@mediaminister
Copy link
Contributor Author

mediaminister commented Jun 5, 2020

Personally I don't like it that add-ons need a service and a player monitor (running all the time) just to ensure that e.g. the subtitles work correctly. So it is unfortunate that we have to work around it like this.

There are too many problems with subtitles which we cannot quickly fix ourselves, so a player monitor that switches between external en internal subtitles will be needed for a long time I think:

  • Sometimes VTM GO MPD manifests only define internal subtitles for the first period (e.g. 13 Geboden episode 2). After inspecting the actual stream, it does contain subtitles for the whole program but this is not detected in InputStream Adaptive.
  • InputStream Adaptive suddenly stops displaying internal WEBVTT subtitles in some cases, fixed in [WebVTT] Fix binary representation xbmc/inputstream.adaptive#456
  • External subtitles get out of sync when a new period starts, this is a rather difficult bug to fix as you need to understand how video player timing works in Kodi or InputStream Adaptive.
  • Kodi's setSubtitles API automatically enables subtitles and sometimes this is unwanted behaviour because we only want to add an external subtitle file without overruling a users choice to disable subtitles.

@dagwieers
Copy link
Collaborator

@mediaminister My comment wasn't criticizing this PR specifically, but rather a general notice. However I prefer if we open a tracking issue for each of the issues and report them upstream now things are still fresh.

Interesting, VTM GO uses the Anvato Universal Player, would be nice to add it to our testing wiki page...

@dagwieers
Copy link
Collaborator

We started watching Matroesjkas, which without subtitles is quite impossible with Russian or Lithuanian conversations.
Test-driving these changes, so far so good.

@dagwieers dagwieers added bug Something isn't working enhancement New feature or request labels Jul 1, 2020
@dagwieers dagwieers mentioned this pull request Jul 3, 2020
Closed
@dagwieers
Copy link
Collaborator

Using PR #183 I get the following error trying to play movie Gulliver's Travel.

2020-07-03 19:36:11.345 T:1042281344  NOTICE: [plugin.video.vtm.go] [vtmgostream] Downloading text from https://dcs-vod.apis.anvato.net/vod/p/session/manifest.mpd?i=i177999829-ndbc68579-a470-4a83-995d-8bdb27af72e1
2020-07-03 19:36:11.431 T:1042281344  NOTICE: [plugin.video.vtm.go] [urllib3.connectionpool] https://dcs-vod.apis.anvato.net:443 "GET /vod/p/session/manifest.mpd?i=i177999829-ndbc68579-a470-4a83-995d-8bdb27af72e1 HTTP/1.1" 200 None
2020-07-03 19:36:11.552 T:1042281344   ERROR: EXCEPTION Thrown (PythonToCppException) : -->Python callback/script returned the following error<--
                                             - NOTE: IGNORING THIS CAN LEAD TO MEMORY LEAKS!
                                            Error Type: <type 'exceptions.AttributeError'>
                                            Error Contents: 'NoneType' object has no attribute 'get'
                                            Traceback (most recent call last):
                                              File "/storage/.kodi/addons/plugin.video.vtm.go/plugin.py", line 16, in <module>
                                                plugin.run(sys.argv)
                                              File "/storage/.kodi/addons/plugin.video.vtm.go/resources/lib/plugin.py", line 218, in run
                                                routing.run(params)
                                              File "/storage/.kodi/addons/script.module.routing/lib/routing.py", line 130, in run
                                                self._dispatch(self.path)
                                              File "/storage/.kodi/addons/script.module.routing/lib/routing.py", line 141, in _dispatch
                                                view_func(**kwargs)
                                              File "/storage/.kodi/addons/plugin.video.vtm.go/resources/lib/plugin.py", line 199, in play
                                                Player(kodi).play(category, item)
                                              File "/storage/.kodi/addons/plugin.video.vtm.go/resources/lib/modules/player.py", line 59, in play
                                                resolved_stream = self._vtm_go_stream.get_stream(category, item)
                                              File "/storage/.kodi/addons/plugin.video.vtm.go/resources/lib/vtmgo/vtmgostream.py", line 102, in get_stream
                                                subtitles = self._extract_subtitles_from_stream_info(stream_info)
                                              File "/storage/.kodi/addons/plugin.video.vtm.go/resources/lib/vtmgo/vtmgostream.py", line 213, in _extract_subtitles_from_stream_info
                                                name = '{} - {}_{}'.format(stream_info.get('video').get('metadata').get('program').get('title'), stream_info.get('video').get('metadata').get('title'), idx)
                                            AttributeError: 'NoneType' object has no attribute 'get'
                                            -->End of Python script error report<--

@mediaminister
Copy link
Contributor Author

Fixed again.

@michaelarnauts
Copy link
Collaborator

I think this branch is better then what we have right now. Thanks @mediaminister!

@michaelarnauts michaelarnauts merged commit db4935a into add-ons:master Jul 9, 2020
@dagwieers dagwieers added this to the v1.1.0 milestone Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants