tinyzimmer / go-gst

Gstreamer bindings and utilities for golang
GNU Lesser General Public License v2.1
130 stars 37 forks source link

Appsink memory leak #33

Open clintlombard opened 2 years ago

clintlombard commented 2 years ago

There seems to be a memory leak (or unbounded memory growth) when using and appsink element and mapping a new sample's buffer but not copying the data out. This can be reproduced using the appsink example, and changing to the videotestsrc, as follows :

func createPipeline() (*gst.Pipeline, error) {
    gst.Init(nil)

    pipeline, err := gst.NewPipeline("")
    if err != nil {
        return nil, err
    }

    src, err := gst.NewElement("videotestsrc")
    if err != nil {
        return nil, err
    }

    // Should this actually be a *gst.Element that produces an Appsink interface like the
    // rust examples?
    sink, err := app.NewAppSink()
    if err != nil {
        return nil, err
    }

    pipeline.AddMany(src, sink.Element)
    src.Link(sink.Element)

    // Tell the appsink what format we want. It will then be the audiotestsrc's job to
    // provide the format we request.
    // This can be set after linking the two objects, because format negotiation between
    // both elements will happen during pre-rolling of the pipeline.
    sink.SetCaps(gst.NewCapsFromString(
        "video/x-raw",
    ))

    // Getting data out of the appsink is done by setting callbacks on it.
    // The appsink will then call those handlers, as soon as data is available.
    sink.SetCallbacks(&app.SinkCallbacks{
        // Add a "new-sample" callback
        NewSampleFunc: func(sink *app.Sink) gst.FlowReturn {

            // Pull the sample that triggered this callback
            sample := sink.PullSample()
            if sample == nil {
                return gst.FlowEOS
            }

            // Retrieve the buffer from the sample
            buffer := sample.GetBuffer()
            if buffer == nil {
                return gst.FlowError
            }

            // At this point, buffer is only a reference to an existing memory region somewhere.
            // When we want to access its content, we have to map it while requesting the required
            // mode of access (read, read/write).
            //
            // We also know what format to expect because we set it with the caps. So we convert
            // the map directly to signed 16-bit little-endian integers.
            buffer.Map(gst.MapRead)
            defer buffer.Unmap()

            return gst.FlowOK
        },
    })

    return pipeline, nil
}

The main change from this

            samples := buffer.Map(gst.MapRead).AsInt16LESlice()
            defer buffer.Unmap()

to this

            buffer.Map(gst.MapRead)
            defer buffer.Unmap()

By not calling AsInt16LESlice() (or any of the other methods that make a copy of the data, e.g. Bytes()), the memory will grow.

I'm trying to get zero-copy access to the underlying data, as follows:

            mapinfo := buffer.Map(gst.MapRead)
            defer buffer.Unmap()
            data := unsafe.Slice((*byte)(mapinfo.Data()), mapinfo.Size())
brucekim commented 2 years ago

I am interested in this issue too because I also had similar problem.

Do you have any progress for this?

Could you provide runnable sample code including main()?

I want look into it later.

clintlombard commented 2 years ago

The only solution I could come up with using appsink was:

buffer.Map(gst.MapRead).Bytes()

which makes a copy.

However, if you use an identity element and use it's handoff signal then the no copy access works. There must be something special with appsink that breaks this.

RSWilli commented 1 year ago

@clintlombard move this issue to https://github.com/go-gst/go-gst (where future development of the bindings will take place) if you think it is necessary.