Skip to content
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.

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

Appsink memory leak #33

Open
clintlombard opened this issue Mar 30, 2022 · 3 comments
Open

Appsink memory leak #33

clintlombard opened this issue Mar 30, 2022 · 3 comments

Comments

@clintlombard
Copy link

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
Copy link
Collaborator

brucekim commented Apr 5, 2022

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
Copy link
Author

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
Copy link

RSWilli commented Aug 24, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants