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

Add option to set volume #18

Merged
merged 3 commits into from
Aug 27, 2023
Merged

Conversation

Yasumoto
Copy link
Contributor

Thanks so much for writing this! It's awesome 👏🏼

This passes through a value to ffplay setting the initial volume of the stream.

This does make the --help output very verbose, happy to change this to use something like IntRange if you prefer!

This passes through a value to ffplay setting the initial volume of the stream.
@github-actions
Copy link

Hi, @Yasumoto,
Thanks for opening this PR 💙 .
Contributors 🧑‍🤝‍🧑 like you make the open source community 🌍 such an amazing place to learn 📖 , inspire 👼, and create 🎨 .
We will review it 👀 and get back to you as soon as possible 👍 . Just make sure you have followed the contribution guidelines.

By that time enjoy this meme 👇 , hope you like it 😄

meme

Use this action on your projects. Use jokes on issues instead.

@deep5050
Copy link
Owner

first of all thank you much for your love for the project and now this contribution!! I'll look into this ASAP :)

@deep5050
Copy link
Owner

deep5050 commented Oct 7, 2021

@Yasumoto Sorry, could not check your PR. I'm kinda busy these days. Initial checks looks good, have to run on multiple environments before I merge this. Hope you understand.

@Yasumoto
Copy link
Contributor Author

Yasumoto commented Oct 7, 2021

Yes of course! 🎉

No rush at all, this is for fun when we have spare time 🎶 🎧

@Yasumoto
Copy link
Contributor Author

Yasumoto commented Oct 7, 2021

And if there are testing steps I can help run, please feel free to point me in that direction 👍

@deep5050
Copy link
Owner

deep5050 commented Oct 7, 2021

basically have to check whether -volume was introduced on latest versions or it is there already from the older versions of ffmpeg. I guess its there from the first release, if not, have to compare the version and handle things accordingly. If you can run it on a ubuntu ( or other debian) and windows 10, it would be great. should not be an issue i guess. but have to make sure.

Also any other feature you suggest.. will be much appreciated.

@Yasumoto
Copy link
Contributor Author

Yasumoto commented Oct 7, 2021

Looks like the first major release with it was 4.0. Do you still want to support Ubuntu 18.04, which has version 3?

If so we should only set that option if ffplay is version 4 or greater, otherwise looks like this should be available in modern distros.

@deep5050
Copy link
Owner

deep5050 commented Oct 8, 2021

@Yasumoto yes, we have to handle both the cases. Ubuntu 18.04 is still being used. just check for the ffmpeg version. if >=4 add volume to the command else add a DEBUG info saying its not supported. If a user try to set volume on older than v4,we have to notify him (a simple WARN log). on either case ffplay should start because volume is not a mandatory option.

@deep5050

This comment has been minimized.

@allcontributors

This comment has been minimized.

@deep5050
Copy link
Owner

deep5050 commented Oct 8, 2021

@all-contributors please add @Yasumoto for ideas, tests and code

@allcontributors
Copy link
Contributor

@deep5050

I've updated the pull request to add @Yasumoto! 🎉

@deep5050
Copy link
Owner

changing volume options from (0-100) to (0-100 with steps 10)

gitpod /workspace/radio-active (yasumoto-volume) $ radio --volume
usage: radio-active [--version] [--help] [--station STATION_NAME] [--uuid STATION_UUID] [--log-level LOG_LEVEL]
                    [--discover-by-country DISCOVER_COUNTRY_CODE] [--discover-by-tag DISCOVER_TAG] [--discover-by-state DISCOVER_STATE]
                    [--discover-by-language DISCOVER_LANGUAGE] [--limit LIMIT] [--add-station] [--add-to-favourite ADD_TO_FAVOURITE]
                    [--show-favourite-list] [--random] [--flush]
                    [--volume {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99}]
radio-active: error: argument --volume: expected one argument
gitpod /workspace/radio-active (yasumoto-volume) $ radio --volume 89
usage: radio-active [--version] [--help] [--station STATION_NAME] [--uuid STATION_UUID] [--log-level LOG_LEVEL]
                    [--discover-by-country DISCOVER_COUNTRY_CODE] [--discover-by-tag DISCOVER_TAG] [--discover-by-state DISCOVER_STATE]
                    [--discover-by-language DISCOVER_LANGUAGE] [--limit LIMIT] [--add-station] [--add-to-favourite ADD_TO_FAVOURITE]
                    [--show-favourite-list] [--random] [--flush] [--volume {0,10,20,30,40,50,60,70,80,90,100}]
radio-active: error: argument --volume: invalid choice: 89 (choose from 0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100)

@deep5050 deep5050 merged commit 4b14893 into deep5050:main Aug 27, 2023
1 check failed
@deep5050
Copy link
Owner

@all-contributors please add @Yasumoto for tests and code

@allcontributors
Copy link
Contributor

@deep5050

@Yasumoto already contributed before to test, code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants