Add audio FIFO #71
Add audio FIFO #71
Conversation
audio_fifo.go
Outdated
| @@ -0,0 +1,46 @@ | |||
| package astiav | |||
|
|
|||
| //#cgo pkg-config: libavutil | |||
There was a problem hiding this comment.
Could you remove this line? I've just merged all the #cgo pkg-config into one astiav.go file 👍
audio_fifo.go
Outdated
| import "unsafe" | ||
|
|
||
| type AudioFifo struct { | ||
| c *C.struct_AVAudioFifo |
There was a problem hiding this comment.
Can you remove the struct_ prefix everywhere in this file?
audio_fifo.go
Outdated
| return newAudioFifoFromC(C.av_audio_fifo_alloc(C.enum_AVSampleFormat(sampleFmt), C.int(channels), C.int(nbSamples))) | ||
| } | ||
|
|
||
| func (a *AudioFifo) AudioFifoRealloc(nbSamples int) int { |
There was a problem hiding this comment.
For all the methods you've added, can you remove the AudioFifo prefix from their name?
audio_fifo.go
Outdated
| return newAudioFifoFromC(C.av_audio_fifo_alloc(C.enum_AVSampleFormat(sampleFmt), C.int(channels), C.int(nbSamples))) | ||
| } | ||
|
|
||
| func (a *AudioFifo) AudioFifoRealloc(nbSamples int) int { |
There was a problem hiding this comment.
Can you return an error here instead of int?
You can use
return newError(C.av_audio_fifo_realloc((*C.struct_AVAudioFifo)(a.c), C.int(nbSamples)))
audio_fifo.go
Outdated
| return int(C.av_audio_fifo_space((*C.struct_AVAudioFifo)(a.c))) | ||
| } | ||
|
|
||
| func (a *AudioFifo) AudioFifoWrite(data **uint8, nbSamples int) int { |
There was a problem hiding this comment.
Can you return (int, error) instead of int?
For that you'll need to do something like:
ret := C.av_audio_fifo_write(...)
if ret < 0 {
return 0, newError(ret)
}
return ret, nil
audio_fifo.go
Outdated
| return int(C.av_audio_fifo_write((*C.struct_AVAudioFifo)(a.c), (*unsafe.Pointer)(unsafe.Pointer(data)), C.int(nbSamples))) | ||
| } | ||
|
|
||
| func (a *AudioFifo) AudioFifoRead(data **uint8, nbSamples int) int { |
There was a problem hiding this comment.
Can you return (int, error) instead of int?
For that you'll need to do something like:
ret := C.av_audio_fifo_read(...)
if ret < 0 {
return 0, newError(ret)
}
return ret, nil
audio_fifo_test.go
Outdated
| ) | ||
|
|
||
| func TestAudioFIFO(t *testing.T) { | ||
| audioFifo := AllocAudioFifo( |
audio_fifo_test.go
Outdated
| ) | ||
|
|
||
| func TestAudioFIFO(t *testing.T) { | ||
| audioFifo := AllocAudioFifo( |
There was a problem hiding this comment.
Can you add defer af.Free() below
audio_fifo_test.go
Outdated
| require.Equal(t, writeSamples, written) | ||
| read := audioFifo.AudioFifoRead(readFrame.DataPtr(), readFrame.NbSamples()) | ||
| require.Equal(t, readSamples, read) | ||
| require.Equal(t, audioFifo.AudioFifoSize(), writeSamples-readSamples) |
There was a problem hiding this comment.
Can you test Realloc() and Space() as well?
|
I applied your all requests. |
frame.go
Outdated
| @@ -217,3 +217,7 @@ func (f *Frame) MoveRef(src *Frame) { | |||
| func (f *Frame) UnsafePointer() unsafe.Pointer { | |||
| return unsafe.Pointer(f.c) | |||
| } | |||
|
|
|||
| func (f *Frame) DataPtr() **uint8 { | |||
audio_fifo.go
Outdated
| } | ||
|
|
||
| func (a *AudioFifo) Write(f *Frame) (int, error) { | ||
| frameRawData := (**uint8)(unsafe.Pointer(&f.c.data[0])) |
There was a problem hiding this comment.
Can you avoid creating this variable and use (*unsafe.Pointer)(unsafe.Pointer(&f.c.data[0])) instead of (*unsafe.Pointer)(unsafe.Pointer(frameRawData))?
audio_fifo.go
Outdated
| } | ||
|
|
||
| func (a *AudioFifo) Read(f *Frame) int { | ||
| frameRawData := (**uint8)(unsafe.Pointer(&f.c.data[0])) |
There was a problem hiding this comment.
Can you avoid creating this variable and use (*unsafe.Pointer)(unsafe.Pointer(&f.c.data[0])) instead of (*unsafe.Pointer)(unsafe.Pointer(frameRawData))?
audio_fifo.go
Outdated
|
|
||
| func (a *AudioFifo) Read(f *Frame) int { | ||
| frameRawData := (**uint8)(unsafe.Pointer(&f.c.data[0])) | ||
| return int(C.av_audio_fifo_read(a.c, (*unsafe.Pointer)(unsafe.Pointer(frameRawData)), C.int(f.NbSamples()))) |
There was a problem hiding this comment.
Can you return (int, error) for this function the same way you did for the Write() method?
audio_fifo_test.go
Outdated
| readFrame.AllocBuffer(0) | ||
|
|
||
| written, err := af.Write(writeFrame) | ||
| require.Equal(t, err, nil) |
There was a problem hiding this comment.
Can you replace this with require.NoError(t, err)?
audio_fifo_test.go
Outdated
| require.Equal(t, af.Size(), writeSamples-readSamples) | ||
| reallocSamples := 3000 | ||
| err = af.Realloc(reallocSamples) | ||
| require.Equal(t, err, nil) |
There was a problem hiding this comment.
Can you replace this with require.NoError(t, err)?
|
Great! I'll merge the PR and do some minor tweaks to avoid bothering you too much 👍 Let me know whether you need a tag |
|
I need new version tag. Advance thank you. |
|
I've created the |
I added an audio FIFO to the astiav library to support audio transcoding, such as AAC to Opus and Opus to AAC.