[vlc-commits] picture_pool: improve and update documentation

[Rémi Denis-Courmont]

vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Mon Nov  3 20:42:57 2014 +0200| [d7d0b381722d4f1cc45e77b35c89d3b0bcbd792e] | committer: Rémi Denis-Courmont

picture_pool: improve and update documentation

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=d7d0b381722d4f1cc45e77b35c89d3b0bcbd792e
---

 include/vlc_picture_pool.h |   99 +++++++++++++++++++++++++++++---------------
 1 file changed, 66 insertions(+), 33 deletions(-)

diff --git a/include/vlc_picture_pool.h b/include/vlc_picture_pool.h
index aa51678..2b3718c 100644
--- a/include/vlc_picture_pool.h
+++ b/include/vlc_picture_pool.h
@@ -33,9 +33,6 @@
 
 /**
  * Picture pool handle
- *
- * XXX it is not thread safe, all pool manipulations and picture_Release
- * must be properly locked if needed.
  */
 typedef struct picture_pool_t picture_pool_t;
 
@@ -51,47 +48,75 @@ typedef struct {
 } picture_pool_configuration_t;
 
 /**
- * It creates a picture_pool_t wrapping the given configuration.
+ * Creates a pool of preallocated pictures. Free pictures can be allocated from
+ * the pool, and are returned to the pool when they are no longer referenced.
+ *
+ * This avoids allocating and deallocationg pictures repeatedly, and ensures
+ * that memory consumption remains within limits.
+ *
+ * To obtain a picture from the pool, use picture_pool_Get(). To increase and
+ * decrease the reference count, use picture_Hold() and picture_Release()
+ * respectively.
  *
- * It avoids useless picture creations/destructions.
- * The given picture must not have a reference count greater than 1.
- * The pool takes ownership of the picture and MUST not be used directly.
- * When deleted, the pool will release the pictures using picture_Release.
  * If defined, picture_pool_configuration_t::lock will be called before
  * a picture is used, and picture_pool_configuration_t::unlock will be called
- * as soon as a picture is unused. They are allowed to modify picture_t::p and
- * access picture_t::p_sys.
+ * as soon as a picture is returned to the pool.
+ * Those callbacks can modify picture_t::p and access picture_t::p_sys.
+ *
+ * @return A pointer to the new pool on success, or NULL on error
+ * (pictures are <b>not</b> released on error).
  */
 VLC_API picture_pool_t * picture_pool_NewExtended( const picture_pool_configuration_t * ) VLC_USED;
 
 /**
- * It creates a picture_pool_t wrapping the given arrays of picture.
+ * Creates a picture pool with pictures in a given array.
+ * This is a convenience wrapper for picture_pool_NewExtended() without the
+ * lock and unlock callbacks.
  *
- * It is provided as convenience.
+ * @param count number of pictures in the array
+ * @param tab array of pictures
+ *
+ * @return a pointer to the new pool on success, or NULL on error
+ * (pictures are <b>not</b> released on error)
  */
 VLC_API picture_pool_t * picture_pool_New(unsigned count,
                                           picture_t *const *tab) VLC_USED;
 
 /**
- * It creates a picture_pool_t creating images using the given format.
+ * Allocates pictures from the heap and creates a picture pool with them.
+ * This is a convenience wrapper for picture_NewFromFormat() and
+ * picture_pool_New().
+ *
+ * @param fmt video format of pictures to allocate from the heap
+ * @param count number of pictures to allocate
  *
- * Provided for convenience.
+ * @return a pointer to the new pool on success, NULL on error
  */
-VLC_API picture_pool_t * picture_pool_NewFromFormat(const video_format_t *,
+VLC_API picture_pool_t * picture_pool_NewFromFormat(const video_format_t *fmt,
                                                     unsigned count) VLC_USED;
 
 /**
- * It destroys a pool created by picture_pool_New.
+ * Releases a pool created by picture_pool_NewExtended(), picture_pool_New()
+ * or picture_pool_NewFromFormat().
+ *
+ * @note If there are no pending references to the pooled pictures, and the
+ * picture_resource_t.pf_destroy callback was not NULL, it will be invoked.
+ * Otherwise the default callback will be used.
  *
- * All pictures must already be released to the pool. The pool will then
- * released them.
+ * @warning If there are pending references (a.k.a. late pictures), the
+ * pictures will remain valid until the all pending references are dropped by
+ * picture_Release().
  */
 VLC_API void picture_pool_Release( picture_pool_t * );
 
 /**
- * It retreives a picture_t from a pool.
+ * Obtains a picture from a pool if any is immediately available.
  *
- * The picture must be release by using picture_Release.
+ * The picture must be released with picture_Release().
+ *
+ * @return a picture, or NULL if all pictures in the pool are allocated
+ *
+ * @note This function is thread-safe.
  */
 VLC_API picture_t * picture_pool_Get( picture_pool_t * ) VLC_USED;
 
@@ -104,6 +129,7 @@ VLC_API picture_t * picture_pool_Get( picture_pool_t * ) VLC_USED;
  * @note Allocated pictures may be accessed asynchronously by other threads.
  * Therefore, only read-only picture parameters can be read by the callback,
  * typically picture_t.p_sys.
+ * Provided those rules are respected, the function is thread-safe.
  */
 VLC_API void picture_pool_Enum( picture_pool_t *,
                                 void (*cb)(void *, picture_t *), void *data );
@@ -114,35 +140,42 @@ VLC_API void picture_pool_Enum( picture_pool_t *,
  * @warning This can only be called when it is known that all pending
  * references to the picture pool are stale, e.g. a decoder failed to
  * release pictures properly when it terminated.
- /
+ *
  * @return the number of picture references that were freed
  */
 unsigned picture_pool_Reset( picture_pool_t * );
 
 /**
- * It forces the next picture_pool_Get to return a picture even if no
- * pictures are free.
+ * Forcefully marks one picture free if all pictures in the pool are allocated.
+ *
+ * @warning This is intrinsically race-prone. If the freed picture is still
+ * used, video will be corrupt, and the process will likely crash.
  *
- * It does it by releasing itself the oldest used picture if none is
- * available.
- * XXX it should be used with great care, the only reason you may need
- * it is to workaround a bug.
+ * @bug Do not use this function. It will never work properly.
+ * Fix the decoder bugs instead.
  */
 void picture_pool_NonEmpty( picture_pool_t * );
 
 /**
- * It reserves picture_count pictures from the given pool and returns
- * a new pool with thoses pictures.
+ * Reserves pictures from a pool and creates a new pool with those.
+ *
+ * When the new pool is released, pictures are returned to the master pool.
+ * If the master pool was already released, pictures will be destroyed.
+ *
+ * @param count number of picture to reserve
+ *
+ * @return the new pool, or NULL if there were not enough pictures available
+ * or on error
  *
- * The master pool must be full.
- * The returned pool must be deleted before the master pool.
- * When deleted, all pictures return to the master pool.
+ * @note This function is thread-safe (but it might return NULL if other
+ * threads have already allocated too many pictures).
  */
 VLC_API picture_pool_t * picture_pool_Reserve(picture_pool_t *, unsigned count)
 VLC_USED;
 
 /**
- * It returns the size of the given pool.
+ * @return the total number of pictures in the given pool
+ * @note This function is thread-safe.
  */
 VLC_API unsigned picture_pool_GetSize(const picture_pool_t *);