-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fixed issue #76 #79
base: master
Are you sure you want to change the base?
Fixed issue #76 #79
Conversation
I really appreciate that you want to contribute! Thanks. I am currently on mobile but looking forward to fix this issue together with you. Anyway, before we starting the code, we should generally decide what the plugin should do. While I understand your idea with the user input I don’t think it will be ever used at all (my personal opinion). IMHO the plugin is called SENDtokodi not „input url at Kodi“. So in MY opinion it should maybe just show an info on how to use it and maybe something like a QR code to download one of the companion apps? As this is not a technical issue I would really like to hear some other voices on what is really useful and should be the intended functionality. |
@nullket I understand your point. But in my opinion, this plugin is not actually sending to Kodi, It's a middle layer which is receiving URL from multiple sources like Kore, Chrome extension etc, converting URL to playable URL with the help of yt-dlp and play on Kodi. So, even sendToKodi name is also not fully correct, because main role of the plugin is to receive and play. I think, while using kodi on PC, if we will have option to just copy url from browser and paste it in kodi, It will be useful feature. |
i actually made a chrome extension todo this: https://github.com/firsttris/chrome.sendtokodi but i was so lazy not to extend my chrome developer account and it was deleted from the store. the kodi addon was acutally intended as service addon. still i understand your idea to add some functionality if you open the addon in kodi best regards |
@nullket and @firsttris |
Aye. I second this. But I would also argue that one should work on the root of the issue (e.g. sending the link directly from your browser would be much more convenient than navigating to the add-on). But that means not just reactivating your chrome developer account firsttris but somewhat support for firefox and maybe even safari. So in conclusion to that, either showing an info page (on how to use the add-on) or providing an input makes sense. @GautamMKGarg so I would say go on with it. Your designed input should not cause any issues (at least I hope so). But your introduction of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some comments to the code but all of those are basically really minor thins and just suggestions - so please do not get shocked of 6 comments on the approx 3 changed lines of code ;) Oh and btw it seems there is a merge conflict (try to merge the current master in your branch an resolve the issue from there)
service.py
Outdated
@@ -60,6 +70,10 @@ def showErrorNotification(message): | |||
def getParams(): | |||
result = {} | |||
paramstring = sys.argv[2] | |||
|
|||
if paramstring == '': | |||
paramstring = "?" + get_user_input() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't know this but for what reason is the ?
needed? May maybe something like the following would be more direct
if not paramstring:
result['url'] = paramstring[1:]
else:
result['url'] = get_local_user_input()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that in future we might need to give option to user to select video quality, or pass any other customer parameters. That's why I did not pass URL directly in url variable. Kindly suggest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would even then suggest to explicitly return the result dict already correctly assigned to the keys instead of parsing it from a string.
Forcing a quality via the resolver will not really be possible anymore. Basically all streams are now fragmented with a manifest. Many still provide one legacy all in one stream where there is not much to choose from. For Kodi you can set the desired quality of a fragmented stream within the adaptive input stream addon. We partially supported manifest since the last PR we were testing so much.
Yes, You are right @nullket. Different users have different browser preference. I like using Firefox and Brave. Brave is also chromium based, but some of the chrome addons don't work on Brave. I would like to add one more point. Adding third-party addons make browser slower. And also some users don't like to install addons due to their security concern. People use web browser to do banking and stuff, so not all users like to install a addon on browser. I know that sendToKodi is an opensource safe plugin, but not all users know that. Secondly, if someone use Kodi regularly then it's fine to install an addon, but if user use it once in a while, then they will not prefer to install addon. Just coping URL will be more convenient. for them.
We can keep both options. For time being we can provide user input option. Later on when documentation is ready, we can create a Menu, where we can provide both options,
|
I am fine with the suggestions you have given. If you want to edit the code on your own, I am fine with it. Or if you want me to do changes, then also I can work on the same. Please suggest, how would you like to proceed? I am beginner for sendToKodi, that's why I have made many basic mistakes. You know the code better. |
Conflicts: service.py
@nullket
As mentioned earlier, while trying to play video, Video playable URL is getting fetched properly. But the video is not getting played. Will you be able to fix the bug in your free time? URL: https://www.youtube.com/watch?v=g6fnFALEseI
|
I had a longer look at this and I don't know why the playback does not start. In my understanding @firsttris can you explain how calling the addon via the json api works and how the start of the playback work? I assume in the json is a |
|
Thanks! @GautamMKGarg that means you either need something that opens the player (and does not try to open it twice when called via the api) or call the plugin recursively with the api call shown above and the manual retrieved url. |
I have only had a cursory glance at the changes and the issue description, but it seems to me that this relies on functionality intended for script addons (async user input), as opposed to plugin addon functionality (which this currently is). This plugin is already using script functionality (according to consultation with Kodi forum users). We should probably not be tricked into spending time asking why something doesn't work at that point — we're already off the map, and might end up rolling a boulder uphill. My understanding thus far is that a plugin addon should always be able to resolve into Kodi navigation tree nodes without additional user input. Correct me if I'm wrong. If we want these things I still suspect we'd need a script addon or something. Possibly a combination of both. But I'm no Kodi expert. |
Hello @nullket and @firsttris
Tried fixing the issue #76. Issue is almost fixed. When I try to enter any youtube URL or any other URL in sendToKodi plugin directly, data is getting fetched but video/audio is not getting played. But when I send the URL via Kore, video is getting played.
I am testing with same url in both the cases. Not sure if I am doing something wrong. Could you please suggest me changes if you are available?