-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix(song.ts): changing play-dl to ytdl-core in Song.ts makeResource() #1645
base: master
Are you sure you want to change the base?
fix(song.ts): changing play-dl to ytdl-core in Song.ts makeResource() #1645
Conversation
thanks for fixing! |
I am convinced that it would be worthwhile to completely replace the youtube-sr library with ytdl-core to maintain uniformity and improve clarity, but that's rather a larger topic for potential future consideration. Here's a quick fix ready. |
6b21834
to
4ab3a5a
Compare
4ab3a5a
to
4bb7c0f
Compare
If a longer video is played, the following error occurs and the playback is interrupted after some time:
|
I think this problem comes from the library that was used to replace play-dl. As we see here in the play-dl documentation , ytdl is already known for this error. Play-dll.stream issue play-dl/play-dl#371 |
My bad ofc i replaced play-dl.stream() not youtube-sr as stated in pull name. I must have been thinking about something else while commiting... |
Thank you so much for your contribution! Your code solved the issue perfectly. I couldn't wait for it to be merged, so I went ahead and used it already. @crackodille |
Thank you for your fix @crackodille |
can also confirm it worked on a docker environment |
How did you access the file to make the change tho? I cannot find the song.ts file in the container whatsoever. |
I would suggest we don't replace play-dl with ytdl-core. Another bot I use with ytdl-core regularly has issues as well. What might be nice is having a flag or smart logic to bounce between the two. |
I checked-out the branch with the fix (https://github.com/crackodille/evobot/tree/issue-1644-fix-with-ytdl), built the container locally and pushed to a private registry And if you're hosting the container locally, you don't even need to push it to a remote registry |
Works fine, thanks ! |
Seems to be giving 403 now :(
|
Thank you! EDIT: NVM I also got 403 |
same error as well |
I think I found where it fails try {
const resource = await next.makeResource();
this.resource = resource!;
this.player.play(this.resource); // -- AudioPlayerError: Status code: 403 --
this.resource.volume?.setVolumeLogarithmic(this.volume / 100);
} catch (error) {
console.error(error);
return this.processQueue();
} finally {
this.queueLock = false;
} The ressource could maybe create an issue but I also found an issue on the main repo so I am not sure if @crackodille 's changes on Song.ts are related to this error. It might be a direct error from discordjs/voice dependency |
The solution for error 403, You can follow the issue here |
Submitted a pull request to @crackodille's branch that integrates @paulodsncir's new solution. Can confirm it works. |
Use @distube/ytdl-core
@paulodsncir @CoocooFroggy nice work. merged :) |
Can confirm the new updated code to be working |
Unfortunately new problems. Error: While getting info from url youtube is not sleeping I guess. They are trying to eliminate bots. |
@crackodille I saw a possible workaround in this issue, maybe it could help. I didn't have the opportunity to test it. YouTube is giving me so much headache, I just wanted to listen to music with my friends |
I have a question |
fix #1644