feat: Allow logarithmic amplification

[daalfox]

Adds volume control in decibels discussed in #714

[daalfox]

This is my first contribution on this crate. Just checking in with you about the general approach for this. Is it ok?

[dvdsk]

I understand the choices you made however I think we can do better. Let me know what you think of the feedback.

Further more did you look at the resource Roderick linked in the issue? It features a slightly more involved calculation that leads to a nicer volume curve. We could use that instead or have both the current log and that nicer one.

[daalfox]

Yeah I wanted to keep myself in the same page with you. I just skimmed through the article I will check it out. Thanks for review

[roderickvd]

You can look at the code I linked to, which compresses that article into 10 lines of code. You can copy and/or modify those lines from me under public domain.

[daalfox]

You can look at the code I linked to, which compresses that article into 10 lines of code. You can copy and/or modify those lines from me under public domain.

@roderickvd Do I add this to Sink, SpacialSink and ChannelVolume and call it set_normalized_volume or something? Maybe I open a new PR for that?

[dvdsk]

Take a look at source/mod.rs. Do we want a amplify_decibel() in there?

[dvdsk]

@roderickvd Do I add this to Sink, SpacialSink and ChannelVolume and call it set_normalized_volume or something? Maybe I open a new PR for that?

I would also add it to amplify. But you are right, there are many places we deal with volume atm, we will probably want it there too.

[roderickvd]

@roderickvd Do I add this to Sink, SpacialSink and ChannelVolume and call it set_normalized_volume or something? Maybe I open a new PR for that?

I would also add it to amplify. But you are right, there are many places we deal with volume atm, we will probably want it there too.

This is why I liked @PetrGlad's suggestion here: #714 (comment)

If we just add such public helper methods, then the user can plug it where-ever they want.

[daalfox]

So maybe we keep to_linear that is for any decibel value, an internal api and we add a public to_linear_normalized interface to change a linear value from [0.0, 1.0] to a logarithmic one?
Not sure if I'm understanding this currectly

[daalfox]

The main issue I'm having here is that I don't understand how I fit the mapping that @roderickvd suggested into the Amplify interface
To my little knowledge I got from this PR, this Amplify struct just multiplies each sample in the source with the given factor and we don't deal with volume here at all.
@roderickvd Am I missing something?

[roderickvd]

Maybe we need to sketch out & agree on the API first. To me, Sink::set_volume and Source::amplify are the same in that both modify signal amplitude.

// Direct linear gain
sink.set_volume(1.5);  // amplify by 150%
source.amplify(0.5);   // attenuate to 50%

// Using decibels
sink.set_volume(db_to_gain(6.0));  // +6 dB
source.amplify(db_to_gain(-3.0));  // -3 dB

// Using normalized with logarithmic scaling
sink.set_volume(norm_to_log_gain(0.5));   // 50% perceived volume
source.amplify(norm_to_log_gain(0.75));   // 75% perceived volume

[daalfox]

The thing I like about the Amplify is that it just amplifies the samples and doesn't care about the human interpretation of loudness and changing that feels wrong. I also think that just adding some public helper functions feel a lot nicer.
@dvdsk What do you think?

[dvdsk]

Terms

Maybe we need to sketch out & agree on the API first. To me, Sink::set_volume and Source::amplify are the same in that both modify signal amplitude.

Funny, to me setting volume and amplifying are different :)

Lets see what kind of terms we got:

• Volume: absolute signal strength at output
• Gain: change to signal strength before output
• Amplification: unit of gain?

The name of Sink::set_volume follows that definition. Since Sink is connected directly into the output mixer it is the last part of a rodio pipeline so you could say amplifying their is modifying the volume.

amplifies the samples and doesn't care about the human interpretation of loudness and changing that feels wrong

Yes I get that. That makes sense from a programming standpoint. On the other hand human hearing is logarithmic so linear amplification is generally not the best from a UI perspective and thus should not be the default.

Helper vs UI

adding some public helper functions feel a lot nicer

That would make it clear that decibels is just another way of expressing that amplification factor. On the other hand its not super discoverable and using a factor would still be the default.

this:

some_source
  .pausable()
  .fade_in(Duration::from_secs(60))
  .amplify(to_linear(10));

other_source
  .pausable()
  .fade_in(Duration::from_secs(60))
  .amplify(1.1);

is to me less clear then:

some_source
  .pausable()
  .fade_in(Duration::from_secs(60))
  .amplify_by_db(10);

some_source
  .pausable()
  .fade_in(Duration::from_secs(60))
  .amplify_by_factor(1.1);

Its a difficult UI to get right....

[dvdsk]

sink.set_volume(norm_to_log_gain(0.5));   // 50% perceived volume
source.amplify(norm_to_log_gain(0.75));   // 75% perceived volume

I like the use of perceived there, maybe perceived_gain would work somewhere. amplify_with_perceived_increase (using increase since is an easier word then gain).

[dvdsk]

I agree with @roderickvd btw and I think we should keep going back and forth till we all agree we have the best possible API.