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

Add an example to play an mp3 #18

Merged
merged 1 commit into from
Aug 6, 2017
Merged

Conversation

svanharmelen
Copy link
Contributor

@svanharmelen svanharmelen commented Aug 6, 2017

Tried to keep the style inline with the other examples

Fixes #16

Copy link
Owner

@gordonklaus gordonklaus left a comment

Choose a reason for hiding this comment

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

Looks good but I'm not sure it works.

examples/mp3.go Outdated

portaudio.Initialize()
defer portaudio.Terminate()
out := make([]int32, 8192)
Copy link
Owner

Choose a reason for hiding this comment

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

Here you effectively assume that encoding is mpg123.ENC_SIGNED_32. You should either assert that, make it so, or handle different encodings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, will fix in the next update.

chk(stream.Start())
defer stream.Stop()
for {
audio := make([]byte, 2*len(out))
Copy link
Owner

Choose a reason for hiding this comment

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

2*len(out) is half as many bytes as are necessary to fill out. I'd expect you to hear some pretty bad audio artifacts as a result of the the latter half of out containing silence.


// create mpg123 decoder instance
decoder, err := mpg123.NewDecoder("")
chk(err)
Copy link
Owner

Choose a reason for hiding this comment

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

chk is not defined. I get the feeling you didn't run this code 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't run this exact code no. That's not possible because main() is defined in all files in the examples dir 😏

But I should indeed have double checked this example by copying the code to a different dir and test it. Will do that and update in a bit...

Copy link
Owner

Choose a reason for hiding this comment

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

Hehe, yeah the easy way is just to do go run mp3.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good one! Thanks for the pointer/suggestion.

@svanharmelen
Copy link
Contributor Author

@gordonklaus just updated the example and double checked it by running this exact code as a standalone program...

@gordonklaus gordonklaus merged commit e66c30a into gordonklaus:master Aug 6, 2017
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