discordjs / opus

Native opus bindings for node
MIT License
192 stars 55 forks source link

feat(opus): add `adjustBitrate` function #143

Closed dager-mohamed closed 8 months ago

dager-mohamed commented 1 year ago

Please describe the changes this PR makes and why it should be merged:

Semantic versioning classification:

OpusEncoder::SetBitrate is a function that sets the bitrate of the encoder to a specific value that is passed as an argument. It directly sets the bitrate to the provided value, regardless of the current bitrate of the encoder.

On the other hand, OpusEncoder::AdjustBitrate is a function that adjusts the current bitrate of the encoder by a certain value that is passed as an argument. It does not set the bitrate to a specific value, but rather it increases or decreases the current bitrate by a certain amount.

For example, if the current bitrate of the encoder is 64kbps and you call OpusEncoder::AdjustBitrate(32000), the new bitrate will be 96kbps (64kbps + 32kbps).

This function is more flexible than OpusEncoder::SetBitrate, it allows you to adjust the bitrate by a certain amount.

for a clear explanation, The main difference between the OpusEncoder::AdjustBitrate and OpusEncoder::SetBitrate methods is that OpusEncoder::SetBitrate is typically used to set the bitrate of the encoder before the encoding process begins, whereas OpusEncoder::AdjustBitrate is used to change the bitrate of the encoder during the encoding process.

The OpusEncoder::SetBitrate method is used to set the bitrate of the encoder to a specific value before the encoding process begins. It takes an integer argument representing the target bitrate in bits per second, and it uses the Opus codec library's OPUS_SET_BITRATE control command to set the bitrate of the encoder.

dager-mohamed commented 1 year ago

The implementation of OpusEncoder::AdjustBitrate() looks the same as the existing OpusEncoder::SetBitrate()

it is not because setBitrate is setting the bitrate before the application or processing start but the adjustBitrate is can change the bitrate during the app running or process running

twlite commented 12 months ago

The implementation of OpusEncoder::AdjustBitrate() looks the same as the existing OpusEncoder::SetBitrate()

it is not because setBitrate is setting the bitrate before the application or processing start but the adjustBitrate is can change the bitrate during the app running or process running

both seem to use opus_encoder_ctl(this->encoder, OPUS_SET_BITRATE(value)), what's the difference? Plus you are not checking if encoder exists in first place (encoder is lazy loaded so you should call EnsureEncoder()). AdjustBitrate seems unnecessary to me.

// existing
void OpusEncoder::SetBitrate(const CallbackInfo& args) {
    Napi::Env env = args.Env();
    int bitrate = args[0].ToNumber().Int32Value();
    if (this->EnsureEncoder() != OPUS_OK) {
        Napi::Error::New(env, "Could not create encoder. Check the encoder parameters").ThrowAsJavaScriptException();
        return;
    }
    if (opus_encoder_ctl(this->encoder, OPUS_SET_BITRATE(bitrate)) != OPUS_OK) {
        Napi::TypeError::New(env, "Invalid bitrate").ThrowAsJavaScriptException();
        return;
    }
}

// your implementation
void OpusEncoder::AdjustBitrate(const CallbackInfo& args) {
    Napi::Env env = args.Env();
    int targetBitrate = args[0].As<Napi::Number>().Int32Value();
    int error = opus_encoder_ctl(this->encoder, OPUS_SET_BITRATE(targetBitrate));
    if (error != OPUS_OK) {
        Napi::Error::New(env, std::string("Error adjusting bitrate: ") + opus_strerror(error)).ThrowAsJavaScriptException();
        return;
    }
}