audio
audio copied to clipboard
Basic reflect types for package consumers to use
This feature derived from conversation in https://github.com/go-audio/wav/issues/20
I am using reflective types but performing no reflection, there should be no performance difference or compatibility issues, outside of projects conforming to the interface.
The intention for these changes would be for the Write() encoder function to be able to ask the type so it can utilize AsIntBuffer
only when required - allowing Write() to accept the buf Interface instead of type so it can accept any type but then silently convert to int before passing it on.
This is meant to be an early step to support any type in write without actually writing any type.
Codecov Report
Merging #19 into master will increase coverage by
10.2%
. The diff coverage is45.45%
.
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 29.94% 40.14% +10.2%
==========================================
Files 4 4
Lines 521 543 +22
==========================================
+ Hits 156 218 +62
+ Misses 346 300 -46
- Partials 19 25 +6
Impacted Files | Coverage Δ | |
---|---|---|
float_buffer.go | 81.25% <100%> (+0.98%) |
:arrow_up: |
int_buffer.go | 40% <100%> (+2.26%) |
:arrow_up: |
pcm_buffer.go | 18.85% <25%> (+18.85%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 7b2a6ca...eb30983. Read the comment docs.
I think that's a good approach but it also comes with a lot of challenges. The main one being that any function receiving a buffer might have to do the check and convert the date, so yo might switch back and forth many times within the processing of a file. The other issue is the allocation of lots of temporary buffers (but we can work around that issue by seeing a scratch buffer). I took a similar approach in my initial version: https://github.com/mattetti/audio/blob/master/pcm_buffer.go
The one thing I am not a fan of is to bring an entire package just to get a type lookup table. I think we'd be better off using an enum of constants here.
I have to review what I did, but I'm thinking I might be able to implement or use our own iota map (don't have code in front of me right now). I'll check later and see, I used reflect only because it's stdlib and quick but it may make sense to wrap the types anyways
On Sat, May 4, 2019, 6:46 AM Matt Aimonetti [email protected] wrote:
I think that's a good approach but it also comes with a lot of challenges. The main one being that any function receiving a buffer might have to do the check and convert the date, so yo might switch back and forth many times within the processing of a file. The other issue is the allocation of lots of temporary buffers (but we can work around that issue by seeing a scratch buffer). I took a similar approach in my initial version: https://github.com/mattetti/audio/blob/master/pcm_buffer.go
The one thing I am not a fan of is to bring an entire package just to get a type lookup table. I think we'd be better off using an enum of constants here.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/go-audio/audio/pull/19#issuecomment-489328576, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMLNBZYGYRFCQZYAI5NVTLPTWHUZANCNFSM4HKYHBCQ .
DeepEqual on two slices is an O (n) operation so it may bring a strong impact to the performance.
Deep equal is only used in tests as you can see it was already being used by the maintainers. Otherwise only the reflect kind structure was being used and I'm going to replace it with an enum
On Sat, May 4, 2019, 6:10 PM 方泓睿 [email protected] wrote:
DeepEqual on two slices is an O (n) operation so it may bring a strong impact to the performance.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/go-audio/audio/pull/19#issuecomment-489377830, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMLNB4UB5VILI23UZAYKZ3PTYXZLANCNFSM4HKYHBCQ .
Oh sorry I didn't see that it's a test.