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

Better/own PlayerProcessInfo implementation #220

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

pannal
Copy link
Contributor

@pannal pannal commented Jul 9, 2018

GHI (If applicable): #

Description:

Adds PMS stream/decision details to the PlayerProcessInfo window; PlayerProcessInfo directly implemented in SeekDialog

This ultimately supersedes #218

Checklist:

  • I have based this PR against the develop branch

@ruuk ruuk self-assigned this Jul 9, 2018
self.attributes.add("Subtitles", *data)

def fillServerInfo(self):
self.attributes.add("User", *["%s @ %s" % (self.session.user.title, self.mediaItem.server.name)])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be u"%s @ %s" I guess.

@@ -22,7 +22,7 @@
</control>
</control>
<control type="group">
<visible>!Window.IsVisible(sliderdialog) + !Window.IsVisible(osdvideosettings) + !Window.IsVisible(osdaudiosettings)</visible>
<visible>!Window.IsVisible(sliderdialog) + !Window.IsVisible(osdvideosettings) + !Window.IsVisible(osdaudiosettings) + !Window.IsVisible(playerprocessinfo)</visible>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a remnant and can be removed

@ruuk
Copy link
Contributor

ruuk commented Jul 10, 2018

I still need to review the code, but this looks awesome!

I noticed I'm not always getting video info - the line isn't visible.
This either needs to show below the OSD, or hide when the OSD is visible, as it covers the preview images.

You might as well change the 'o' key handler to use this:

                 elif action.getButtonCode() == 61519:
-                    # xbmc.executebuiltin('Action(PlayerProcessInfo)')
-                    xbmc.executebuiltin('Action(CodecInfo)')
+                    if self.getProperty('show.PPI'):
+                        self.hidePPIDialog()
+                    else:
+                        self.showPPIDialog()

@pannal
Copy link
Contributor Author

pannal commented Jul 10, 2018

OK, this is reviewable now.

The session data seems to only be available for the user that owns the server, so the PMS stream detail part doesn't work on a friends' server nor when the current user is a managed user.

@pannal
Copy link
Contributor Author

pannal commented Jul 11, 2018

This now works with non-owned servers as well, by taking partial data from the transcodeSession if available.

@pannal
Copy link
Contributor Author

pannal commented Jul 13, 2018

Done.

@pannal
Copy link
Contributor Author

pannal commented Aug 13, 2018

Rebased onto develop

@pannal
Copy link
Contributor Author

pannal commented Aug 28, 2018

Rebased

@ruuk ruuk force-pushed the develop branch 3 times, most recently from 78f9acc to 126aaba Compare August 31, 2018 22:50
reference_data["video_stream"] = stream
elif stream.id == mediaChoice.audioStream.id:
reference_data["audio_stream"] = stream
elif stream.id == mediaChoice.subtitleStream.id:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before executing mediaChoice.subtitleStream.id, you should check to make sure the subtitle stream exists (it doesn't always) otherwise this will throw. So this line is better as elif mediaChoice.subtitleStream != None and stream.id == mediaChoice.subtitleStream.id: Similar should be done with the other streams just to make sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants