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

NewContext can be called only once #60

Closed
AkselsLedins opened this issue Jan 23, 2019 · 14 comments
Closed

NewContext can be called only once #60

AkselsLedins opened this issue Jan 23, 2019 · 14 comments

Comments

@AkselsLedins
Copy link

Hello FYI,

image

It's may be related to 3459272

And maybe it belongs to : https://github.com/faiface/beep

I don't know I'm not sure

Thanks

@hajimehoshi
Copy link
Member

I think this is an implementation issue in beep

@faiface ?

@hajimehoshi
Copy link
Member

Looks like beep calls NewPlayer only once in Init.

What kind of code are you trying to execute?

@AkselsLedins
Copy link
Author

https://github.com/HowlSlack/app/blob/master/howl/audio.go#L20

It's a pretty simple app that reads incoming messages from a slack channel and then reads them out loud.
But I use beep that internally use your awesome lib.

I should propose to make a go.mod file for faiface/beep to avoid breaking changes.

@faiface

@hajimehoshi
Copy link
Member

hajimehoshi commented Jan 23, 2019

My guess is that beep's speaker.Init must be called only once, thought this is not documented. Actually Oto doesn't make this (whether oto.NewPlayer is called multiple times or not) explicit, so beep is not to blame. It depends on @faiface how to solve this problem: allow to call speaker.Init multiple times by fixing implementation, or do not allow explicitly.

@AkselsLedins
Copy link
Author

Thank you @hajimehoshi for your time.
I really appreciate your work.

@hajimehoshi
Copy link
Member

Oh have you solved this?

Thank you for compliments!

@AkselsLedins
Copy link
Author

HowlSlack/app@cfc24b3

I did what you suggested. The issue is that this way the speaker has a fixed sample rate. Imagine you receive audio from multiple sources with different sample rates: that could not work.

I did a workaround to resample the audio streams:

HowlSlack/app@74253c7

Only thing that is mysterious to me is that I ask an audio stream with 22050 sample rate to AWS Polly I need to set the speaker's sample rate to 12000 in order to play the sound correctly. And if I ask an audio stream with a 16000 sample rate I need to set the speaker sample rate to 8000 in order to play it properly.

IMHO, But all of this is more a problem in my code, or aws polly, or beep package issue than oto

@faiface
Copy link
Contributor

faiface commented Jan 28, 2019

@AkselsLedins Yeah, resampling is what you're supposed to do when you have different sample rates.

Btw, @hajimehoshi, in Beep, you're supposed to call Init once at the beginning, but you're also allowed to call it later to reset the speaker and change configuration, such as sample rate, etc. How would that be achieved now?

@hajimehoshi
Copy link
Member

hajimehoshi commented Jan 28, 2019

you're also allowed to call it later to reset the speaker and change configuration, such as sample rate, etc. How would that be achieved now?

Oto doesn't allow it? Now unfortunately it is impossible to reset the sample rate so far.

@faiface
Copy link
Contributor

faiface commented Jan 28, 2019

So, Oto does or does not allow it? It did allow it before, and Beep was using that.

@hajimehoshi
Copy link
Member

hajimehoshi commented Jan 28, 2019

No, Oto doesn't allow it. Even if you could do it, you were just lucky...

Of course we can change the specification and fix the implementation (perhaps we wouldn't need any change but at least we need to double-check). However, IMHO, I don't think changing sample-rate is an important feature...

@cswank
Copy link

cswank commented Oct 23, 2021

Changing sample rate is important for my app (GitHub.com/cswank/mcli). I have a collection of flac files that I bought from Bandcamp and so far there are 3 different sample rates in the collection. Calling beep’s speaker.Close() and then speaker.Init() with a new sample rate works fine on a Linux machine but hangs somewhere in the oto code on MacOS. I’ll be digging in to find out more, but I thought I’d just mention the fact for now.

@hajimehoshi
Copy link
Member

hajimehoshi commented Oct 24, 2021

Please see #149 for the discussion to implement this.

@cswank
Copy link

cswank commented Oct 24, 2021

Thank you for the reply, that's good to know.

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

No branches or pull requests

4 participants