Resolution filter fix #20
Conversation
…ion to make use of stash's transcoding for low power devices like some google TVs. Due to the encoding delays on consumer hardware inputstream support has also been added to use the DASH streams from stash
There was a problem hiding this comment.
Thank you for your contribution.
I created a PR myself to fix the 0.24 incompatibility (#21). After I merged that PR, would you be able to rebase your branch and incorporate the requested changes? Afterwards I will then test it (and if everything is fine it can be merged).
build.sh
Outdated
| @@ -0,0 +1,10 @@ | |||
| #! /bin/bash | |||
There was a problem hiding this comment.
I don't think this file is necessary? Seems to be for yourself to build a ZIP which you can then install into Kodi?
Personally I'm must developing (/cloned the repository) into the Kodi addon directory so I don't need to build it and it allows for a much faster development cycle.
So could you please remove it?
There was a problem hiding this comment.
yeah, that was mainly so i could quickly build it, as i was then using adb to push it to a android devie Ill remove it from the branch
| @@ -1 +1,3 @@ | |||
| __pycache__ | |||
| venv/ | |||
There was a problem hiding this comment.
I think neither of these are required? The plugin doesn't require a separate venv (it's running in Kodi anyway), and there is no need to build a ZIP (see below as well).
There was a problem hiding this comment.
this is so i could install the kodi stubs to get some code completion from VS Code relating to the Kodi functions.
I prefer to use a venv to do this rather than installing it locally/globally to avoid polluting my users python libaries with development libaries
There was a problem hiding this comment.
If you want to keep them locally, and ignored. You can add it to .git/info/exclude. Git also reads that file as a .gitignore but it will be local only / not part of the repository.
There was a problem hiding this comment.
from VS Code
That explains all the (re)formatting of further unchanged code. PyCharm doesn't do that (automatically), which I prefer.
resources/lib/criterion_parser.py
Outdated
| @@ -1,4 +1,10 @@ | |||
| import json | |||
| from resources.lib.utils.resolutions import Resolution_map | |||
| from resources.lib.utils import logger | |||
There was a problem hiding this comment.
logger is unused so this line can be removed
| from resources.lib.utils.resolutions import Resolution_map | ||
| from resources.lib.utils import logger | ||
|
|
||
| """ elif "resolution" == criterion: |
There was a problem hiding this comment.
Commented code, so can be removed
resources/lib/plugin.py
Outdated
| @@ -6,6 +6,7 @@ | |||
| import xbmcgui | |||
| import xbmcplugin | |||
| from resources.lib import utils | |||
| from resources.lib.utils import logger | |||
There was a problem hiding this comment.
logger is unused so can be removed
resources/lib/utils/logger.py
Outdated
| @@ -0,0 +1,52 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
This file is unused so can be removed.
Personally I debug using the method described here: https://kodi.wiki/view/HOW-TO:Debug_Python_Scripts_with_Web-PDB
So temporarily adding web-pdb as dependency and triggering that from the code.
But adding some form of actual logging would be fine too.
There was a problem hiding this comment.
If you're running Kodi solely on Android then using web-pdb might be a bit harder indeed.
|
Do you mind this PR Also including the DASH+Input Stream support too? So instead of merging 19 and this we just merge this PR |
Ah right, sorry, it's two individual fixes actually, one for the resolution filter and one for DASH / input stream (I thought this was a fix on the DASH / input stream support at first, as that also includes "resolution stuff"). If you want to you can split them. Makes it a bit cleaner. And in that case you might also rebase on top of #21 already, as I guess it will result in some merge conflicts. |
|
Manually merged the additional properties in 1aa964b plugin.video.stash/resources/lib/criterion_parser.py Lines 29 to 30 in f2cfe6b Changing resolution which is also part of this branch is also manually merged, as mentioned in the PR Thanks |
a small fix + some random things i started to add which fixes a exception when a saved filter has a resolution
Should be merged after #19