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

mpsyt support #30

Closed
atomi opened this issue Mar 20, 2017 · 20 comments
Closed

mpsyt support #30

atomi opened this issue Mar 20, 2017 · 20 comments

Comments

@atomi
Copy link

atomi commented Mar 20, 2017

I was hoping to get catt to work with mpsyt. I opened an issue here mps-youtube/yewtube#607

I think if there was an option to set catt to stay in foreground until the end of the video we can have some basic compatibility with mpsyt.

@theychx
Copy link
Collaborator

theychx commented Mar 20, 2017

I dont't see this happening. Catt is not a conventional media player, it just takes the url it gets from youtube-dl and passes it on to a chromecast. And besides, why not just bake chromecast support directly into mps-youtube? As you can see in catt's source code, thats pretty easy to do.

@theychx
Copy link
Collaborator

theychx commented Mar 20, 2017

I see now that mps-youtube relies on external programs for playback, so disregard that last comment. I'm still not in favour of putting functionality into catt, that is going to be obscure to 99.9% of users.

@atomi
Copy link
Author

atomi commented Mar 20, 2017

I just really love the idea of being able to cast a playlist of youtube videos from the command line.
I was thinking something simple. catt cast --foreground [url] with SIGINT support.

@skorokithakis
Copy link
Owner

That would be a nice piece of functionality indeed. I don't know how this could work, as I'm not sure how mps works. Does the solution in mps-youtube/yewtube#607 work reliably?

@atomi
Copy link
Author

atomi commented Mar 20, 2017

Yeah it actually does work as is except for a small bug:

In mpsyt, catt works like this:

set player catt
set playerargs cast

But as catt goes into the background, mpsyt thinks the video has terminated so it goes on to the next video. If catt is able to stay in the foreground (with an optional arg perhaps) mpsyt will properly consider the video still playing.

@skorokithakis
Copy link
Owner

Ah, I see. The problem with that is that catt can't (easily) know when the video has stopped playing, as it's not playing the video itself. All it could do is ask the TV whether the video has stopped playing every few seconds, but that would have different problems (if the user performed a seek at some point, catt might get confused, or delay playing the next song too much, or play it too early)...

@atomi
Copy link
Author

atomi commented Mar 20, 2017

Hmmm I think it wont matter to mpsyt. As long as catt is running the video is considered still playing. When catt terminates mpsyt will know to execute another catt command.

@atomi
Copy link
Author

atomi commented Mar 20, 2017

Oh nevermind. I see what you're saying.

@atomi atomi closed this as completed Mar 20, 2017
@atomi atomi reopened this Mar 20, 2017
@atomi
Copy link
Author

atomi commented Mar 20, 2017

We might be able to use a listener described here. home-assistant-libs/pychromecast#84 (comment)

Or if it's outside the scope of catt. I can open a new project just for this.

@skorokithakis
Copy link
Owner

Hmm, this might work. I'm not against having this in catt, although it will have to be a bit unsupported/hidden (perhaps just be a --foreground argument). Would you like to do some testing and see if it's doable?

@atomi
Copy link
Author

atomi commented Mar 20, 2017

Yeah.

@theychx
Copy link
Collaborator

theychx commented Mar 20, 2017

@atomi
I did this PoC, to use in place of catt. This has the behaviour you requested. It also responds to SIGINT, but that does not seem to work with mpsyt.

@atomi
Copy link
Author

atomi commented Mar 20, 2017

@theychx How was your script called from mpsyt?
I opened up an issue at mps-youtube/yewtube#607
perhaps @ids1024 can comment.

@theychx
Copy link
Collaborator

theychx commented Mar 20, 2017

I just put it in my path, and then did something like set player cc-play from within mpsyt. Please note that playback itself works fine with mpsyt.

@theychx
Copy link
Collaborator

theychx commented Mar 21, 2017

@skorokithakis @atomi
So I incoporated some more of that catt goodness into my script.
I've grown quite fond of mpsyt, so I might expand the script, so that the user also has basic playback controls. But since that strays away quite a bit from what catt is doing, I'm wondering if it isn't better if this becomes sort of a "sister"-project to catt, instead of incorporating this functionality into catt. Let me know what you think.

@atomi
Copy link
Author

atomi commented Mar 21, 2017

@theychx haha yeah that's awesome, but none of these are my projects. If I wasn't so bogged down yesterday/today I might have done a simple script like you. Thanks for sharing it.

@theychx theychx closed this as completed Sep 17, 2017
@skorokithakis
Copy link
Owner

What's the resolution on this ticket?

@skorokithakis
Copy link
Owner

I have gotten to quite like mpsyt as well, but I agree with you, @theychx. Adding controls to catt would be a bit too much, and possibly break easily if other people connected to the network seek/stop/pause playback on their devices/phones.

If it were rather bulletproof, I wouldn't be against doing this, but both error-prone and somewhat-out-of-scope is a bit much, I think.

@theychx
Copy link
Collaborator

theychx commented Sep 18, 2017

@skorokithakis, sorry about closing this without consulting you, I was tired and this annoyed me for some reason.

I actually fiddled with a prototype that was remote-controllable through the mpv(media player) json-ipc protocol, and got it mostly working. However, adding support for a new "player" in mpsyt proved to be really hard. There is actually an ongoing effort to make the relevant code more object-oriented (which would make my project a lot easier), but that seems to (also) have gone stale.

I agree that this functionallity does not belong in catt, and that mpsytsupport is not really useful without the control part.

@skorokithakis
Copy link
Owner

@theychx No problem, I agree with the closure even though I'd like it if we had something like this implemented (if not for the caveats above).

Let's revisit if mpsyt becomes more modular, maybe we can have a special mode in a separate, bundled utility or something like that.

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

No branches or pull requests

3 participants