Audio Library

Status
Not open for further replies.
Looks good to me so far.
I will have a look through the code I've done to make sure it conforms to the function/parameter convention.

Pete
 
Hi everybody,
I will to reproduce some sound by headphones using the Audio adaptor, but i need to know how much is the range in Wattios on the output of the jack (max and min)? how much it change with the volume?
 
@Paul:
[edit]You have suggested function names for Playback objects. What about filters and effects? Would .begin (or .start?) and .stop be better for them?

I have made a pull request with 4 mods so far and I can add one for the FIR code once begin/start/play is decided.

Pete
 
Last edited:
I'm looking at making my code conform to the guidelines and currently I'm wondering whether the use of .begin or .start would be appropriate for filters and effects since they don't actually 'play' in the way that playback objects do.

In the chorus and flange effects I have a function 'modify' which allows you to change the effect on-the-fly. For example, after the chorus effect is running, modify can be used to change the number of 'voices' in the chorus. Is that name OK?

Pete
 
Filters and effects probably should not need begin() and end(). Several objects do currently have a begin() function that merely initializes the object. Those should all become part of the constructors, if there's never a need to use begin() and end() from Arduino sketches.

For an effect like chorus, it's probably best to name the function voices(), which indicates what it actually does. Arduino's style guide says "Use full, everyday words .... Pick terms that correspond to popular perception of the concept at hand."
 
Good morning :)

@drjohn: Did you ever find those files? I went looking for files with specifically those names anywhere on the internet and only found similes, no precise matches - probably intention is that you bring your own files but it seems remiss to me that there is no mention of either that or where to find them.

@Pete: Any chance you would consider changing the necessary in AudioSynthWaveform to make it support fractions of frequencies? Some people like to play in fractions :)


@Paul:
Complete agreement about object naming.

The terminology you use regarding function & parameter conventions makes it sound like you want to limit things like gain to 1.0, please confirm that you do not intend to block end users from doing things like myMixer.gain(1.1); (or even .gain(50);) - if you do intend such a block I think it is bad.

Complete agreement about playback and control status.

Mostly agree about Analysis Output pending what is considered as analysis perhaps - I'll come back to this if I don't decide I'm wrong (or that it is hardly important, no detriment etc etc.)
 
Last edited:
FWIW: The problem with using voices() for the chorus effect is that for other effects (e.g. flange) the name voices() isn't applicable. To change what flange does requires a name other than voices and other effects/filters will probably have their own unique function which changes what they do. I think it might be better to have a 'paradigm' such as:
begin, modify, stop (or pause?), end
which applies to the whole library. In each specific effect/filter/etc, modify might require a different number of arguments but its intent/functionality is clear throughout the library.

Pete
 
Thanks very much, about fractions, Pete.



@everybody: I should have asked this before but it just didn't occur to me; How many people would prefer method/function names like '.dap_enable()' were '.dapEnable()' instead? or even just '.dapenable()' - It has occurred to me that maybe all those underscores suck and certainly don't value add, am I wrong?

If there is much response in favor of ditching the underscores I will happily submit an update doing so - if anybody wants to propose new names for any of them I'd like to know.
 
Last edited:
I've changed audioSynthWaveform to use a floating point frequency although I wouldn't expect that a fractional frequency would have any detectable effect except at very low frequencies.
Once I've got all the function names straight, the fix will be in a pull request but if you're in a rush, you can change your copy of synth_waveform.cpp and synth_waveform.h so that begin() has this definition:
Code: 
boolean AudioSynthWaveform::begin(float t_amp,float t_hi,short type)
It doesn't require any other changes.

Pete
 
Pete, in synth_waveform.h
Code: 
short tone_freq
would need to change to a float as well, wouldn't it?

As an example of why fractions of frequencies is better than integer frequencies: Not many musical notes are integer frequencies, 'A' being the only I can think of that is integer in all octaves.

E2 is 82.406Hz and it is pretty flat if you have to settle for 82.0Hz to represent it.
 
tone_freq is fine as a short integer (as long as the library doesn't increase it's sampling above 65535Hz!). It is only used to turn the tone on (tone_freq!=0) or off(==0).

Not many musical notes are integer frequencies
Click to expand...
Those are the low frequencies I was thinking of. Let me know if you find that the musical tones are off - I don't have perfect pitch .
When Paul incorporates my current pull request, AudioSynthWaveform will produce much better tones than it does now. The William Tell Overture in PlaySynthMusic will sound much better.

Pete
 
robsoles said:
Good morning :)

@drjohn: Did you ever find those files? I went looking for files with specifically those names anywhere on the internet and only found similes, no precise matches - probably intention is that you bring your own files but it seems remiss to me that there is no mention of either that or where to find them.
Click to expand...

Never found them. I would throw arbitrary files names in there, but my head infers short with the S suffix'd one and medium length with the M.
 
Hey Doc, my guess was 'S'tereo and 'M'ono but your guess is as valid as mine until Paul sets us straight :)

Pete, I read past 'tone_freq=t_hi;' and see tone_incr is the one to watch. I don't see why the tones would be 'off' unless the clock the whole thing is based on is off - if we commanded .begin(1,1000,TONE_TYPE_SINE); before and our cro/oscilloscope showed 1000Hz with little or no deviation then I expect the change to float in that equation should simply enable 987.77hz being close enough to a B5 to make most musically inclined people happy enough that it is in tune.
 
Last edited:
robsoles said:
How many people would prefer method/function names like '.dap_enable()' were '.dapEnable()' instead? or even just '.dapenable()' - It has occurred to me that maybe all those underscores suck and certainly don't value add, am I wrong?
Click to expand...

Personally I find camelCase to be a little easier to read, under_scores to make it read a little slower and everythinginoneblobwithneithercamelcasenoreunderscorestosegmentwords to be the worst.

I'm reliably informed that blind people using text to speech converters find camelCase the most legible, too (meaning its pronounced as several words close together, rather than longwinded "under UNDERSCORE score" or nonsense, respectively).
 
robsoles said:
The terminology you use regarding function & parameter conventions makes it sound like you want to limit things like gain to 1.0, please confirm that you do not intend to block end users from doing things like myMixer.gain(1.1); (or even .gain(50);) - if you do intend such a block I think it is bad.
Click to expand...

Good point. I believe the intention is to abstract numbers rather than exposing meaningless, or subject to change, internal values that would change as the hardware changes. So a volume of 0.0 (silent) to 1.0 (full volume) is better than 0 to 2047 for example. Therefore, a gain of 0.0 (mute) 1.0 (unity) 50 (gain of 50) is in keeping with the spirit of that intention (and much better than, say, 0.0, 0.02, and 1.0 for the same meanings because remembering that 0.02 means unity gain is just as irritating as having to remember some other magic number).

Similarly, if level were being expressed in dBU its better to use the actual units (4.0 for +4dBU, 24.0 for +24.0dBU, -96.0 for -96 dBU) because these have external objective meaning across multiple hardware implementations. (+4dBU is the nominal line level for professional recording equipment).

In terms of writing specifications, this is the classic MUST vs. SHOULD.
 
The Arduino style guide definitely calls for camelCase. Inside the library either is fine, for local and private variables and private functions. I personally prefer underscores, and I'm perfectly happy to accept pull requests with almost any style for internal stuff. Following the style guide is important for the public functions.

Regarding uniformity and consistency across the whole library, I believe there are trade-offs. Again, I want to follow the Arduino style guide. Specifically regarding naming of functions in the effects:

To change what flange does requires a name other than voices and other effects/filters will probably have their own unique function which changes what they do. I think it might be better to have a 'paradigm' such as:
begin, modify, stop (or pause?), end
which applies to the whole library. In each specific effect/filter/etc, modify might require a different number of arguments but its intent/functionality is clear throughout the library.
Click to expand...

The Arduino style guide definitely calls for descriptive names using whole English words that are easily understood by intelligent people without a background in programming or audio jargon. It recommends "pick terms that correspond to popular perception of the concept at hand".

So I would say yes to using different names specific to each type of effect. While I personally see the technical beauty in a highly uniform API across the whole library, I believe going to very generic terms that lack description of what the function does is straying too far from the Arduino style.

Where I do want to see uniformity is within certain categories, especially Play and Control. Well, at least for the very basic functions in Control. The reason is not to make the entire library more understandable, but rather to allow more easily substituting one object for another. This interchangeability should allow most sketches that use only the basic Control functions to use a different chip only by changing the object name. We already have SGTL5000 and two flavors of WM8731, and over time several other chips are likely to be added. Of course, most chips like the SGTL5000 are loaded with special features, so sketches that use those features won't be able to switch. On the Control category, I started some work early on to define the common set, but like so many things in the library, it needs more work.

Play objects are the other category where I want to make switch between objects easy. Perhaps someone writes a sketch that does some awesome sound effects playing from internal flash memory. Someone else uses the code in another project, but their sounds are longer. Or maybe the memory ones are 11 kHz to save space and the new project high a high-end sound system and wants full 44 kHz samples. The idea behind standard names in the Play class is to allow that new project to reuse the code, but just store the sounds on a SD card (or perhaps the optional SP flash chip someday). Because the Play functions are the same across objects, the same sketch can be used by just changing the object name.

Hopefully this helps a bit? I know it's a lot of not-so-fun work to mess with code that already works. I really to appreciate every contribution you guys have made and I hope this renaming of stuff isn't too painful. Pretty soon one of the major distributors is going to start selling the audio adaptor and there will be a lot more pressure to get a 1.0 release done, so this really is the last opportunity to rename things for the Arduino style and better consistency.


Edit: plenty of the parts I've written don't quite follow the style guide either, or have inconsistencies. Feel free to mention those; my feelings won't be hurt. My main goal is to make this as usable as possible, with the simplicity and style people expect in Arduino.
 
Last edited:
robsoles said:
How many people would prefer method/function names like '.dap_enable()' were '.dapEnable()' instead? or even just '.dapenable()'
Click to expand...

Yes, these and all public functions should follow the Arduino style guide, which calls for camelCase.

The SGTL5000 DAP is particularly challenging for defining a public API. The first issue is just the name itself. Should we keep calling it "dap"? Here's what the Arduino Style Guide has to say:

Use full, everyday words. Don’t be terse with your function names or variables. Use everyday terms instead of technical ones. Pick terms that correspond to popular perception of the concept at hand. Don’t assume specialized knowledge. For example, this is why we used analogWrite() rather than pwm(). Abbreviations are acceptable, though, if they’re in common use or are the primary name for something. For example, “HTML” is relatively common
Click to expand...

I wouldn't know "dap" = "Digital Audio Processor" if I hadn't already read the SGTL5000 datasheet. Then again, spelling out all 3 or even 2 words might too much? I really don't have all the answers here, but I would like to raise the question if anyone can think of a not-too-long name that would be more descriptive?

Once the API is updated, I will write a page on the site to document whatever the names up being, so however this all works out, there will be documentation. Still, the more we can follow the style guide, the better it'll work for Arduino users who don't read the documentation.

The other bit of Arduino's style guide that might really be relevant to the DAP is this:

Organize your public functions around the data and functionality that the user wants. Quite often, the command set for a particular electronic module is overly complicated for the most common uses, or can be re-organized around higher level functionality. Think about what the average person thinks the thing does, and try to organise your API functions around that. Adafruit's BMP085 library is a good example. The readPressure() command performs all the necessary steps to get the final pressure. The library wraps this commonly executed series of functions into a high-level single command which returns the value the user's looking for in a format she expects. It abstracts away not only the low-level I2C commands, but also the mid-level temperature and pressure calculations, while still offering those mid-level functions as public functions for those who want them.
Click to expand...

For a 1.0 release, this probably isn't necessary. Higher level functions can be added pretty easily in future revs. The thing that's tough to change are lots of names and ways of doing things that lots of sketches depend upon.
 
I will change control_sgtl5000.* a great deal based on that I think.

Functions like dap_audio_eq(..) and dap_peqs(..) could just as well be named digAudProcEQset(..) and digAudProcFilters(..) respectively and then functions like equalizerSet(bass,mid_bass,mid,mid_treble,treble) (equalizerSet(band_num,level) too) could check that DAP_AUDIO_EQ->EN is set to GEQ and set it so if not. Likewise toneSet(bass,treble) would make sure DAP_AUDIO_EQ->EN is 2 to select tone and .biquadFilter(filter_num,params) would make sure DAP_AUDIO_EQ->EN is set to PEQ and that DAP_PEQ is equal to (or bigger than) filter_num and so on in that vein.

.digAudProcEnable() isn't that long in the scheme of things and is heaps more likely to be guessed correctly than dapEnable(), right?

Working on the idea of adding a flag to dap_enable() to allow cartere to put the DAP before Teensy is making me think a .route(i2s_out,dac,dap[,dap_mix]) routine would be better;
Code: 
#define SGTL_ADC 0x0
#define SGTL_I2S_IN 0x1
#define SGTL_DAP 0x3
Into control_sgtl5000.h and a shiny new .route() function into control_sgtl5000.cpp
Code: 
setup() {
  ..
  ..
  audioAdapter.enable();
  audioAdapter.route(SGTL_DAP,SGTL_I2S_IN,SGTL_ADC);
  ..
}
Please comment if you have the slightest inclination.
 
robsoles said:
Pete, I read past 'tone_freq=t_hi;' and see tone_incr is the one to watch. I don't see why the tones would be 'off'
Click to expand...

There are two ways to turn sound off. Setting amplitude to zero continues to generate a stream of zero samples. Setting frequency to zero stops the audio generation altogether.

Pete
 
robsoles said:
Functions like dap_audio_eq(..) and dap_peqs(..) could just as well be named digAudProcEQset(..) and digAudProcFilters(..) respectively and then functions like equalizerSet(bass,mid_bass,mid,mid_treble,treble) (equalizerSet(band_num,level) too) could check that DAP_AUDIO_EQ->EN is set to GEQ and set it so if not. Likewise toneSet(bass,treble) would make sure DAP_AUDIO_EQ->EN is 2 to select tone and .biquadFilter(filter_num,params) would make sure DAP_AUDIO_EQ->EN is set to PEQ and that DAP_PEQ is equal to (or bigger than) filter_num and so on in that vein.
Click to expand...

Yes, these sound like excellent changes. Functions with clear purpose that automatically take care of technical details are what makes all the difference for Arduino users.

.digAudProcEnable() isn't that long in the scheme of things and is heaps more likely to be guessed correctly than dapEnable(), right?
Click to expand...

Arduino spells about "digitalWrite" for one of the most commonly used functions and people seem to be really happy with it.

I think "digAudProc" (10 characters) is fine and far better than "dap". "audioProc" (9 char) or even "audioProcess" (12 chars) or "audioProcessor" (14 chars) would be good too. In fact, if you're willing to go with 12 chars, I'd prefer to see both words spelled out. Yeah, it's getting a bit on the long side, but it fits the Arduino style really well.

But really, it's your call on the name. I think any of these is pretty good.

Working on the idea of adding a flag to dap_enable() to allow cartere to put the DAP before Teensy is making me think a .route(i2s_out,dac,dap[,dap_mix]) routine would be better.
Click to expand...

Yes, very nice. Routing the signals is probably how people will think about using the part. Writing these higher level functions that do the things people want and handle all the minor technical details inside the library is exactly the sort of thing that makes for great Arduino libraries. :)
 
Last edited:
Thanks for the elaboration Pete but it wasn't what I was angling at/for - I was on about whether or not the 'musical tones might be "off"', imho they certainly should not be.


Thanks very much Paul, .route(..) today, the world by Thursday perhaps :)


my apologies to anybody who has written much using the existing function names, one prompt for it and I will publish a list of 'original name,new/equiv name' covering every name change - maybe I should find an Arduino help style guide (don't hesitate to link to one below :)) and see if I can bear writing help for control_sgtl5000 once it is much nearer final form?
 
Status
Not open for further replies.
Back
Top