memory: Require implementation to implement _share

Require the memory implementations to implement a share operation. This allows
us to remove the fallback share implementation which uses a different allocator
implementation and complicates things too much.
Update design doc a bit.

docs/design/part-memory.txt

@@ -50,6 +50,23 @@ Allocators
                                         GFreeFunc free_func, gsize maxsize,
                                         gsize offset, gsize size);
 
+Lifecycle
+~~~~~~~~~
+
+GstMemory objects are refcounted. When the GstMemory object is first created, it
+has a refcount of 1. 
+
+Each variable holding a reference to the GstMemory object is responsible for
+updating the refcount.
+
+The refcount determines the writability of the object. If the refcount > 1, it is
+by definition used by multipled objects and thus cannot be safely written to.
+
+When the refcount reaches 0, and thus no objects hold a reference anymore, we
+can free the memory. The GstMemoryFreeFunction of the allocator will be called
+to cleanup the memory.
+
+
 Memory layout
 ~~~~~~~~~~~~~
 
@@ -76,10 +93,48 @@ Memory layout
 
    void        gst_memory_resize     (GstMemory *mem, gsize offset, gsize size);
 
+ The visible memory region can currently only be made smaller.
+
+ The memory object is always readable. The memory block is writable when:
+
+  - the refcount is exactly 1
+  - the memory object has no parent, or if it has a parent, the parent is
+    writable.
+  - the memory object is not marked as READONLY.
+
 
 Data Access
 ~~~~~~~~~~~
 
+ Access to the memory region is always controlled with a map and unmap method
+ call. This allows the implementation to monitor the access patterns or set up
+ the required memory mappings when needed.
+
+ Mapping a memory region requires the caller to specify the access method: READ
+ and/or WRITE. For write access, the GstMemory object must be writable.
+
+ After the data has been accessed in the object, the unmap call must be
+ performed. The call will update the new memory size with the specified size.
 
 
+Copy
+~~~~
+
+ A GstMemory copy can be made with the gst_memory_copy() call. Normally,
+ allocators will implement a custom version of this function to make a copy of
+ the same kind of memory as the original one.
+
+ This is what the fallback version of the copy function will do, albeit slower
+ than what as custom implementation could do.
+
+ The copy operation is only required to copy the visible range of the memory
+ block.
+
+
+Share
+~~~~~
+
+ A memory region can be shared between GstMemory object with the
+ gst_memory_share() operation. 
+
 

gst/gstmemory.c

@@ -87,7 +87,6 @@ static const GstMemoryAllocator *_default_allocator;
 
 /* our predefined allocators */
 static const GstMemoryAllocator *_default_mem_impl;
-static const GstMemoryAllocator *_default_share_impl;
 
 /* initialize the fields */
 static void
@@ -95,7 +94,7 @@ _default_mem_init (GstMemoryDefault * mem, GstMemoryFlags flags,
     GstMemory * parent, gsize slice_size, gpointer data,
     GFreeFunc free_func, gsize maxsize, gsize offset, gsize size)
 {
-  mem->mem.allocator = data ? _default_mem_impl : _default_share_impl;
+  mem->mem.allocator = _default_mem_impl;
   mem->mem.flags = flags;
   mem->mem.refcount = 1;
   mem->mem.parent = parent ? gst_memory_ref (parent) : NULL;
@@ -189,22 +188,6 @@ _default_mem_map (GstMemoryDefault * mem, gsize * size, gsize * maxsize,
   return mem->data + mem->offset;
 }
 
-static gpointer
-_default_share_map (GstMemoryDefault * mem, gsize * size, gsize * maxsize,
-    GstMapFlags flags)
-{
-  guint8 *data;
-
-  data = gst_memory_map (mem->mem.parent, size, maxsize, flags);
-
-  if (size)
-    *size = mem->size;
-  if (maxsize)
-    *maxsize -= mem->offset;
-
-  return data + mem->offset;
-}
-
 static gboolean
 _default_mem_unmap (GstMemoryDefault * mem, gpointer data, gsize size)
 {
@@ -213,23 +196,6 @@ _default_mem_unmap (GstMemoryDefault * mem, gpointer data, gsize size)
   return TRUE;
 }
 
-static gboolean
-_default_share_unmap (GstMemoryDefault * mem, gpointer data, gsize size)
-{
-  gboolean res;
-  guint8 *ptr = data;
-
-  if (size != -1)
-    mem->size = size;
-  else
-    size = mem->size - mem->offset;
-
-  res =
-      gst_memory_unmap (mem->mem.parent, ptr - mem->offset, size + mem->offset);
-
-  return res;
-}
-
 static void
 _default_mem_free (GstMemoryDefault * mem)
 {
@@ -307,16 +273,6 @@ _fallback_copy (GstMemory * mem, gsize offset, gsize size)
   return (GstMemory *) copy;
 }
 
-static GstMemory *
-_fallback_share (GstMemory * mem, gsize offset, gsize size)
-{
-  GstMemoryDefault *sub;
-
-  sub = _default_mem_new (0, mem, NULL, NULL, size, offset, size);
-
-  return (GstMemory *) sub;
-}
-
 static gboolean
 _fallback_is_span (GstMemory * mem1, GstMemory * mem2, gsize * offset)
 {
@@ -324,7 +280,7 @@ _fallback_is_span (GstMemory * mem1, GstMemory * mem2, gsize * offset)
 }
 
 static GStaticRWLock lock = G_STATIC_RW_LOCK_INIT;
-static GHashTable *memoryimpl;
+static GHashTable *allocators;
 
 void
 _gst_memory_init (void)
@@ -341,25 +297,11 @@ _gst_memory_init (void)
     (GstMemoryIsSpanFunction) _default_mem_is_span,
     NULL
   };
-  static const GstMemoryInfo _share_info = {
-    (GstMemoryAllocFunction) _default_mem_alloc,
-    (GstMemoryGetSizesFunction) _default_mem_get_sizes,
-    (GstMemoryResizeFunction) _default_mem_resize,
-    (GstMemoryMapFunction) _default_share_map,
-    (GstMemoryUnmapFunction) _default_share_unmap,
-    (GstMemoryFreeFunction) _default_mem_free,
-    NULL,
-    NULL,
-    NULL,
-    NULL
-  };
 
-  memoryimpl = g_hash_table_new (g_str_hash, g_str_equal);
+  allocators = g_hash_table_new (g_str_hash, g_str_equal);
 
   _default_mem_impl =
       gst_memory_allocator_register ("GstMemoryDefault", &_mem_info);
-  _default_share_impl =
-      gst_memory_allocator_register ("GstMemorySharebuffer", &_share_info);
 
   _default_allocator = _default_mem_impl;
 }
@@ -591,7 +533,7 @@ gst_memory_is_span (GstMemory * mem1, GstMemory * mem2, gsize * offset)
  * Registers the memory allocator with @name and implementation functions
  * @info.
  *
- * All functions in @info are mandatory exept the copy, share and is_span
+ * All functions in @info are mandatory exept the copy and is_span
  * functions, which will have a default implementation when left NULL.
  *
  * The user_data field in @info will be passed to all calls of the alloc
@@ -615,19 +557,19 @@ gst_memory_allocator_register (const gchar * name, const GstMemoryInfo * info)
   g_return_val_if_fail (info->map != NULL, NULL);
   g_return_val_if_fail (info->unmap != NULL, NULL);
   g_return_val_if_fail (info->free != NULL, NULL);
+  g_return_val_if_fail (info->share != NULL, NULL);
 
   allocator = g_slice_new (GstMemoryAllocator);
   allocator->name = g_quark_from_string (name);
   allocator->info = *info;
   INSTALL_FALLBACK (copy);
-  INSTALL_FALLBACK (share);
   INSTALL_FALLBACK (is_span);
 #undef INSTALL_FALLBACK
 
   GST_DEBUG ("register \"%s\" of size %" G_GSIZE_FORMAT, name);
 
   g_static_rw_lock_writer_lock (&lock);
-  g_hash_table_insert (memoryimpl, (gpointer) name, (gpointer) allocator);
+  g_hash_table_insert (allocators, (gpointer) name, (gpointer) allocator);
   g_static_rw_lock_writer_unlock (&lock);
 
   return allocator;
@@ -650,7 +592,7 @@ gst_memory_allocator_find (const gchar * name)
 
   g_static_rw_lock_reader_lock (&lock);
   if (name) {
-    allocator = g_hash_table_lookup (memoryimpl, (gconstpointer) name);
+    allocator = g_hash_table_lookup (allocators, (gconstpointer) name);
   } else {
     allocator = _default_allocator;
   }

tests/check/gst/gstmeta.c

@@ -130,8 +130,10 @@ GST_START_TEST (test_meta_test)
   gpointer data;
 
   buffer = gst_buffer_new_and_alloc (4);
+  fail_if (buffer == NULL);
 
   data = gst_buffer_map (buffer, NULL, NULL, GST_MAP_WRITE);
+  fail_if (data == NULL);
   memset (data, 0, 4);
   gst_buffer_unmap (buffer, data, 4);