search

roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

Added --list-drivers -L support Issue:251 #257

Closed tomd1990 closed 5 years ago

tomd1990 commented 5 years ago

Made the get_drivers function an abstract function for the iBackend class. Also updated the ggo/ main.cpp files accordingly.

Currently sox can support all of the following formats/ audio device drivers (based on what sox_get_format_fn gives me)

AUDIO FILE FORMATS: aifc aiffc aiff aif al au snd avr cdda cdr cvsd cvs cvu dat dvms vms f4 f32 f8 f64 gsrt hcom htk ima la lu maud prc raw s1 s8 sb s2 s16 sw s3 s24 s4 s32 sl sf ircam sln smp sndr sndt sox sph nist 8svx txw u1 u8 ub sou fssd u2 u16 uw u3 u24 u4 u32 ul voc vox wav wavpcm amb wve xa gsm lpc10 lpc PLAYLIST FORMATS: m3u pls AUDIO DEVICE DRIVERS: alsa ossdsp oss pulseaudio

I wasn't sure if there needs to be any separation between Audio FIle formats and device drivers, so I just bundled them together in the output.

gavv commented 5 years ago

Great! I'll take a look.

Regarding to formatting. I guess you have run scons fmt right? What version of clang-format do you use? I use 7.1.0 and it seems that the formatting is a bit different (your version removed some spaces?). In this case I think we should choose the most recent version of these two.

gavv commented 5 years ago

/link #251

gavv commented 5 years ago

I wasn't sure if there needs to be any separation between Audio FIle formats and device drivers, so I just bundled them together in the output.

I think it would be convenient to print them separately, like:

device drivers:
  alsa
  pulseaudio
  ...

file drivers:
  wav
  ...

We already have IBackend::ProbeFlags. We can rename it to something like FilterFlags and then use them like this:

backend_dispatcher.get_drivers(arr, FilterDevice);
// print
backend_dispatcher.get_drivers(arr, FilterFile);
// print
gavv commented 5 years ago

I've reviewed this, see the comments above. Most of them are cosmetic thought.

dshil commented 5 years ago

@tomd1990, a small nitpick about the commit message.

You have:

 Added --list-drivers -L support Issue:251

It would be nice to have something similar to:

Added --list-drivers -L support

Relates: #251

I want to separate the commit title and the issue reference. Also note that we can use # before the issue number, Github will link this issue with the current PR.

tomd1990 commented 5 years ago

Great! I'll take a look.

Regarding to formatting. I guess you have run scons fmt right? What version of clang-format do you use? I use 7.1.0 and it seems that the formatting is a bit different (your version removed some spaces?). In this case I think we should choose the most recent version of these two.

Sorry for the late response,

I ran scons fmt, but my clang-format is definitely older, version 6.0.0. This probably explains the difference. I'll update and run it again.

Thanks for all of the feedback, Ill make these changes soon.

tomd1990 commented 5 years ago

I'm closing this PR as merging this would make things messy, I'll submit the commit in a new PR