Skip to content

Add method to specify the output sample rate - #9

Merged
DoumanAsh merged 4 commits into
DoumanAsh:masterfrom
ginnyTheCat:out-samplerate
Apr 11, 2026
Merged

Add method to specify the output sample rate#9
DoumanAsh merged 4 commits into
DoumanAsh:masterfrom
ginnyTheCat:out-samplerate

Conversation

@ginnyTheCat

Copy link
Copy Markdown
Contributor

I made the function accept a Option<NonZeroU32>, but I would be open to replace it with a enum of the 9 accepted values.

Additionally to this change I have 3 possible options + pseudocode, that I'm unsure which one would be best for the future:

  1. Do Nothing
  2. Rename function and mark old one as deprecated
fn set_input_sample_rate(rate) { lame_set_in_samplerate(rate); }

#[deprecated]
fn set_sample_rate(rate) { self.set_input_sample_rate(rate); }
  1. Make function set both sample rates. This is what FFmpeg currently does. This would, however, change the behaviour of the function by probably having worse compression rates (I haven't tested this, it's just a guess) and returning errors for values that were previously accepted.
fn set_input_sample_rate(rate) { lame_set_in_samplerate(rate) }

fn set_sample_rate(rate) {
    self.set_input_sample_rate(rate);
    self.set_output_sample_rate(rate);
}

@DoumanAsh

Copy link
Copy Markdown
Owner

I think it is fine to just have it as separate function for now
I didn't consider adding it because mp3lame default behaviour is usually what you want and I was only concerned with input sample rate as you need to adjust from default value always
On other hand defaulting output sampling to mp3lame will probably always best choice, unless user knows what he is doing

So this PR is fine as it is

I would be open to replace it with a enum of the 9 accepted values.

I do not really see much value to it as these values are basically constants you to use as enums

You can make PR to introduce enum that gets converted into Option<NonZeroU32> to avoid breaking change, there is not much reasong make semver breaking change as enums do not add anything other than documenting acceptable values

Comment thread src/lib.rs Outdated
@ginnyTheCat

Copy link
Copy Markdown
Contributor Author

values are basically constants you to use as enums

I agree. I thought you might prefer it since the Bitrate enum is also more restrictive than what LAME supports, I think.

Comment thread src/lib.rs
@DoumanAsh

DoumanAsh commented Apr 11, 2026

Copy link
Copy Markdown
Owner

I agree. I thought you might prefer it since the Bitrate enum is also more restrictive than what LAME supports, I think.

Ah it might, I do not remember now much, but I think I added Bitrate enum because there was quite a lot of allowed values so I wanted to make sure I do not make mistake with units

Sample rate at least a bit more easier as there small range of values

@DoumanAsh
DoumanAsh merged commit 147281e into DoumanAsh:master Apr 11, 2026
@DoumanAsh

Copy link
Copy Markdown
Owner

Thanks, I'll release new version soon

@ginnyTheCat
ginnyTheCat deleted the out-samplerate branch April 11, 2026 14:01
@DoumanAsh

Copy link
Copy Markdown
Owner

@ginnyTheCat I released 0.2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants