go-gst icon indicating copy to clipboard operation
go-gst copied to clipboard

Appsink memory leak

Open clintlombard opened this issue 2 years ago • 3 comments

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())

clintlombard avatar Mar 30 '22 10:03 clintlombard

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.

brucekim avatar Apr 05 '22 13:04 brucekim

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.

clintlombard avatar Apr 05 '22 13:04 clintlombard

@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.

RSWilli avatar Aug 24 '23 20:08 RSWilli