Skip to content

perfect: 通过 cgo.Handle 传递回调函数，减少读写锁开销 #17

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.

Sign up for GitHub

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

Closed
AkimioJR wants to merge 8 commits into from
Closed

perfect: 通过 cgo.Handle 传递回调函数，减少读写锁开销 #17

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b3ec17d
fix: 增加读写锁，避免多线程中同时操作callbacks和lastId
AkimioJR Feb 6, 2026
c768dec
perfect: 使用defer操作清理函数
AkimioJR Feb 6, 2026
246dd83
fix: 修复 C.CString(option.HWDevice) 内存泄漏问题
AkimioJR Feb 6, 2026
e3c81ac
refactor: 仅对callbacks加锁，防止影响性能
DawningW Feb 7, 2026
12db156
fix: 修改Player方法为指针接收者
DawningW Feb 7, 2026
82cb8c3
Merge branch 'DawningW:v1' into v1
AkimioJR Feb 7, 2026
b24bac4
perfect: 通过 cgo.Handle 在c和go之间传递数据以实现回调的传递
AkimioJR Feb 7, 2026
07835cf
refactor: 使用 swich 优化 goCallback 函数的事件处理逻辑
AkimioJR Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 19 additions & 28 deletions binding/go/gcrp/gcrp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ static int frame_get_height(struct Frame* frame) { return frame->height; }
*/
import "C"
import (
"sync"
"runtime/cgo"
"unsafe"
)

Expand Down Expand Up @@ -87,25 +87,17 @@ type Callback interface {
}

type Player struct {
handle C.crp_handle
callbackId int
handle C.crp_handle
cbHandle cgo.Handle
}

var mutex sync.RWMutex
var callbacks = make(map[int]Callback)
var lastId = 0

//export goCallback
func goCallback(event C.enum_Event, data unsafe.Pointer, userData unsafe.Pointer) {
id := int(uintptr(userData))
mutex.RLock()
callback, ok := callbacks[id]
mutex.RUnlock()
if !ok {
return
}

if event == C.CRP_EV_NEW_FRAME {
callback := cgo.Handle(uintptr(userData)).Value().(Callback)

switch event {
Comment on lines +97 to +99
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cgo.Handle(uintptr(userData)).Value().(Callback) will panic if userData is nil/0, if the handle has already been deleted, or if the stored value is a nil interface / wrong type. Panics crossing the cgo boundary will crash the process. Add guards (e.g., return early on userData == nil, use a safe type assertion, and consider recovering from runtime/cgo: misuse of an invalid Handle).

Copilot uses AI. Check for mistakes.
case C.CRP_EV_NEW_FRAME:
frame := (*C.struct_Frame)(data)
height := C.frame_get_height(frame)
callback.OnFrame(false, Frame{
Expand All @@ -122,7 +114,7 @@ func goCallback(event C.enum_Event, data unsafe.Pointer, userData unsafe.Pointer
[4]int{int(frame.stride[0]), int(frame.stride[1]), int(frame.stride[2]), int(frame.stride[3])},
uint64(frame.pts),
})
} else if event == C.CRP_EV_NEW_AUDIO {
case C.CRP_EV_NEW_AUDIO:
frame := (*C.struct_Frame)(data)
callback.OnFrame(true, Frame{
0, 0,
Expand All @@ -138,10 +130,10 @@ func goCallback(event C.enum_Event, data unsafe.Pointer, userData unsafe.Pointer
[4]int{int(frame.stride[0]), int(frame.stride[1]), int(frame.stride[2]), int(frame.stride[3])},
uint64(frame.pts),
})
} else if event == C.CRP_EV_VIDEO_EXTRADATA || event == C.CRP_EV_AUDIO_EXTRADATA {
case C.CRP_EV_VIDEO_EXTRADATA, C.CRP_EV_AUDIO_EXTRADATA:
ed := (*C.struct_ExtraData)(data)
callback.OnEvent(Event(event), C.GoBytes(unsafe.Pointer(ed.data), C.int(ed.size)))
} else {
default:
callback.OnEvent(Event(event), int64(uintptr(data)))
}
}
Expand All @@ -154,9 +146,10 @@ func (player *Player) Destroy() {
C.crp_destroy(player.handle)
player.handle = nil

mutex.Lock()
delete(callbacks, player.callbackId)
mutex.Unlock()
if player.cbHandle != 0 {
player.cbHandle.Delete()
player.cbHandle = 0
}
}

func (player *Player) Auth(username, password string, isMd5 bool) {
Expand Down Expand Up @@ -191,14 +184,12 @@ func (player *Player) Play(url string, option Option, callback Callback) {
timeout: C.int64_t(option.Timeout),
}

mutex.Lock()
id := lastId
player.callbackId = id
callbacks[id] = callback
lastId += 1
mutex.Unlock()
if player.cbHandle != 0 {
player.cbHandle.Delete()
}
player.cbHandle = cgo.NewHandle(callback)

C.crp_play(player.handle, curl, &coption, C.crp_callback(C.goCallback), unsafe.Pointer(uintptr(id)))
C.crp_play(player.handle, curl, &coption, C.crp_callback(C.goCallback), unsafe.Pointer(player.cbHandle))
Comment on lines +187 to +192
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting and replacing cbHandle inside Play can leave the native player still invoking callbacks with the old userData (e.g., if a previous stream is still running, or if StreamPuller::start refuses to start when already playing and keeps the old callback/userData). In that case goCallback will receive a deleted handle and panic. Ensure the native side is fully stopped before deleting the existing handle (e.g., call Stop() first and only delete after stop completes), or avoid deleting until you know no more callbacks can occur.

Copilot uses AI. Check for mistakes.
Comment on lines +187 to +192
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is existing Go test coverage for Play, but the new cgo.Handle lifecycle behavior (replacing/deleting handles, repeated Play calls, Play after Stop, etc.) isn’t covered. Adding a test that calls Play twice (or Play → Stop → Play) and asserts no panic/crash would help prevent regressions in handle lifetime management.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsafe.Pointer(player.cbHandle) is not a safe/valid way to pass a cgo.Handle through C void*. cgo.Handle is a defined uintptr type; this conversion will not compile and/or will be incorrect on some platforms. Convert via uintptr explicitly (and keep the contract that C treats it as an opaque value and only passes it back).

Suggested change
C.crp_play(player.handle, curl, &coption, C.crp_callback(C.goCallback), unsafe.Pointer(player.cbHandle))
C.crp_play(player.handle, curl, &coption, C.crp_callback(C.goCallback), unsafe.Pointer(uintptr(player.cbHandle)))

Copilot uses AI. Check for mistakes.
}

func (player *Player) Replay() {
Expand Down
Loading