Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: get audioBitrate instead of audioRate #14

Merged

Conversation

xage93
Copy link
Contributor

@xage93 xage93 commented Jan 22, 2024

This PR ensures we get audio bitrate instead of audio rate in output_getters file. Fixes the same issue as the one fixed here for output_setters.

Test Proof

Code being tested:

package main

import (
	"fmt"
	"os"

	fluentffmpeg "github.com/modfy/fluent-ffmpeg"
)

func main() {
	Cmd := fluentffmpeg.NewCommand("").
		Options("-hide_banner").
		InputPath("./audio (1)_input.wav").
		OutputFormat("mp3").
		OutputLogs(os.Stdout).
		OutputPath("./audio.mp3").
		AudioBitRate(64000).
		AudioRate(22000)

	bitRate := Cmd.Args.GetAudioBitrate()
	sampleRate := Cmd.Args.GetAudioRate()
	fmt.Println(bitRate)
	fmt.Println(sampleRate)
}

Screenshots of the code to test and the output. Basically set the output bitrate and sample rate to 64000 and 22000 respectively.

Then fetched the same and printed out. Shows the correct settings.

Before Changes - (using current release v0.1.0)

Screenshot 2024-01-23 at 12 00 51 AM

You can see that it's only setting the audioRate and then getting audioRate as well because both setter and getter use audioRate instead of audioBitrate.

After Changes

Screenshot 2024-01-23 at 12 00 43 AM

While I fixed the setter in the other PR (#13), this PR fixes the getter as well.

@xage93
Copy link
Contributor Author

xage93 commented Jan 22, 2024

bump @CryogenicPlanet could you please merge this PR as well that fixes the issue in output_getters file.
And also would love it if a release could be done with the changes so I can use the fixed version in my code. Thanks 🙏

@xage93
Copy link
Contributor Author

xage93 commented Jan 22, 2024

@CryogenicPlanet I've added a screenshot as test proof on your suggestion in the PR description. I've run the exact scenario I want to run in my own code which is to set the output bitrate and sample rate.
You can see that the bitrate and sample rate both are set as intended after the code change.

Copy link
Member

@CryogenicPlanet CryogenicPlanet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CryogenicPlanet CryogenicPlanet merged commit a574c4f into scalarhq:master Jan 22, 2024
2 checks passed
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