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
Modify TuneIn to use the AudioService #7
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for the pull request. I like the changes but one of my test case stations using search string "KEXP" doesn't work for some reason with the new audio service. I took a quick peek but I couldn't find anything obvious of why or any logging. Could you possibly take a look to see if you can spot anything? |
|
sure, i'll see what I can figure out! |
|
it worked fine for me: |
|
of course, if i try it again immediately, i get this: |
|
every other attempt I made to play that station, both from cli and stt seems to work okay. |
|
For me it says that it finds the station but I get no audio. If you're getting audio then it might just be something in my setup here. |
|
@johnbartkiw that's possible. I previously installed vlc and set it to be my default audio backend, because otherwise I was getting some things (emby streams that did not use mp3) playing on a chromecast in the house instead of locally! you might check the /var/log/mycroft/audio.log to see if there's something similarly odd going on for you |
|
Checking the logs everything looks fine but still no audio. 12:37:16.162 - mycroft.audio.speech:mute_and_speak:123 - INFO - Speak: Just a second I'm not sure what to check next. |
|
next thing I would try is to try the audio backend with the URL in that logging manually. what backend are you using? is it mpg123? |
|
At least in my test, it plays fine on the command line, both from VLC and from mpg123 |
|
Hard coding the URL gives me the same results. How do you test the AudioService from the command line outside of a skill? |
|
I did it by running |
|
your log indicates you're using the default SimpleAudioService to play. That uses mycroft's play_mp3 behind the scenes, as you can see here: https://github.com/MycroftAI/mycroft-core/blob/dev/mycroft/audio/services/simple/__init__.py#L101-L102. Since this was using play_mp3 itself before this pull request, I'm not sure how it could make a difference. |
|
possibly https://github.com/johnbartkiw/mycroft-skill-tunein/pull/7/files#diff-743b1adbf37db12e16e01ef62a6f7d72L32 should be removed since play_mp3 isn't actually used here any more |
|
It was working fine with the straight calls to mpg123 so calling that from the command line doesn't really test anything different for me. Any chance the service is misinterpreting the MIME type and using something other than play_mp3? |
|
that sounds like a possibility. I checked the content type returned with curl, and got an empty reply: however, https://github.com/MycroftAI/mycroft-core/blob/dev/mycroft/audio/services/simple/__init__.py#L27-L39 looks like it would correctly guess mp3 based on the extension. even if it fell through to None, the default action is to use mp3. |
|
This weekend i can try using the simple audio service and see if maybe i can reproduce |
|
I would like to see this change to, as using the play_mp3 directly do give problem, as other skills cant tell/see that there is audio going on. If using audioservice then other skills would know that. |
|
If adding the mime type to the audio_service.play then KEXP radio station and other stations which return empty conttype will work (if they ofcause is mp3 content) this work for me |
|
What would people think about switching over to VLC since there do seem to be issues with play_mp3 in general? If you check out my IHeartRadio skill I use VLC in the context of the audio service there. Would that work if I moved this skill to be the same? |
|
I would prefear to use audioservice and let the mycroftinstallation deside the audiobackend. If not using audioservice other skills dosnt know if speaker is in use or not and tht can give some problems. |
|
My only problem with this pull request is that it doesn't work for me as I still get no audio. @andlo are you able to test it and verify that it works fine for you? |
|
I got it working also with kexp and danisk radio DR P1 I had to change audioservice call to include streamtype |
|
I am not that good githubber....I have forked benklop:master as he has made the change to use audioservice. I have made changes to help with the missing content-type in some streams, and it looks like it is working. BUt I dont know how to pull that back up. Do I first pull it to benklop:master, and then @benklop can merge and pull back to you or how do one do this ? |
|
Can you test for me if this station would work with those commits. I am getting an error in tunein when trying to play this station: |
|
:( no it dossnt play :( I am not good at analysing and dosnt have much knoledeg on audio streams. But maybe if you open the stream in VLC or mplayer it could tell more what it is and then we could make some logic to set right content-type to streams without or with wrong type. |
|
testing with mpg123 I get this output: trying with curl -I I get I notise Content-Type: audio/mpeg which should be same as Content-Type: audio/x-mpeg. What I could se were a solution is using audioserver and add contenttype to empty streams. And then let the user if one wantto change the audio server backend to vlc. That would make the skill easy to install, and use, but if some streams isnt working user has to change audioserver backend too vlc. But the skill by itself dossnt use vlc directly and mycroft and other skills arnt getting confused. |
|
sorry didnt get that. Does it mean I should install VLC on my picroft installation ? |
|
It means it dosnt work by the default mpg123 as backend. But if VLC is backend I think it would work. VLC do play that exact stream. |
|
How do I set VLC as backend in my installation ? Where is that config ? |
|
Sorry I dont know....Never used it but i seen reference to it in the mycroft.conf. Maybe tehre is some doc on mycroft.ai? my best gues is install vlc (only the cli version) and then set backend in mycroft.conf https://github.com/MycroftAI/mycroft-core/blob/dev/mycroft/configuration/mycroft.conf#L112 |
|
I installed VLC now on my installation, still not playing. |
|
I dont know.....sorry. |
|
ok now when I type the command in the client it says now l´playing... |
|
Hi John. |
| # Ignore entries that are marked as unavailable | ||
| self.mpeg_url = entry.getAttribute("URL") | ||
| self.station_name = entry.getAttribute("text") | ||
| # this URL will return audio/x-mpegurl data. This is just a list of URLs to the real streams | ||
| self.stream_url = self.get_stream_url(self.mpeg_url) | ||
| self.audio_state = "playing" | ||
| LOG.debug("Found stream URL: " + self.stream_url) | ||
| self.audio_service = AudioService(self.bus) |
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.
suggestion: move self.audio_service = AudioService(self.bus) into initialize, you can reuse the audio service object this way
| @@ -87,17 +89,20 @@ def CPS_match_query_phrase(self, phrase): | |||
|
|
|||
| return phrase, CPSMatchLevel.GENERIC, phrase | |||
|
|
|||
| def stop(self): | |||
| pass | |||
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.
you can just remove this, no need to include an empty stop method
| @@ -25,6 +25,7 @@ | |||
| from xml.dom.minidom import parseString | |||
|
|
|||
| from mycroft.skills.common_play_skill import CommonPlaySkill, CPSMatchLevel | |||
| from mycroft.skills.audioservice import AudioService | |||
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.
you can also remove from mycroft.util import play_mp3
|
Hey John. I really would like to use your skill. |
|
Hey I'm going to update it in the next day or two to use VLC and audio service. It's just a busy time right now with the new year so I haven't had the time. I haven't accepted this PR because it just didn't work for me. I should be able to make the move to VLC pretty easy with a separate change. |
|
I've updated the main branch to utilize AudioService and VLC as the backend. If this ends up solving problems for people then I will close this PR. |
|
I have a question @johnbartkiw . Do I have to change something in the mycroft.conf file ? |
|
Also why does it say "This branch has conflicts" in init.py that have to be resolved ? |
|
This branch has conflicts because of the changes in the PR to the .py file don't match up with the changes I made on the main branch. You should be using the main branch and not this branch at this point. |
|
Sorry. I reinstalled the skill again with your updates i guess but its still not working. I had changed before the mycroft.conf to default backend "vlc". Should i put local again? What else should i check? |
|
Without more information I don't know if you've just got a bad client state or something else. The skill works fine on both my dev machine and my Mark-1. I didn't touch the mycroft.conf on either of those so it's possible yours is in a bad state? |
|
Hello. I pulled again mycroft-core to fix it. Didnt work. I uninstalled vlc and reinstalled your skill. Also nothing. Only other thing is that i have mopidy installed. Maybe i should also remove it? Otherwise i do a fresh entire install. |
|
Anything in the logs or debug output? Doesn't work doesn't really give me anything to go on. |
using the audioservice so mycroft handles stop and gets better integration with other audio services