serenity-rs / songbird

An async Rust library for the Discord voice API
ISC License
381 stars 108 forks source link

Add ytdl search #210

Closed cycle-five closed 7 months ago

cycle-five commented 7 months ago

Adds to YoutubeDl the ability to just return the search results as text.

cycle-five commented 7 months ago

What are your linter settings? Mine seem to be conflicting.

GnomedDev commented 7 months ago

You need to run cargo +nightly fmt.

cycle-five commented 7 months ago

I should have commented originally with the impetus for this feature. I am currently using it in my bot, albeit it is not fully implemented, however the entire purpose is to do a search and return options which the client can then choose from, before it plays the song.

cycle-five commented 7 months ago

I think that this feature is needed and useful, but there's work needing done to make it fit and function as expected if you're able to.

I am more than happy to work on this to have it fit properly into songbird, it is absolutely needed for a feature I'm using in my bot, so as long as I can make that work, I'm happy to make whatever changes you feel are appropriate.

FelixMcFelix commented 7 months ago

Thanks, I'm just keen that it works great for everyone. The main assumption with Compose is that you should be able to 'just play it', ideally. I.e., new_yt_search should produce an object that will play a search result, or produce a different struct entirely (which does not support Compose). I hope that makes sense?

cycle-five commented 7 months ago

I believe I've made all the requested changes and have got it working with these changes in my bot (which of course is the most important piece in this equation).

I feel that my approach to using the yt-dlp search feature and parsing that into the AuxMetadata is either exactly as it is intended, or completely wrong.

Also for the future I believe yt-dlp may support other searches, and also are there plans to not need yt-dlp?

FelixMcFelix commented 7 months ago

Thanks for making changes, I'll take a proper look in the next few days.

I feel that my approach to using the yt-dlp search feature and parsing that into the AuxMetadata is either exactly as it is intended, or completely wrong.

It looks valid at a glance, thank you. If there are any changes then I'll either push to your branch or provide a patchset.

Also for the future I believe yt-dlp may support other searches,

We can support these as and when they come up.

and also are there plans to not need yt-dlp?

You don't need yt-dlp for songbird, but practically speaking you need it to get any usable data from youtube given the way it works.

FelixMcFelix commented 7 months ago

@cycle-five please have a look and see that you're okay with the changes I've made here:

cycle-five commented 7 months ago

Awesome! I'm just going to pull this update and make sure my bot can integrate apropos.

cycle-five commented 7 months ago

This works exactly as I need, so it has my OK.