Buffer and Memory Optimizations (#6)

* Fixes for memory leaks when creating or mapping the contents of a buffer
* Unmap methods on Buffer and Memory objects for required use after utilizing their Map methods
* Optimize MapInfo.WriteData to use memcpy
* Re-implement BufferFromBytes using GBytes and `gst_buffer_new_wrapped_bytes` instead of `gst_buffer_new_wrapped`.

examples/appsink/main.go

@@ -70,6 +70,7 @@ func createPipeline() (*gst.Pipeline, error) {
 			// We also know what format to expect because we set it with the caps. So we convert
 			// the map directly to signed 16-bit integers.
 			samples := buffer.Map(gst.MapRead).AsInt16Slice()
+			defer buffer.Unmap()
 
 			// Calculate the root mean square for the buffer
 			// (https://en.wikipedia.org/wiki/Root_mean_square)

examples/appsrc/main.go

@@ -71,11 +71,11 @@ func createPipeline() (*gst.Pipeline, error) {
 			fmt.Println("Producing frame:", i)
 
 			// Create a buffer that can hold exactly one video RGBA frame.
-			buf := gst.NewBufferWithSize(videoInfo.Size())
+			buffer := gst.NewBufferWithSize(videoInfo.Size())
 
 			// For each frame we produce, we set the timestamp when it should be displayed
 			// The autovideosink will use this information to display the frame at the right time.
-			buf.SetPresentationTimestamp(time.Duration(i) * 500 * time.Millisecond)
+			buffer.SetPresentationTimestamp(time.Duration(i) * 500 * time.Millisecond)
 
 			// Produce an image frame for this iteration.
 			pixels := produceImageFrame(palette[i])
@@ -87,10 +87,11 @@ func createPipeline() (*gst.Pipeline, error) {
 			//
 			// There are convenience wrappers for building buffers directly from byte sequences as
 			// well.
-			buf.Map(gst.MapWrite).WriteData(pixels)
+			buffer.Map(gst.MapWrite).WriteData(pixels)
+			defer buffer.Unmap()
 
 			// Push the buffer onto the pipeline.
-			self.PushBuffer(buf)
+			self.PushBuffer(buffer)
 
 			i++
 		},

examples/pad-probes/main.go

@@ -60,6 +60,7 @@ func padProbes(mainLoop *gst.MainLoop) error {
 		// So mapping the buffer makes the underlying memory region accessible to us.
 		// See: https://gstreamer.freedesktop.org/documentation/plugin-development/advanced/allocation.html
 		mapInfo := buffer.Map(gst.MapRead)
+		defer buffer.Unmap()
 
 		// We know what format the data in the memory region has, since we requested
 		// it by setting the fakesink's caps. So what we do here is interpret the

gst/gst_buffer.go

@@ -36,6 +36,7 @@ func GetMaxBufferMemory() uint64 { return uint64(C.gst_buffer_get_max_memory())
 // Buffer is a go representation of a GstBuffer.
 type Buffer struct {
 	ptr     *C.GstBuffer
+	mapInfo *MapInfo
 }
 
 // NewEmptyBuffer returns a new empty buffer.
@@ -74,11 +75,9 @@ func NewBufferAllocate(alloc *Allocator, params *AllocationParams, size int64) *
 
 // NewBufferFromBytes returns a new buffer from the given byte slice.
 func NewBufferFromBytes(b []byte) *Buffer {
-	str := string(b)
-	p := unsafe.Pointer(C.CString(str))
-	// memory is freed by gstreamer after building the new buffer
-	buf := C.gst_buffer_new_wrapped((C.gpointer)(p), C.gsize(len(str)))
-	return wrapBuffer(buf)
+	gbytes := C.g_bytes_new((C.gconstpointer)(unsafe.Pointer(&b[0])), C.gsize(len(b)))
+	defer C.g_bytes_unref(gbytes) // gstreamer takes another ref so we don't need ours after returning
+	return wrapBuffer(C.gst_buffer_new_wrapped_bytes(gbytes))
 }
 
 // NewBufferFromReader returns a new buffer from the given io.Reader.
@@ -144,6 +143,10 @@ func (b *Buffer) Reader() io.Reader { return bytes.NewBuffer(b.Bytes()) }
 // Bytes returns a byte slice of the data inside this buffer.
 func (b *Buffer) Bytes() []byte {
 	mapInfo := b.Map(MapRead)
+	if mapInfo == nil {
+		return nil
+	}
+	defer b.Unmap()
 	return mapInfo.Bytes()
 }
 
@@ -612,20 +615,29 @@ func (b *Buffer) IterateMetaFiltered(meta *Meta, apiType glib.Type) *Meta {
 	return wrapMeta(C.gst_buffer_iterate_meta_filtered(b.Instance(), (*C.gpointer)(&ptr), C.GType(apiType)))
 }
 
-// Map will map the data inside this buffer.
+// Map will map the data inside this buffer. This function can return nil if the memory is not read or writable.
+//
+// Unmap the Buffer after usage.
 func (b *Buffer) Map(flags MapFlags) *MapInfo {
-	var mapInfo C.GstMapInfo
+	mapInfo := C.malloc(C.sizeof_GstMapInfo)
 	C.gst_buffer_map(
 		(*C.GstBuffer)(b.Instance()),
-		(*C.GstMapInfo)(unsafe.Pointer(&mapInfo)),
+		(*C.GstMapInfo)(mapInfo),
 		C.GstMapFlags(flags),
 	)
-	return wrapMapInfo(&mapInfo, func() {
-		// This may not be necesesary
-		// if gobool(C.isBuffer(b.Instance())) {
-		// 	C.gst_buffer_unmap(b.Instance(), (*C.GstMapInfo)(unsafe.Pointer(&mapInfo)))
-		// }
-	})
+	if mapInfo == C.NULL {
+		return nil
+	}
+	b.mapInfo = wrapMapInfo((*C.GstMapInfo)(mapInfo))
+	return b.mapInfo
+}
+
+// Unmap will unmap the data inside this memory. Use this after calling Map on the buffer.
+func (b *Buffer) Unmap() {
+	if b.mapInfo == nil {
+		return
+	}
+	C.gst_buffer_unmap(b.Instance(), (*C.GstMapInfo)(b.mapInfo.Instance()))
 }
 
 // MapRange maps the info of length merged memory blocks starting at idx in buffer.
@@ -636,18 +648,20 @@ func (b *Buffer) Map(flags MapFlags) *MapInfo {
 // When the buffer is writable but the memory isn't, a writable copy will automatically be
 // created and returned. The readonly copy of the buffer memory will then also be replaced with this writable copy.
 //
-// Unmap after usage.
+// Unmap the Buffer after usage.
 func (b *Buffer) MapRange(idx uint, length int, flags MapFlags) *MapInfo {
-	var mapInfo C.GstMapInfo
+	mapInfo := C.malloc(C.sizeof_GstMapInfo)
 	C.gst_buffer_map_range(
 		(*C.GstBuffer)(b.Instance()),
 		C.guint(idx), C.gint(length),
-		(*C.GstMapInfo)(unsafe.Pointer(&mapInfo)),
+		(*C.GstMapInfo)(mapInfo),
 		C.GstMapFlags(flags),
 	)
-	return wrapMapInfo(&mapInfo, func() {
-		C.gst_buffer_unmap(b.Instance(), (*C.GstMapInfo)(unsafe.Pointer(&mapInfo)))
-	})
+	if mapInfo == C.NULL {
+		return nil
+	}
+	b.mapInfo = wrapMapInfo((*C.GstMapInfo)(mapInfo))
+	return b.mapInfo
 }
 
 // Memset fills buf with size bytes with val starting from offset. It returns the

gst/gst_mapinfo.go

@@ -0,0 +1,135 @@
+package gst
+
+/*
+#include "gst.go.h"
+*/
+import "C"
+
+import (
+	"encoding/binary"
+	"unsafe"
+)
+
+// MapInfo is a go representation of a GstMapInfo.
+type MapInfo struct {
+	ptr *C.GstMapInfo
+}
+
+// Instance returns the underlying GstMapInfo instance.
+func (m *MapInfo) Instance() *C.GstMapInfo {
+	return m.ptr
+}
+
+// WriteData writes the given sequence directly to the map's memory.
+func (m *MapInfo) WriteData(data []byte) {
+	C.memcpy(unsafe.Pointer(m.ptr.data), unsafe.Pointer(&data[0]), C.gsize(len(data)))
+}
+
+// Memory returns the underlying memory object.
+func (m *MapInfo) Memory() *Memory {
+	return wrapMemory(m.ptr.memory)
+}
+
+// Data returns a pointer to the raw data inside this map.
+func (m *MapInfo) Data() unsafe.Pointer {
+	return unsafe.Pointer(m.ptr.data)
+}
+
+// Flags returns the flags set on this map.
+func (m *MapInfo) Flags() MapFlags {
+	return MapFlags(m.ptr.flags)
+}
+
+// Size returrns the size of this map.
+func (m *MapInfo) Size() int64 {
+	return int64(m.ptr.size)
+}
+
+// MaxSize returns the maximum size of this map.
+func (m *MapInfo) MaxSize() int64 {
+	return int64(m.ptr.maxsize)
+}
+
+// Bytes returns a byte slice of the data inside this map info.
+func (m *MapInfo) Bytes() []byte {
+	return C.GoBytes(m.Data(), (C.int)(m.Size()))
+}
+
+// AsInt8Slice returns the contents of this map as a slice of signed 8-bit integers.
+func (m *MapInfo) AsInt8Slice() []int8 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]int8, m.Size())
+	for i := range out {
+		out[i] = int8(uint8sl[i])
+	}
+	return out
+}
+
+// AsInt16Slice returns the contents of this map as a slice of signed 16-bit integers.
+func (m *MapInfo) AsInt16Slice() []int16 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]int16, m.Size()/2)
+	for i := range out {
+		out[i] = int16(binary.LittleEndian.Uint16(uint8sl[i*2 : (i+1)*2]))
+	}
+	return out
+}
+
+// AsInt32Slice returns the contents of this map as a slice of signed 32-bit integers.
+func (m *MapInfo) AsInt32Slice() []int32 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]int32, m.Size()/4)
+	for i := range out {
+		out[i] = int32(binary.LittleEndian.Uint32(uint8sl[i*4 : (i+1)*4]))
+	}
+	return out
+}
+
+// AsInt64Slice returns the contents of this map as a slice of signed 64-bit integers.
+func (m *MapInfo) AsInt64Slice() []int64 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]int64, m.Size()/8)
+	for i := range out {
+		out[i] = int64(binary.LittleEndian.Uint64(uint8sl[i*8 : (i+1)*8]))
+	}
+	return out
+}
+
+// AsUint8Slice returns the contents of this map as a slice of unsigned 8-bit integers.
+func (m *MapInfo) AsUint8Slice() []uint8 {
+	out := make([]uint8, m.Size())
+	for i, t := range (*[1 << 30]uint8)(m.Data())[:m.Size():m.Size()] {
+		out[i] = t
+	}
+	return out
+}
+
+// AsUint16Slice returns the contents of this map as a slice of unsigned 16-bit integers.
+func (m *MapInfo) AsUint16Slice() []uint16 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]uint16, m.Size()/2)
+	for i := range out {
+		out[i] = uint16(binary.LittleEndian.Uint16(uint8sl[i*2 : (i+1)*2]))
+	}
+	return out
+}
+
+// AsUint32Slice returns the contents of this map as a slice of unsigned 32-bit integers.
+func (m *MapInfo) AsUint32Slice() []uint32 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]uint32, m.Size()/4)
+	for i := range out {
+		out[i] = uint32(binary.LittleEndian.Uint32(uint8sl[i*4 : (i+1)*4]))
+	}
+	return out
+}
+
+// AsUint64Slice returns the contents of this map as a slice of unsigned 64-bit integers.
+func (m *MapInfo) AsUint64Slice() []uint64 {
+	uint8sl := m.AsUint8Slice()
+	out := make([]uint64, m.Size()/8)
+	for i := range out {
+		out[i] = uint64(binary.LittleEndian.Uint64(uint8sl[i*8 : (i+1)*8]))
+	}
+	return out
+}

gst/gst_memory.go

@@ -2,14 +2,10 @@ package gst
 
 /*
 #include "gst.go.h"
-
-void writeMapData (GstMapInfo * mapInfo, gint idx, guint8 data) { mapInfo->data[idx] = data; }
 */
 import "C"
 
 import (
-	"encoding/binary"
-	"runtime"
 	"unsafe"
 
 	"github.com/gotk3/gotk3/glib"
@@ -22,6 +18,7 @@ import (
 // Use the Buffer and its Map methods to interact with memory in both a read and writable way.
 type Memory struct {
 	ptr     *C.GstMemory
+	mapInfo *MapInfo
 }
 
 // NewMemoryWrapped allocates a new memory block that wraps the given data.
@@ -83,151 +80,37 @@ func (m *Memory) Copy(offset, size int64) *Memory {
 	return wrapMemory(mem)
 }
 
-// Map the data inside the memory. This function can return nil if the memory is not readable.
-func (m *Memory) Map() *MapInfo {
-	var mapInfo C.GstMapInfo
+// Map the data inside the memory. This function can return nil if the memory is not read or writable.
+//
+// Unmap the Memory after usage.
+func (m *Memory) Map(flags MapFlags) *MapInfo {
+	mapInfo := C.malloc(C.sizeof_GstMapInfo)
 	C.gst_memory_map(
 		(*C.GstMemory)(m.Instance()),
-		(*C.GstMapInfo)(unsafe.Pointer(&mapInfo)),
-		C.GST_MAP_READ,
+		(*C.GstMapInfo)(mapInfo),
+		C.GstMapFlags(flags),
 	)
-	return wrapMapInfo(&mapInfo, func() {
-		C.gst_memory_unmap(m.Instance(), (*C.GstMapInfo)(unsafe.Pointer(&mapInfo)))
-	})
+	if mapInfo == C.NULL {
+		return nil
+	}
+	m.mapInfo = wrapMapInfo((*C.GstMapInfo)(mapInfo))
+	return m.mapInfo
+}
+
+// Unmap will unmap the data inside this memory. Use this after calling Map on the Memory.
+func (m *Memory) Unmap() {
+	if m.mapInfo == nil {
+		return
+	}
+	C.gst_memory_unmap(m.Instance(), (*C.GstMapInfo)(m.mapInfo.Instance()))
 }
 
 // Bytes will return a byte slice of the data inside this memory if it is readable.
 func (m *Memory) Bytes() []byte {
-	mapInfo := m.Map()
-	if mapInfo.ptr == nil {
+	mapInfo := m.Map(MapRead)
+	if mapInfo == nil {
 		return nil
 	}
+	defer m.Unmap()
 	return mapInfo.Bytes()
 }
-
-// MapInfo is a go representation of a GstMapInfo.
-type MapInfo struct {
-	ptr *C.GstMapInfo
-}
-
-// Memory returns the underlying memory object.
-func (m *MapInfo) Memory() *Memory {
-	return wrapMemory(m.ptr.memory)
-}
-
-// Data returns a pointer to the raw data inside this map.
-func (m *MapInfo) Data() unsafe.Pointer {
-	return unsafe.Pointer(m.ptr.data)
-}
-
-// Flags returns the flags set on this map.
-func (m *MapInfo) Flags() MapFlags {
-	return MapFlags(m.ptr.flags)
-}
-
-// Size returrns the size of this map.
-func (m *MapInfo) Size() int64 {
-	return int64(m.ptr.size)
-}
-
-// MaxSize returns the maximum size of this map.
-func (m *MapInfo) MaxSize() int64 {
-	return int64(m.ptr.maxsize)
-}
-
-// Bytes returns a byte slice of the data inside this map info.
-func (m *MapInfo) Bytes() []byte {
-	return C.GoBytes(m.Data(), (C.int)(m.Size()))
-}
-
-// AsInt8Slice returns the contents of this map as a slice of signed 8-bit integers.
-func (m *MapInfo) AsInt8Slice() []int8 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]int8, m.Size())
-	for i := range out {
-		out[i] = int8(uint8sl[i])
-	}
-	return out
-}
-
-// AsInt16Slice returns the contents of this map as a slice of signed 16-bit integers.
-func (m *MapInfo) AsInt16Slice() []int16 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]int16, m.Size()/2)
-	for i := range out {
-		out[i] = int16(binary.LittleEndian.Uint16(uint8sl[i*2 : (i+1)*2]))
-	}
-	return out
-}
-
-// AsInt32Slice returns the contents of this map as a slice of signed 32-bit integers.
-func (m *MapInfo) AsInt32Slice() []int32 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]int32, m.Size()/4)
-	for i := range out {
-		out[i] = int32(binary.LittleEndian.Uint32(uint8sl[i*4 : (i+1)*4]))
-	}
-	return out
-}
-
-// AsInt64Slice returns the contents of this map as a slice of signed 64-bit integers.
-func (m *MapInfo) AsInt64Slice() []int64 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]int64, m.Size()/8)
-	for i := range out {
-		out[i] = int64(binary.LittleEndian.Uint64(uint8sl[i*8 : (i+1)*8]))
-	}
-	return out
-}
-
-// AsUint8Slice returns the contents of this map as a slice of unsigned 8-bit integers.
-func (m *MapInfo) AsUint8Slice() []uint8 {
-	out := make([]uint8, m.Size())
-	for i, t := range (*[1 << 30]uint8)(m.Data())[:m.Size():m.Size()] {
-		out[i] = t
-	}
-	return out
-}
-
-// AsUint16Slice returns the contents of this map as a slice of unsigned 16-bit integers.
-func (m *MapInfo) AsUint16Slice() []uint16 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]uint16, m.Size()/2)
-	for i := range out {
-		out[i] = uint16(binary.LittleEndian.Uint16(uint8sl[i*2 : (i+1)*2]))
-	}
-	return out
-}
-
-// AsUint32Slice returns the contents of this map as a slice of unsigned 32-bit integers.
-func (m *MapInfo) AsUint32Slice() []uint32 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]uint32, m.Size()/4)
-	for i := range out {
-		out[i] = uint32(binary.LittleEndian.Uint32(uint8sl[i*4 : (i+1)*4]))
-	}
-	return out
-}
-
-// AsUint64Slice returns the contents of this map as a slice of unsigned 64-bit integers.
-func (m *MapInfo) AsUint64Slice() []uint64 {
-	uint8sl := m.AsUint8Slice()
-	out := make([]uint64, m.Size()/8)
-	for i := range out {
-		out[i] = uint64(binary.LittleEndian.Uint64(uint8sl[i*8 : (i+1)*8]))
-	}
-	return out
-}
-
-// WriteData writes the given values directly to the map's memory.
-func (m *MapInfo) WriteData(data []uint8) {
-	for i, x := range data {
-		C.writeMapData(m.ptr, C.gint(i), C.guint8(x))
-	}
-}
-
-func wrapMapInfo(mapInfo *C.GstMapInfo, unmapFunc func()) *MapInfo {
-	info := &MapInfo{ptr: mapInfo}
-	runtime.SetFinalizer(info, func(_ *MapInfo) { unmapFunc() })
-	return info
-}

gst/gst_wrappers.go

@@ -169,6 +169,7 @@ func wrapEvent(ev *C.GstEvent) *Event                     { return &Event{ptr: e
 func wrapGhostPad(obj *glib.Object) *GhostPad             { return &GhostPad{wrapProxyPad(obj)} }
 func wrapMainContext(ctx *C.GMainContext) *MainContext    { return &MainContext{ptr: ctx} }
 func wrapMainLoop(loop *C.GMainLoop) *MainLoop            { return &MainLoop{ptr: loop} }
+func wrapMapInfo(mapInfo *C.GstMapInfo) *MapInfo          { return &MapInfo{ptr: mapInfo} }
 func wrapMemory(mem *C.GstMemory) *Memory                 { return &Memory{ptr: mem} }
 func wrapMessage(msg *C.GstMessage) *Message              { return &Message{msg: msg} }
 func wrapMeta(meta *C.GstMeta) *Meta                      { return &Meta{ptr: meta} }