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

Improving data types. #76

Closed
azul3d-bot opened this issue Mar 6, 2016 · 5 comments
Closed

Improving data types. #76

azul3d-bot opened this issue Mar 6, 2016 · 5 comments
Labels
Milestone

Comments

@azul3d-bot
Copy link

Issue by slimsag
Monday Sep 29, 2014 at 23:48 GMT
Originally opened as azul3d-legacy/audio#6


Something that struck me just recently is that if another package defines a function like this:

// Returns unsigned PCM 8-bit audio samples.
func getSamples() []uint8 {
}

We can't directly convert []uint8 to audio.PCM8Samples because it is defined as type PCM8Samples []PCM8 instead of type PCM8Samples []uint8. This is in fact different from the image package which doesn't do this sort of custom type definition for individual pixels.

I think it would make sense to remove the PCM8 type and instead directly use uint8 (this goes for other types like PCM16, F64, ALaw etc).

@azul3d-bot azul3d-bot added this to the Version 2 milestone Mar 6, 2016
@azul3d-bot
Copy link
Author

Comment by slimsag
Monday Sep 29, 2014 at 23:50 GMT


If others agree, it would make sense to rename PCM8Samples and friends to just PCM8 too.

@azul3d-bot
Copy link
Author

Comment by slimsag
Tuesday Sep 30, 2014 at 00:22 GMT


In fact, F32 and F64 types are PCM encoded even though the type doesn't say it (like PCM8 types do). I propose the following renaming:

Old type New type
type ALawSamples []ALaw type ALaw []uint8
type MuLawSamples []MuLaw type MuLaw []uint8
type PCM8Samples []PCM8 type Uint8 []uint8
type PCM16Samples []PCM16 type Int16 []int16
type PCM32Samples []PCM32 type Int32 []int32
type F32Samples []F32 type Float32 []float32
type F64Samples []F64 type Float64 []float64

This opens up the package to support new PCM encoded types like uint16 if needed in the future as well.

@azul3d-bot
Copy link
Author

Comment by mdlayher
Tuesday Sep 30, 2014 at 00:33 GMT


👍, looks good to me!

@azul3d-bot
Copy link
Author

Comment by mewmew
Tuesday Sep 30, 2014 at 13:37 GMT


I agree, this is definitely a step in the right direction as it simplifies the API and makes integration with independent backends easier. And, I also like that this brings it closer to the behaviour of the image package.

@mewmew
Copy link
Member

mewmew commented Mar 7, 2016

As far as I can tell, this issue has been resolved by v2 of the audio API.

The issue can therefore be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants