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

Implement playing audio through discord voice connections. #180

Merged
merged 14 commits into from
Oct 18, 2020

Conversation

BrandtHill
Copy link
Contributor

@BrandtHill BrandtHill commented Sep 30, 2020

Implement voice 👍

closes #84

@@ -1,7 +1,7 @@
# Intro
Nostrum is a an Elixir library that can be used to interact with Discord.

Nostrum currently supports the latest stable version of Elixir, v. 1.7.
Nostrum currently supports the latest stable version of Elixir, v. 1.9.
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't latest stable 1.10?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just changed it to what mix.exs is set to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to change the wording as well as some other fixes, stay tuned, very spicy commits inbound

@Kraigie
Copy link
Owner

Kraigie commented Oct 1, 2020

Just wanted to say thanks first and foremost! This looks like it took a lot of work. I'll have some time this weekend to go over the code and check it out!

@BrandtHill
Copy link
Contributor Author

1337 lines added (nice). If you average one line a minute, it should only take a little over 22 hours.

@Kraigie
Copy link
Owner

Kraigie commented Oct 5, 2020

This is sick! I tested it out locally and things worked flawlessly. My only complaint would be with the errors related to ffmpeg or youtube-dl not being available.

Right now if you don't have those configured properly you get what I believe is probably a porcelain error saying the command isn't available.

** (ArgumentError) argument error
    :erlang.apply({:error, "Command not found: ffmpeg"}, :out, [])

** (MatchError) no match of right hand side value: {:error, "Command not found: youtube-dl"}

I think we should probably improve these a bit, or even prevent them from happening in the first place. We can either be proactive or reactive and I'm not sure what's the best approach.

Proactive

My first idea is that we could check somewhere that the user has the two programs in their path, or at the path they specify in the config. However, I'm not sure where it would be best to do this.

We could optionally have the user set an option in the config that specifies that they want to use voice and then just check that the programs exist on startup if they intend to use voice.

Or we could also just do it every time the voice supervisor starts up. This might be problematic because I don't think we'd want to warn when this happens because they might not be using voice, but users may miss an info. Maybe that's OK though.

I can't think of a way to check before the function calls themselves that doesn't involve spinning up porcelain every time so this might be out of the picture.

Reactive

Alternatively, and this is probably the better idea, we could just check for those errors and throw an exception. Things aren't going to continue anyways without those tools so blowing it up seems reasonable. This doesn't change much from how it acts now, but it tells the user explicitly what is wrong!

@BrandtHill
Copy link
Contributor Author

I've done some of both: I've added a suppressible warning message upon startup if the configured executable(s) weren't found in the system path, and running either ffmpeg or youtube-dl will raise a Nostrum.Error.VoiceError if Porcelain can't find the executable.

No ffmpeg

18:01:50.666 [warn]  ffmpeeeg was not found in your path. Nostrum requires ffmpeg to use voice.
If you don't intend to use voice, configure :nostrum, :ffmpeg to nil to suppress.

ffmpeg without youtube-dl

18:18:35.457 [warn]  youtube-dlll was not found in your path. Nostrum supports youtube-dl for voice.
If you don't require youtube-dl support, configure :nostrum, :youtubedl to nil to suppress.

Voice.play/3 without ffmpeg

17:48:07.207 [error] Task #PID<0.343.0> started from #PID<0.327.0> terminating
** (Nostrum.Error.VoiceError) ERROR: Command not found: ffmpeeeg - ffmpeg improperly configured
    (nostrum 0.4.4) lib/nostrum/voice/audio.ex:166: Nostrum.Voice.Audio.spawn_ffmpeg/2
    (nostrum 0.4.4) lib/nostrum/voice.ex:132: Nostrum.Voice.play/3
    ....

If the play/3 call was issued by a consumer, which will be the most common use case, the error will pop 4 times if uncaught since the started under the ConsumerSupervisor. I guess this is the intended behaviour.

@BrandtHill
Copy link
Contributor Author

End of input will be automatically detected for playing audio with pipe/ytdl. :VOICE_SPEAKING_UPDATE events will be emitted when sound starts or stops.

end

def try_send_data(%VoiceState{} = voice, init?) do
wait = if(init?, do: 20_000, else: 200)
Copy link
Owner

Choose a reason for hiding this comment

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

How does this work? If someone plays a short song (under 20s) will we not send the event until after the 20s has elapsed? Or is this for some other functionality entirely? Not sure what stalling means in this context!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stalling occurs when the ffmpeg process stays open waiting for data to be piped into stdin. Calling Enum.take/2 on the process's output stream will hang indefinitely in this case. When playing from a url/file, ffmpeg closes automatically and a "stall" never occurs. The 20 second init timer means that if we are just beginning to play a sound, allow up to 20 seconds for ffmpeg, youtube-dl, or whatever source you might use with :pipe option, to download/buffer data from the source to start playing. Otherwise, if we're already playing audio (init? == false) only wait 200 milliseconds until we consider ffmpeg to have stalled. This is how we detect our non-url source has terminated, as this isn't required when using :url option. On stall, we simply close the ffmpeg process, and this allows the Enum.take/2 call to continue, and the player process will exit normally. If playing a short audio sample, from youtube-dl for example, after the first burst of frames, all subsequent watchdogs will have the 200 millisecond timer, so the event will be sent right as we detect the audio concludes.

@BrandtHill BrandtHill changed the title Implement playing audio through discord voice connections in Nostrum. Implement playing audio through discord voice connections. Oct 15, 2020
@Kraigie Kraigie merged commit d0decb5 into Kraigie:master Oct 18, 2020
@Kraigie
Copy link
Owner

Kraigie commented Oct 18, 2020

Thanks!

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.

Voice state updates not handled
3 participants