search

asticode / go-astiav

Golang ffmpeg and libav C bindings
MIT License
351 stars 38 forks source link

FrameData question about copyPlaneBytes #42

Closed Cacsjep closed 7 months ago

Cacsjep commented 7 months ago

Hey once again =)

I see on my heap profiles that there is still memory allocated what is releated to FrameData (I use frame.Data().ToImage(img). Before I did the heap snapshot I unref and free the frame so there should not anything that hold that memory, also the image does not exist anymore. Did u have an idea why this happen ?

Thank u

0: 0 [1: 1687552] @ 0x7ff6a7debb28 0x7ff6a7debbee 0x7ff6a7dec169 0x7ff6a7e419ab 0x7ff6a7e3a453 0x7ff6a7e3a218 0x7ff6a7e382db 0x7ff6a798f241
#   0x7ff6a7debb27  github.com/asticode/go-astiav.(*FrameData).copyPlaneBytes+0x47              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:106
#   0x7ff6a7debbed  github.com/asticode/go-astiav.(*FrameData).toImagePix+0x2d              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:112
#   0x7ff6a7dec168  github.com/asticode/go-astiav.(*FrameData).ToImage+0x1e8                C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:165
#   0x7ff6a7e419aa  github.com/Cacsjep/gomilestone/pkg/gui.(*LibavDecoder).Decode+0x38a         C:/Development/milunx/pkg/gui/DecoderLibav.go:170
#   0x7ff6a7e3a452  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).updateUIForVideoPacket+0x212   C:/Development/milunx/pkg/gui/PaneStream.go:35
#   0x7ff6a7e3a217  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).processVideoPacket+0xb7        C:/Development/milunx/pkg/gui/PaneStream.go:18
#   0x7ff6a7e382da  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).Start+0x6fa            C:/Development/milunx/pkg/gui/PaneControl.go:73

0: 0 [74: 124878848] @ 0x7ff6a7dea9be 0x7ff6a7dea9af 0x7ff6a7dea958 0x7ff6a7dec3dd 0x7ff6a7debb04 0x7ff6a7debbee 0x7ff6a7dec169 0x7ff6a7e419ab 0x7ff6a7e3a453 0x7ff6a7e3a218 0x7ff6a7e382db 0x7ff6a798f241
#   0x7ff6a7dea9bd  github.com/asticode/go-astiav._Cfunc_GoBytes+0x3d                   _cgo_gotypes.go:2138
#   0x7ff6a7dea9ae  github.com/asticode/go-astiav.bytesFromC.func1+0x2e                 C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/bytes.go:27
#   0x7ff6a7dea957  github.com/asticode/go-astiav.bytesFromC+0x37                       C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/bytes.go:27
#   0x7ff6a7dec3dc  github.com/asticode/go-astiav.(*frameDataFrame).PlaneBytes+0x3c             C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:207
#   0x7ff6a7debb03  github.com/asticode/go-astiav.(*FrameData).copyPlaneBytes+0x23              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:104
#   0x7ff6a7debbed  github.com/asticode/go-astiav.(*FrameData).toImagePix+0x2d              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:112
#   0x7ff6a7dec168  github.com/asticode/go-astiav.(*FrameData).ToImage+0x1e8                C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:165
#   0x7ff6a7e419aa  github.com/Cacsjep/gomilestone/pkg/gui.(*LibavDecoder).Decode+0x38a         C:/Development/milunx/pkg/gui/DecoderLibav.go:170
#   0x7ff6a7e3a452  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).updateUIForVideoPacket+0x212   C:/Development/milunx/pkg/gui/PaneStream.go:35
#   0x7ff6a7e3a217  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).processVideoPacket+0xb7        C:/Development/milunx/pkg/gui/PaneStream.go:18
#   0x7ff6a7e382da  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).Start+0x6fa            C:/Development/milunx/pkg/gui/PaneControl.go:73
asticode commented 7 months ago

Is the result the same 10s, 30s and 1min later? 🤔 I'm trying to be sure this isn't the garbage collector that takes its time to actually collect this c to go data 🤔

Cacsjep commented 7 months ago

yes still after 1 min, whats interresting that it only happens when i do hardware decoding 🤔

asticode commented 7 months ago

And it still happens after running the garbage collector manually using runtime.GC()?

Cacsjep commented 7 months ago

jep still there after runtime.GC()

0: 0 [11: 104192] @ 0x7ff62e22a9be 0x7ff62e22a9af 0x7ff62e22a958 0x7ff62e22c3dd 0x7ff62e22bb04 0x7ff62e22bbee 0x7ff62e22c169 0x7ff62e28298e 0x7ff62e27b38c 0x7ff62e27b198 0x7ff62e278bdb 0x7ff62ddcf241
#   0x7ff62e22a9bd  github.com/asticode/go-astiav._Cfunc_GoBytes+0x3d                   _cgo_gotypes.go:2138
#   0x7ff62e22a9ae  github.com/asticode/go-astiav.bytesFromC.func1+0x2e                 C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/bytes.go:27
#   0x7ff62e22a957  github.com/asticode/go-astiav.bytesFromC+0x37                       C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/bytes.go:27
#   0x7ff62e22c3dc  github.com/asticode/go-astiav.(*frameDataFrame).PlaneBytes+0x3c             C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:207
#   0x7ff62e22bb03  github.com/asticode/go-astiav.(*FrameData).copyPlaneBytes+0x23              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:104
#   0x7ff62e22bbed  github.com/asticode/go-astiav.(*FrameData).toImagePix+0x2d              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:112
#   0x7ff62e22c168  github.com/asticode/go-astiav.(*FrameData).ToImage+0x1e8                C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:165
#   0x7ff62e28298d  github.com/Cacsjep/gomilestone/pkg/gui.(*LibavDecoder).Decode+0x32d         C:/Development/milunx/pkg/gui/DecoderLibav.go:172
#   0x7ff62e27b38b  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).updateUIForVideoPacket+0x1cb   C:/Development/milunx/pkg/gui/PaneStream.go:35
#   0x7ff62e27b197  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).processVideoPacket+0xb7        C:/Development/milunx/pkg/gui/PaneStream.go:18
#   0x7ff62e278bda  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).Start+0x8ba            C:/Development/milunx/pkg/gui/PaneControl.go:73
asticode commented 7 months ago

Could you fetch the new hw-device-ctx-unref branch and let me know whether it makes a difference? Here's the exact change in case you'd rather modify the code locally: https://github.com/asticode/go-astiav/commit/c8bfcd44ffacabffea9e40e67053f4a0558c44e0

Cacsjep commented 7 months ago

hmm thank u, but still there also after runtime.GC 

0: 0 [23: 235520] @ 0x7ff7ec25aa5e 0x7ff7ec25aa4f 0x7ff7ec25a9f8 0x7ff7ec25c67d 0x7ff7ec25bda4 0x7ff7ec25be8e 0x7ff7ec25c409 0x7ff7ec2b2b5f 0x7ff7ec2ab4ec 0x7ff7ec2ab2f8 0x7ff7ec2a8c3b 0x7ff7ebdff241
#   0x7ff7ec25aa5d  github.com/asticode/go-astiav._Cfunc_GoBytes+0x3d                   _cgo_gotypes.go:2138
#   0x7ff7ec25aa4e  github.com/asticode/go-astiav.bytesFromC.func1+0x2e                 C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/bytes.go:27
#   0x7ff7ec25a9f7  github.com/asticode/go-astiav.bytesFromC+0x37                       C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/bytes.go:27
#   0x7ff7ec25c67c  github.com/asticode/go-astiav.(*frameDataFrame).PlaneBytes+0x3c             C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:207
#   0x7ff7ec25bda3  github.com/asticode/go-astiav.(*FrameData).copyPlaneBytes+0x23              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:104
#   0x7ff7ec25be8d  github.com/asticode/go-astiav.(*FrameData).toImagePix+0x2d              C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:112
#   0x7ff7ec25c408  github.com/asticode/go-astiav.(*FrameData).ToImage+0x1e8                C:/Users/c.acs/go/pkg/mod/github.com/asticode/go-astiav@v0.12.1-0.20240130101059-1910f275f7a1/frame_data.go:165
#   0x7ff7ec2b2b5e  github.com/Cacsjep/gomilestone/pkg/gui.(*LibavDecoder).Decode+0x39e         C:/Development/milunx/pkg/gui/DecoderLibav.go:175
#   0x7ff7ec2ab4eb  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).updateUIForVideoPacket+0x1cb   C:/Development/milunx/pkg/gui/PaneStream.go:35
#   0x7ff7ec2ab2f7  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).processVideoPacket+0xb7        C:/Development/milunx/pkg/gui/PaneStream.go:18
#   0x7ff7ec2a8c3a  github.com/Cacsjep/gomilestone/pkg/gui.(*PaneWidget).Start+0x8ba            C:/Development/milunx/pkg/gui/PaneControl.go:76
asticode commented 7 months ago

And with the remove-copy-plane-bytes branch?

Cacsjep commented 7 months ago

same thing but there must be an issue with the hardware stuff let me deeper look into it and I will come back to this, thank u for your efort.

asticode commented 7 months ago

Fair enough. I've merged in master the fact of removing copying the slice coming from GoBytes because we shouldn't need to do that: since you're using it in your app, could you confirm that it doesn't break anything?

Cacsjep commented 7 months ago

it doesnt break =)

Cacsjep commented 7 months ago

oh this was so missleading, I have thinking the codec context free, frees also the hardware context, even after your updated free method this not free the memory, but when i call directly HardwareDeviceContext.free() it frees up the memory strange but I am happy that is solved.

asticode commented 7 months ago

Happy that it's solved 👍

However I don't quite understand what you mean by when i call directly HardwareDeviceContext.free() it frees up the memory: on master, the HardwareDeviceContext.Free() method doesn't exist anymore, but when you call CodecContext.Free(), it unreferences the hardware device context. What did you do exactly to solve your problem?

Cacsjep commented 7 months ago

I had not updated my local working branch that's the reason why i had this function in my local branch, and if I use it, it works.

func (hdc *HardwareDeviceContext) Free() {
    if hdc.c != nil {
        C.av_buffer_unref(&hdc.c)
    }
}

What I can confirm is that new implementation of codec ctx does not free correctly, cc.c.hw_device_ctx is a valid pointer, I did not exactly know why this not work correctly.

func (cc *CodecContext) Free() {
    if cc.c.hw_device_ctx != nil {
        C.av_buffer_unref(&cc.c.hw_device_ctx)
    }
    C.avcodec_free_context(&cc.c)
}

I see this in docs "The reference is owned by the caller and must be released with av_buffer_unref() when no longer needed. On failure, NULL will be written to this pointer."

image also in example the unref the reference image

I don't exactly understand this, I had thought it's the same reference, but looks like it's not.

asticode commented 7 months ago

I think I may know why it doesn't work on master: what is stored is actually the new ref, not the buffer being reffed. Therefore we're not unrefing the original buffer, but the new ref.

Could you try the better-unref branch and let me know whether it works properly?

Cacsjep commented 7 months ago

oh yes u can merge it works perfectly

asticode commented 7 months ago

Merged 👍