Closed Yasumoto closed 1 year ago
Hi, @Yasumoto, Thanks for opening this PR :blue_heart: . Contributors :people_holding_hands: like you make the open source community :earth_africa: such an amazing place to learn :book: , inspire :angel:, and create :art: . We will review it :eyes: and get back to you as soon as possible :+1: . Just make sure you have followed the contribution guidelines.
By that time enjoy this meme :point_down: , hope you like it :smile:
Use this action on your projects. Use jokes on issues instead.
first of all thank you much for your love for the project and now this contribution!! I'll look into this ASAP :)
@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.
Yes of course! 🎉
No rush at all, this is for fun when we have spare time 🎶 🎧
And if there are testing steps I can help run, please feel free to point me in that direction 👍
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.
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.
@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.
@all-contributors please add @Yasumoto for feature, tests and code
@deep5050
I've put up a pull request to add @Yasumoto! :tada:
@all-contributors please add @Yasumoto for ideas, tests and code
@deep5050
I've updated the pull request to add @Yasumoto! :tada:
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)
@all-contributors please add @Yasumoto for tests and code
@deep5050
@Yasumoto already contributed before to test, code
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 likeIntRange
if you prefer!