-
Notifications
You must be signed in to change notification settings - Fork 53
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
A not so small bunch of not so small fixes and improvements #30
Conversation
…atible changes but makes a lot of stuff better / more reliable / more convenient. ContextConfig and DeviceConfig are now Go structs and are not compatible with CGO at all. This means we can use Go types and replace *byte with string and uint32 / uint8 with bool, and it also means that CGO will do all the nasty work of converting between C and Go for us and if Miniaudio gets updated and a struct field or something changes, there will be a nice, compile time error instead of random undefined behavior when the wrong field is accessed. We do need to convert between the C and Go versions of structs, but better boring self-updating code than code that could break at any moment. The AllocatedContext type is now Context, because the only place you could really allocate a context is in the C heap. InitContext and InitDevice no longer take an extra struct argument containing callbacks, the callbacks can be set in the context / device config instead. Updated the examples. Added the pointerList type, which is just a slice of unsafe.Pointer with methods to allocate strings and bytes and free all the pointers. We use this for converting Go strings into C strings and easily freeing everything when the context / device is freed. It has an optimization that converts empty Go strings into null pointers (which is what Miniaudio expects anyway) and it doesn't add null pointers to the list. In short, C strings are only allocated if the go string is set to something other than an empty value. Removed our logic that converts a Miniaudio error code to a string and use ma_result_description instead. All miniaudio constants are now resolved by CGO. At least one of them was already invalid because their values were defined manually, then Miniaudio was updated and the update added a new error code somewhere higher up the enum. Miniaudio now has an Error type. Errors returned by Miniaudio functions are now comparable with the Miniaudio error constants, except that MA_SUCCESS will be returned as a nil error. Even if we update Miniaudio and forget to add new error codes, users just won't have any constants to compare the error value to. Everything else will work, though. All Miniaudio error code constants now have the Err prefix. DeviceConfig now has the Opensl, CoreAudio and Aaudio fields which were apparently never added. Added many many new error codes that weren't already defined. FormatType is now Format. It doesn't make sense for some types to have a Type suffix but for other to not. Optimized goDataCallback so that the buffer size is calculated in C and we avoid a C> go > C call. I don't think it helped (the source of my high-ish CPU usage must be somewhere else) but can't hurt. When implementing methods on any type that's a uintptr such as Device, make sure the receiver isn't *self. This will cause problems if you try to convert self to an unsafe.Pointer, because of course it's not pointing where you think it's pointing. Added Device.SetDataCallback and Device.SetStopCallback, which let us set the data and stop callbacks of a device, even while it's running. This works because we have a mutex and aren't changing the actual C callback, just the user-provided function on the Go side. Implemented (Format).String, which returns a description of the format using ma_get_format_name. Replaced Context.Devices with Context.PlaybackDevices, Context.CaptureDevices and context.AllDevices. More efficient and convenient for everyone! Added (*DeviceInfo).Formats which returns a slice of the devices supported formats DeviceConfig.PeriodSizeInMilliseconds is now PeriodSizeInDuration, which lets you set it using a time.Duration Simplified the capture example. Are you trying to scare away contributors or something? Added the FramesToDuration, DurationToFrames, BytesToFrames, FramesToBytes and BytesToDuration functions which let you convert between stuff. Any uintptr types like Device and Context now check if the pointer is nil when calling cptr. This can cause a segfault otherwise. Go still prints a helpful stack trace somehow, but let's just avoide it entirely. Added Backend.String method to the Backend type, and added the Backend method to the Context type which returns the current backend. Modified the enumerate example to print the currently used backend, and weather loopback devices are supported. Added the DataBuffer type which lets you easily convert the input and output pointers you get in a ma_device_callback_proc into either raw bytes or slices of any of the Miniaudio formats except for FormatF24. DataCallback now uses these types.
Can you please send PRs one by one, with breaking changes left for the end? It is very hard to understand when everything is in the bunch, and not sure about the breaking changes. There are nice changes, from the sound of it, thanks! |
Yeah. Now that I think of it, I should've at least separated
everything into at least kilobyte sized chunks using multiple commits.
This is probably going to result in like five or more pull requests
though.
…On 9/8/21, Milan Nikolic ***@***.***> wrote:
Can you please send PRs one by one, with breaking changes left for the end?
It is very hard to understand when everything is in the bunch, and not sure
about the breaking changes. There are nice changes, from the sound of it,
thanks!
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#30 (comment)
|
Okay, turns out it's kinda hard to split it. Most of it (at least 80)% are breaking changes and some of them require other changes to be made first. I'm starting to think one reason I made only one huge pull request was so we could do just one compatibility break and make a lot of stuff better in the process. That said, I'll try splitting it up, but I can't really put breaking changes last because that's most of them and whatever is left either kind of hard to separate and merge back into the old version of Miniaudio, or is small enough that we might as well just do it later. |
ContextConfig and DeviceConfig are now Go structs and are not compatible with CGO at all. This means we can use Go types and replace *byte with string and uint32 / uint8 with bool, and it also means that CGO will do all the nasty work of converting between C and Go for us and if Miniaudio gets updated and a struct field or something changes, there will be a nice, compile time error instead of random undefined behavior when the wrong field is accessed. We do need to convert between the C and Go versions of structs, but better boring self-updating code than code that could break at any moment.
The AllocatedContext type is now Context, because the only place you could really allocate a context is in the C heap.
InitContext and InitDevice no longer take an extra struct argument containing callbacks, the callbacks can be set in the context / device config instead.
Updated the examples.
Added the pointerList type, which is just a slice of unsafe.Pointer with methods to allocate strings and bytes and free all the pointers. We use this for converting Go strings into C strings and easily freeing everything when the context / device is freed. It has an optimization that converts empty Go strings into null pointers (which is what Miniaudio expects anyway) and it doesn't add null pointers to the list. In short, C strings are only allocated if the go string is set to something other than an empty value.
Removed our logic that converts a Miniaudio error code to a string and use ma_result_description instead.
All miniaudio constants are now resolved by CGO. At least one of them was already invalid because their values were defined manually, then Miniaudio was updated and the update added a new error code somewhere higher up the enum.
Miniaudio now has an Error type. Errors returned by Miniaudio functions are now comparable with the Miniaudio error constants, except that MA_SUCCESS will be returned as a nil error. Even if we update Miniaudio and forget to add new error codes, users just won't have any constants to compare the error value to. Everything else will work, though.
All Miniaudio error code constants now have the Err prefix.
DeviceConfig now has the Opensl, CoreAudio and Aaudio fields which were apparently never added.
Added many many new error codes that weren't already defined.
FormatType is now Format. It doesn't make sense for some types to have a Type suffix but for other to not.
Optimized goDataCallback so that the buffer size is calculated in C and we avoid a C> go > C call. I don't think it helped (the source of my high-ish CPU usage must be somewhere else) but can't hurt.
When implementing methods on any type that's a uintptr such as Device, make sure the receiver isn't *self. This will cause problems if you try to convert self to an unsafe.Pointer, because of course it's not pointing where you think it's pointing.
Added Device.SetDataCallback and Device.SetStopCallback, which let us set the data and stop callbacks of a device, even while it's running. This works because we have a mutex and aren't changing the actual C callback, just the user-provided function on the Go side.
Implemented (Format).String, which returns a description of the format using ma_get_format_name.
Replaced Context.Devices with Context.PlaybackDevices, Context.CaptureDevices and context.AllDevices. More efficient and convenient for everyone!
Added (*DeviceInfo).Formats which returns a slice of the devices supported formats
DeviceConfig.PeriodSizeInMilliseconds is now PeriodSizeInDuration, which lets you set it using a time.Duration
Simplified the capture example. Are you trying to scare away contributors or something?
Added the FramesToDuration, DurationToFrames, BytesToFrames, FramesToBytes and BytesToDuration functions which let you convert between stuff.
Any uintptr types like Device and Context now check if the pointer is nil when calling cptr. This can cause a segfault otherwise. Go still prints a helpful stack trace somehow, but let's just avoide it entirely.
Added Backend.String method to the Backend type, and added the Backend method to the Context type which returns the current backend.
Modified the enumerate example to print the currently used backend, and weather loopback devices are supported.
Added the DataBuffer type which lets you easily convert the input and output pointers you get in a ma_device_callback_proc into either raw bytes or slices of any of the Miniaudio formats except for FormatF24. DataCallback now uses these types.
Fixes #28 even though it's noted in the commit.