3d0c / gmf

Go Media Framework
MIT License
889 stars 170 forks source link

format.go GetNewPacket() memory leak? #67

Open x8522207x opened 7 years ago

x8522207x commented 7 years ago

Is there a memory leak at here "if ret := C.av_read_frame(this.avCtx, &p.avPacket); int(ret) < 0"? Because when I use the function ,my system memory always increase.

func (this FmtCtx) GetNewPackets() chan Packet { yield := make(chan *Packet)

    go func() {
            for {
                    p := NewPacket()
                    if ret := C.av_read_frame(this.avCtx, &p.avPacket); int(ret) < 0 {//here memory leak
                            break
                    }
                    yield <- p
            }

            close(yield)
    }()

    return yield

}

gorebill commented 6 years ago

hi, i found issue like above when i combined apis of custom I/O.

i've written the following iocontext as avio.go comment and use it for reading a webm file(2mb, chrome's demo webm file) : avoictx, _ := NewAVIOContext(inputCtx, &AVIOHandlers{ReadPacket: myReader})

the leak problem occurs while calling GetNewPackets (i just modified example video-to-jpeg.go to cusom I/O) at about 2000th or 4000th loops randomly. for packet := range inputCtx.GetNewPackets() { // commented all and error occurs when calling inputCtx.GetNewPackets() }

3d0c commented 6 years ago

Yes, there is a problem. I will fix it soon. Thank You.

3d0c commented 6 years ago

@gorebill could you also attach your source code?

gorebill commented 6 years ago

@3d0c Thanks for reply, here's the codes i've made changes in 3d0c/gmf/examples/video-to-jpeg.go

func main() {
    srcFileName := "./chrome.webm"

    data, _ := ioutil.ReadFile(srcFileName)
    var length = len(data)

    log.Printf("Media file: %v bytes", length)
    var inputCtx = NewCtx()//pCtx - AVFormatContext
    myReader := func() ([]byte, int) {
        return data, length
    }
    avoictx, _ := NewAVIOContext(inputCtx, &AVIOHandlers{ReadPacket: myReader})//pb, AVIOContext
    defer inputCtx.CloseInputAndRelease()

    var probeData C.AVProbeData
    probeData.buf = (*C.uchar)(unsafe.Pointer(&data[0]))
    probeData.buf_size = (C.int)(length)
    var iformat *C.AVInputFormat
    iformat = C.av_probe_input_format(&probeData, 1)
    var cstr = C.GoString(iformat.name)
    if iformat == nil {
        log.Printf("unable to probe format for it")
        return
    }else{
        // https://www.ffmpeg.org/doxygen/trunk/structAVInputFormat.html
        log.Printf("Found format: %v", cstr)
    }

    inputCtx.SetInputFormat(cstr)
    inputCtx.SetPb(avoictx)
    inputCtx.SetFlag(AVFMT_FLAG_CUSTOM_IO)

    log.Printf("Opening stream....")
    err2 := inputCtx.OpenInput("")
    if err2 != nil {
        panic(err2)
    }
    log.Printf("Open stream finished....")

    os.Mkdir("./tmp", 0755)

    if len(os.Args) > 1 {
        srcFileName = os.Args[1]
    }

    log.Printf("GetBestStream...")
    srcVideoStream, err := inputCtx.GetBestStream(AVMEDIA_TYPE_VIDEO)
    if err != nil {
        panic(err)//fmt.Sprintf("No video stream found")
    }

    log.Printf("FindEncoder...")
    codec, err := FindEncoder(AV_CODEC_ID_JPEG2000)
    if err != nil {
        fatal(err)
    }

    cc := NewCodecCtx(codec)
    defer Release(cc)

    log.Printf("Set up cc params...")
    cc.SetTimeBase(AVR{1,1}) // my ffmpeg3.4.2 requires this

    cc.SetPixFmt(AV_PIX_FMT_RGB24).SetWidth(srcVideoStream.CodecCtx().Width()).SetHeight(srcVideoStream.CodecCtx().Height())
    //cc.SetPixFmt(AV_PIX_FMT_RGB24).SetWidth(480).SetHeight(270)

    if codec.IsExperimental() {
        cc.SetStrictCompliance(FF_COMPLIANCE_EXPERIMENTAL)
    }

    if err := cc.Open(nil); err != nil {
        fatal(err)
    }

    swsCtx := NewSwsCtx(srcVideoStream.CodecCtx(), cc, SWS_BICUBIC)
    defer Release(swsCtx)

    dstFrame := NewFrame().
        SetWidth(srcVideoStream.CodecCtx().Width()).
        SetHeight(srcVideoStream.CodecCtx().Height()).
        SetFormat(AV_PIX_FMT_RGB24)
    defer Release(dstFrame)

    if err := dstFrame.ImgAlloc(); err != nil {
        fatal(err)
    }

    log.Printf("Start to read frames...")
    var count int
    for packet := range inputCtx.GetNewPackets() {
        count++
        //log.Printf("GetNewPackets...%v, %v", count, packet)

        if packet.StreamIndex() != srcVideoStream.Index() {
            // skip non video streams
            Release(packet)
            continue
        }

        ist := assert(inputCtx.GetStream(packet.StreamIndex())).(*Stream)

        log.Printf("packet.Frames...")
        for frame := range packet.Frames(ist.CodecCtx()) {
            log.Printf("tp 1...")

            swsCtx.Scale(frame, dstFrame)

            log.Printf("tp 2...")
            if p, ready, _ := dstFrame.EncodeNewPacket(cc); ready {
                writeFile(p.Data())
                defer Release(p)
            }
        }

        log.Printf("Releasing packet...")
        Release(packet)
    }

    Release(dstFrame)

}

The code above stucks at output like this (randomly, sometimes throws other exception):

2018/03/01 22:44:28 tp 1...
2018/03/01 22:44:28 tp 2...
2018/03/01 22:44:28 177935 bytes written to ./tmp/1027.jpg
2018/03/01 22:44:28 Releasing packet...
2018/03/01 22:44:28 packet.Frames...
2018/03/01 22:44:28 tp 1...
2018/03/01 22:44:28 tp 2...
2018/03/01 22:44:28 182976 bytes written to ./tmp/1028.jpg
2018/03/01 22:44:28 Releasing packet...

The video file used is download from chrome: https://storage.googleapis.com/webfundamentals-assets/videos/chrome.webm

Added following headers to video-to-jpeg.go for test:

/*
    #cgo pkg-config: libavformat libavdevice libavfilter

    #include <stdlib.h>
    #include "libavformat/avformat.h"
    #include <libavdevice/avdevice.h>
    #include "libavutil/opt.h"
 */
import "C"

import (
    "log"
    "os"
    "runtime/debug"
    "strconv"
    "io/ioutil"
    "unsafe"

    . "github.com/3d0c/gmf"
)

Env: go: go version go1.9.2 darwin/amd64 ffmpeg: 3.4.2

PS: It should be ran with setting GODEBUG=cgocheck=0 as:

GODEBUG=cgocheck=0 go run video-to-jpeg.go
3d0c commented 6 years ago

@gorebill The major problem is here:

for packet := range inputCtx.GetNewPackets() {
        count++
        //log.Printf("GetNewPackets...%v, %v", count, packet)

        if packet.StreamIndex() != srcVideoStream.Index() {
            // skip non video streams
            Release(packet)
            continue
        }

        ist := assert(inputCtx.GetStream(packet.StreamIndex())).(*Stream)

        log.Printf("packet.Frames...")
        for frame := range packet.Frames(ist.CodecCtx()) {
            log.Printf("tp 1...")

            swsCtx.Scale(frame, dstFrame)

            log.Printf("tp 2...")
            if p, ready, _ := dstFrame.EncodeNewPacket(cc); ready {
                writeFile(p.Data())
                defer Release(p)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
                                // this defer runs only after exit, so you don't free received packet
                                // remove defer and use  just Release(p)
            }
        }

        log.Printf("Releasing packet...")
        Release(packet)
    }

After this change i don't see any significant memoryleak.

gorebill commented 6 years ago

@3d0c I tried again and removed most of code in GetNewPackets() loop but it still stucks again:

for packet := range inputCtx.GetNewPackets() {
        count++
        log.Printf("GetNewPackets...%v, %v", count, packet)
        Release(packet)
}

Running this loop without EncodeNewPacket() nor Release(x), it stucked at about 3500th loop in my env. So I don't think it is related to Release(p).

gorebill commented 6 years ago

@3d0c Sorry, I have found something wrong in my code of reading input. Thus, i updated them to:

data, _ := ioutil.ReadFile(srcFileName)
    extra := make([]byte, C.AVPROBE_PADDING_SIZE)
    dataWithPaddingZero := append(data, extra...)
    var length = len(data)

    log.Printf("Media file: %v bytes", length)
    var inputCtx = NewCtx()//pCtx - AVFormatContext
    myReader := func() ([]byte, int) {
        return dataWithPaddingZero, len(dataWithPaddingZero)
    }
    avoictx, _ := NewAVIOContext(inputCtx, &AVIOHandlers{ReadPacket: myReader})//pb, AVIOContext
    defer inputCtx.CloseInputAndRelease()

And

    var probeData C.AVProbeData
    probeData.buf = (*C.uchar)(unsafe.Pointer(&dataWithPaddingZero[0]))
    probeData.buf_size = (C.int)(length)

As the ffmpeg document mentioned, AVPROBE_PADDING_SIZE of extra bytes should be filled to buffer.

After this changed, the whole code should be as following:

func main() {
    srcFileName := "./chrome.webm"

    data, _ := ioutil.ReadFile(srcFileName)
    extra := make([]byte, C.AVPROBE_PADDING_SIZE)
    dataWithPaddingZero := append(data, extra...)
    var length = len(data)

    log.Printf("Media file: %v bytes", length)
    var inputCtx = NewCtx()//pCtx - AVFormatContext
    myReader := func() ([]byte, int) {
        return dataWithPaddingZero, len(dataWithPaddingZero)
    }
    avoictx, _ := NewAVIOContext(inputCtx, &AVIOHandlers{ReadPacket: myReader})//pb, AVIOContext
    defer inputCtx.CloseInputAndRelease()

    var probeData C.AVProbeData
    probeData.buf = (*C.uchar)(unsafe.Pointer(&dataWithPaddingZero[0]))
    probeData.buf_size = (C.int)(length)

    var iformat *C.AVInputFormat
    iformat = C.av_probe_input_format(&probeData, 1)
    var cstr = C.GoString(iformat.name)
    if iformat == nil {
        log.Printf("unable to probe format for it")
        return
    }else{
        // https://www.ffmpeg.org/doxygen/trunk/structAVInputFormat.html
        log.Printf("Found format: %v", cstr)
    }

    inputCtx.SetInputFormat(cstr)
    inputCtx.SetPb(avoictx)
    inputCtx.SetFlag(AVFMT_FLAG_CUSTOM_IO)

    log.Printf("Opening stream....")
    err2 := inputCtx.OpenInput("")
    if err2 != nil {
        panic(err2)
    }
    log.Printf("Open stream finished....")

    os.Mkdir("./tmp", 0755)

    if len(os.Args) > 1 {
        srcFileName = os.Args[1]
    }

    log.Printf("GetBestStream...")
    srcVideoStream, err := inputCtx.GetBestStream(AVMEDIA_TYPE_VIDEO)
    if err != nil {
        panic(err)//fmt.Sprintf("No video stream found")
    }

    log.Printf("FindEncoder...")
    codec, err := FindEncoder(AV_CODEC_ID_JPEG2000)
    if err != nil {
        fatal(err)
    }

    cc := NewCodecCtx(codec)
    defer Release(cc)

    log.Printf("Set up cc params...")
    cc.SetTimeBase(AVR{1,1}) // my ffmpeg3.4.2 requires this

    cc.SetPixFmt(AV_PIX_FMT_RGB24).SetWidth(srcVideoStream.CodecCtx().Width()).SetHeight(srcVideoStream.CodecCtx().Height())
    //cc.SetPixFmt(AV_PIX_FMT_RGB24).SetWidth(480).SetHeight(270)

    if codec.IsExperimental() {
        cc.SetStrictCompliance(FF_COMPLIANCE_EXPERIMENTAL)
    }

    if err := cc.Open(nil); err != nil {
        fatal(err)
    }

    swsCtx := NewSwsCtx(srcVideoStream.CodecCtx(), cc, SWS_BICUBIC)
    defer Release(swsCtx)

    dstFrame := NewFrame().
        SetWidth(srcVideoStream.CodecCtx().Width()).
        SetHeight(srcVideoStream.CodecCtx().Height()).
        SetFormat(AV_PIX_FMT_RGB24)
    defer Release(dstFrame)

    if err := dstFrame.ImgAlloc(); err != nil {
        fatal(err)
    }

    log.Printf("Start to read frames...")
    var count int
    for packet := range inputCtx.GetNewPackets() {
        count++
        log.Printf("GetNewPackets...%v, %v", count, packet)
        Release(packet)
    }

    Release(dstFrame)
}

From now on, the code would be significant panic like these:

2018/03/06 00:33:28 GetNewPackets...6811, &{{0x5c00220 640 640 0x5c027e0 356 1 1 [0 0 0 0] <nil> 0 [0 0 0 0] 23 4015667 0} map[] {0}}
[matroska,webm @ 0x7000000] Invalid EBML number size tag 0x02 at pos 242993013 (0xe7bc775)
video-to-jpeg(10768,0x7000041bb000) malloc: *** error for object 0xba3540dd246247bb: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
SIGABRT: abort
PC=0x7fffd2fc9d42 m=8 sigcode=0
signal arrived during cgo execution

This error would happen randomly when calling (*FmtCtx).GetNewPackets.

gorebill commented 6 years ago

Continue digging the panic, I changed GetNewPackets to GetNextPacket:

    for packet := inputCtx.GetNextPacket(); packet != nil; packet = inputCtx.GetNextPacket() {
        count++
        log.Printf("GetNextPacket...%v, %v", count, packet)

        Release(packet)

        log.Printf("GetNextPacket end...%v", count)
    }

The panic appears like this:

2018/03/06 00:49:03 GetNextPacket...3537, &{{0x6a01300 7403 7403 0x6a011b0 298 1 1 [0 0 0 0] <nil> 0 [0 0 0 0] 23 2351493 0} map[] {0}}
2018/03/06 00:49:03 GetNextPacket end...3537
video-to-jpeg(10957,0x7fffdbdb53c0) malloc: *** error for object 0x7031400: incorrect checksum for freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug
SIGABRT: abort
PC=0x7fffd2fc9d42 m=0 sigcode=0
signal arrived during cgo execution

Look into the code, error is produced from here(3d0c/gmf/format.go: GetNextPacket):

        if ret := C.av_read_frame(this.avCtx, &p.avPacket); int(ret) < 0 {
            Release(p)
            return nil
        }
3d0c commented 6 years ago

@gorebill Please find working example of your app here

gorebill commented 6 years ago

@3d0c Many thanks to your effort, it works now!!

By the way, maybe demo 'video-to-jpeg-avio.go' wishes a

os.MkdirAll("./tmp-img", os.ModePerm)

to prevent dir not exist :D

Again, thank you so much.

3d0c commented 6 years ago

@gorebill Yes, you're right, i will add these changes in upcoming big commit.

I leave this issue open, because there is some memory leaks. So if somebody faces it write here.