Discussion:
[PATCH] Don't automatically destroy the buffer(s) passed to vaRenderPicture
(too old to reply)
Xiang, Haihao
2016-12-01 08:55:42 UTC
Permalink
Instead the user must call vaDestroyBuffer() to destroy a buffer explicitly.

If following the previous API specification,
1. Violate "who allocate who release" principle
2. The user cannot re-use VA buffer flexibly
3. The user still has to call vaDestroyBuffer() to destroy the buffers which
are not going to be passed to vaRenderPicture()

We discussed the change at https://bugs.freedesktop.org/show_bug.cgi?id=97970

Signed-off-by: Xiang, Haihao <***@intel.com>
---
va/va.h | 9 +++++++--
va/va_enc_h264.h | 3 +--
va/va_vpp.h | 12 +++---------
3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/va/va.h b/va/va.h
index 8791906..86bef97 100644
--- a/va/va.h
+++ b/va/va.h
@@ -2156,6 +2156,7 @@ typedef struct _VAEncPictureParameterBufferMPEG4
* eliminates this copy is to pass null as "data" when calling vaCreateBuffer(),
* and then use vaMapBuffer() to map the data store from the server side to the
* client address space for access.
+ * The user must call vaDestroyBuffer() to destroy a buffer.
* Note: image buffers are created by the library, not the client. Please see
* vaCreateImage on how image buffers are managed.
*/
@@ -2276,7 +2277,12 @@ VAStatus vaUnmapBuffer (

/**
* After this call, the buffer is deleted and this buffer_id is no longer valid
- * Only call this if the buffer is not going to be passed to vaRenderBuffer
+ *
+ * A buffer can be re-used and sent to the server by another Begin/Render/End
+ * sequence if vaDestroyBuffer() is not called with this buffer.
+ *
+ * Note re-using a shared buffer (e.g. a slice data buffer) between the host and the
+ * hardware accelerator can result in performance dropping.
*/
VAStatus vaDestroyBuffer (
VADisplay dpy,
@@ -2404,7 +2410,6 @@ VAStatus vaBeginPicture (

/**
* Send decode buffers to the server.
- * Buffers are automatically destroyed afterwards
*/
VAStatus vaRenderPicture (
VADisplay dpy,
diff --git a/va/va_enc_h264.h b/va/va_enc_h264.h
index 604877f..2e7eb8d 100644
--- a/va/va_enc_h264.h
+++ b/va/va_enc_h264.h
@@ -386,8 +386,7 @@ typedef struct _VAEncPictureParameterBufferH264 {
*
* If per-macroblock encoder configuration is needed, \c macroblock_info
* references a buffer of type #VAEncMacroblockParameterBufferH264. This
- * buffer is not passed to vaRenderPicture(). i.e. it is not destroyed
- * by subsequent calls to vaRenderPicture() and then can be re-used
+ * buffer is not passed to vaRenderPicture() and it can be re-used
* without re-allocating the whole buffer.
*/
typedef struct _VAEncSliceParameterBufferH264 {
diff --git a/va/va_vpp.h b/va/va_vpp.h
index 8ac0923..082f1d5 100644
--- a/va/va_vpp.h
+++ b/va/va_vpp.h
@@ -371,11 +371,9 @@ typedef struct _VAProcFilterValueRange {
/**
* \brief Video processing pipeline configuration.
*
- * This buffer defines a video processing pipeline. As for any buffer
- * passed to \c vaRenderPicture(), this is a one-time usage model.
- * However, the actual filters to be applied are provided in the
- * \c filters field, so they can be re-used in other processing
- * pipelines.
+ * This buffer defines a video processing pipeline. The actual filters to
+ * be applied are provided in the \c filters field, they can be re-used
+ * in other processing pipelines.
*
* The target surface is specified by the \c render_target argument of
* \c vaBeginPicture(). The general usage model is described as follows:
@@ -491,10 +489,6 @@ typedef struct _VAProcPipelineParameterBuffer {
* #VA_STATUS_ERROR_UNSUPPORTED_FILTER is returned if the list
* contains an unsupported filter.
*
- * Note: no filter buffer is destroyed after a call to vaRenderPicture(),
- * only this pipeline buffer will be destroyed as per the core API
- * specification. This allows for flexibility in re-using the filter for
- * other surfaces to be processed.
*/
VABufferID *filters;
/** \brief Actual number of filters. */
--
1.9.1
Mark Thompson
2016-12-05 18:11:12 UTC
Permalink
Post by Xiang, Haihao
Instead the user must call vaDestroyBuffer() to destroy a buffer explicitly.
If following the previous API specification,
1. Violate "who allocate who release" principle
2. The user cannot re-use VA buffer flexibly
3. The user still has to call vaDestroyBuffer() to destroy the buffers which
are not going to be passed to vaRenderPicture()
We discussed the change at https://bugs.freedesktop.org/show_bug.cgi?id=97970
---
va/va.h | 9 +++++++--
va/va_enc_h264.h | 3 +--
va/va_vpp.h | 12 +++---------
3 files changed, 11 insertions(+), 13 deletions(-)
This is a totally incompatible change to VAAPI, so please make libva before and after this change clearly distinguishable.

Probably you should increment the major version number and change the SONAME of libva, though I realise commercial concerns will likely block that. At least increment the minor version number if nothing else can be done.

Thanks,

- Mark

Loading...