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. #6

Open
slimsag opened this issue Sep 29, 2014 · 4 comments
Open

Improving data types. #6

slimsag opened this issue Sep 29, 2014 · 4 comments
Assignees
Milestone

Comments

@slimsag
Copy link
Contributor

slimsag commented Sep 29, 2014

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).

@slimsag slimsag self-assigned this Sep 29, 2014
@slimsag slimsag added this to the Version 2 milestone Sep 29, 2014
@slimsag
Copy link
Contributor Author

slimsag commented Sep 29, 2014

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

@slimsag
Copy link
Contributor Author

slimsag commented Sep 30, 2014

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.

@mdlayher
Copy link
Contributor

👍, looks good to me!

@mewmew
Copy link
Contributor

mewmew commented Sep 30, 2014

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.

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

3 participants