I think your understanding about VA_RENDER_DEVICE_EXTERNAL/VA_RENDER_MODE_EXTERNAL_xxx is right. So the changes for VADisplayAttribRenderMode and VADisplayAttribRenderDevice isn't needed. As for the rendering is done in the client or in X server, it is up to the implementation. If you want to control the behavior of rendering, you can consider to add a new attribute.
Thanks Haihao > Ok, I can make the changes for VADisplayAttribRenderMode and > VADisplayAttribRenderDevice. > > Anyway, I should just send the new patch with above changes? Or resend all > the patches set again? > > > Thanks > Teng > > -----Original Message----- > From: Zhao, Yakui > Sent: Tuesday, September 17, 2013 1:34 PM > To: Ung, Teng En > Cc: [email protected] > Subject: RE: [Libva] [PATCH 1/4] Initial code for VAAPI/XV interface. Addin > empty i965_output_xv module, render mode attributes, and build code to detect > XV. > > On Mon, 2013-09-16 at 22:36 -0600, Ung, Teng En wrote: > > Hi, > > > > From va.h in libva, the VADisplayAttribRenderMode and > > VADisplayAttribRenderDevice seems related to each other. Cause see some > > "*_LOCAL*" and "*_EXTERNAL*" values in both the attributes, and > > VADisplayAttribRenderDevice specify external display are for display device > > not aware from the window manager. Thus, using the local device (display) > > and local texture/overlay. > > > > Anyway, let me know if I miss understood those. > > For the case of Xv in intel vaapi, it seems that it is enough to define the > VADisplayAttribRenderMode. And it is unnecessary to define the attribute of > VADisplayAttribRenderDevice. > > If XV is handled directly in the intel-vaapi driver, maybe it is better that > it is called as "_LOCAL". If Xv is handled in Xserver, it is called as the > "_EXTERNAL", which is outside of intel-vaapi driver. > > Thanks > > > > > Thanks > > Teng > > > > -----Original Message----- > > From: Zhao, Yakui > > Sent: Friday, September 13, 2013 1:40 PM > > To: Ung, Teng En > > Cc: [email protected] > > Subject: Re: [Libva] [PATCH 1/4] Initial code for VAAPI/XV interface. Addin > > empty i965_output_xv module, render mode attributes, and build code to > > detect XV. > > > > On Thu, 2013-09-12 at 20:16 -0600, Ung, Teng En wrote: > > > This add in the configuration and makefile to detect XV and build > > > the i965_ouput_xv module. Also included the > > > VADisplayRenderModeAttr, will only use overlay is user set to overlay > > > render mode. > > > > > > > Generally speaking, this patch set is OK to me except some small concerns. > > > > > > > Signed-off-by: Ung, Teng En <[email protected]> > > > --- > > > configure.ac | 18 ++++++++++ > > > src/Makefile.am | 5 +++ > > > src/i965_drv_video.c | 53 ++++++++++++++++++++++++++++-- > > > src/i965_drv_video.h | 4 +++ src/i965_output_xv.c | 92 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > src/i965_output_xv.h | 49 ++++++++++++++++++++++++++++ > > > 6 files changed, 219 insertions(+), 2 deletions(-) create mode > > > 100644 src/i965_output_xv.c create mode 100644 src/i965_output_xv.h > > > > > > diff --git a/configure.ac b/configure.ac index e39f1d5..79c7642 > > > 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -51,6 +51,11 @@ AC_ARG_ENABLE(x11, > > > [build with VA/X11 API support > > > @<:@default=yes@:>@])], > > > [], [enable_x11="yes"]) > > > > > > +AC_ARG_ENABLE(xv, > > > + [AC_HELP_STRING([--enable-xv], > > > + [build with X11/XV API support > > > @<:@default=yes@:>@])], > > > + [], [enable_xv="yes"]) > > > + > > > AC_ARG_ENABLE([wayland], > > > [AC_HELP_STRING([--enable-wayland], > > > [build with VA/Wayland API support > > > @<:@default=yes@:>@])], @@ -123,6 +128,18 @@ if test "$USE_X11" = > > > "yes"; then fi AM_CONDITIONAL(USE_X11, test "$USE_X11" = "yes") > > > > > > +dnl Check for X11/XV API > > > +USE_XV="$enable_xv" > > > +if test "$USE_XV" = "yes"; then > > > + PKG_CHECK_MODULES([X11], [x11], [:], [USE_XV="no"]) > > > + PKG_CHECK_MODULES([XEXT], [xext], [:], [USE_XV="no"]) > > > + PKG_CHECK_MODULES([XV], [xv], [:], [USE_XV="no"]) > > > + if test "$USE_XV" = "yes"; then > > > + AC_DEFINE([HAVE_X11_XV], [1], [Defined to 1 if X11/XV API is > > > built]) > > > + fi > > > +fi > > > +AM_CONDITIONAL(USE_XV, test "$USE_XV" = "yes") > > > + > > > dnl Check for VA-API drivers path > > > AC_MSG_CHECKING([for VA drivers path]) > > > LIBVA_DRIVERS_PATH=`$PKG_CONFIG libva --variable driverdir` @@ > > > -188,6 > > > +205,7 @@ dnl Print summary BACKENDS="" > > > AS_IF([test "$USE_DRM" = "yes"], [BACKENDS="$BACKENDS drm"]) > > > AS_IF([test "$USE_X11" = "yes"], [BACKENDS="$BACKENDS x11"]) > > > +AS_IF([test "$USE_XV" = "yes"], [BACKENDS="$BACKENDS xv"]) > > > AS_IF([test "$USE_WAYLAND" = "yes"], [BACKENDS="$BACKENDS > > > wayland"]) > > > > > > echo > > > diff --git a/src/Makefile.am b/src/Makefile.am index > > > 3299733..48a49f1 > > > 100755 > > > --- a/src/Makefile.am > > > +++ b/src/Makefile.am > > > @@ -129,6 +129,11 @@ noinst_HEADERS = $(source_h) > > > if USE_X11 > > > source_c += i965_output_dri.c > > > source_h += i965_output_dri.h > > > +if USE_XV > > > +source_c += i965_output_xv.c > > > +source_h += i965_output_xv.h > > > +driver_cflags += -lXext -lX11 -lXv > > > +endif > > > endif > > > > > > if USE_WAYLAND > > > diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c index > > > ea1f1d0..fb23c8d 100755 > > > --- a/src/i965_drv_video.c > > > +++ b/src/i965_drv_video.c > > > @@ -31,6 +31,9 @@ > > > > > > #ifdef HAVE_VA_X11 > > > # include "i965_output_dri.h" > > > +#ifdef HAVE_X11_XV > > > +# include "i965_output_xv.h" > > > +#endif > > > #endif > > > > > > #ifdef HAVE_VA_WAYLAND > > > @@ -101,6 +104,27 @@ static const VADisplayAttribute > > > i965_display_attributes[] = { > > > 0, 3, VA_ROTATION_NONE, > > > VA_DISPLAY_ATTRIB_GETTABLE|VA_DISPLAY_ATTRIB_SETTABLE > > > }, > > > + > > > +#ifdef HAVE_X11_XV > > > + /* The render device attribute. This attributes is used to determine > > > + * whether the device is a local or external before setting the > > > + * gpu/overlay rendering attribute. > > > + */ > > > + { > > > + VADisplayAttribRenderDevice, > > > + 1, 1, VA_RENDER_DEVICE_LOCAL, > > > + VA_DISPLAY_ATTRIB_GETTABLE > > > + }, > > > + /* Render mode attribute. Use this attribute to control the output > > > in > > > + * X11 going through DRI (GPU) or XV (overlay/sprite) rendering mode. > > > + * The defeult is DRI rendering mode. > > > + */ > > > + { > > > + VADisplayAttribRenderMode, > > > + 1, 2, VA_RENDER_MODE_LOCAL_GPU, > > > + VA_DISPLAY_ATTRIB_GETTABLE|VA_DISPLAY_ATTRIB_SETTABLE > > > + }, > > > +#endif > > > }; > > > > > > /* List of supported image formats */ @@ -2412,6 +2436,12 @@ > > > i965_display_attributes_init(VADriverContextP ctx) > > > if (!i965->rotation_attrib) { > > > goto error; > > > } > > > +#ifdef HAVE_X11_XV > > > + i965->rendermode_attrib = get_display_attribute(ctx, > > > VADisplayAttribRenderMode); > > > + if (!i965->rendermode_attrib) { > > > + goto error; > > > + } > > > +#endif > > > return true; > > > > > > error: > > > @@ -3901,6 +3931,10 @@ i965_PutSurface(VADriverContextP ctx, > > > unsigned int flags) /* de-interlacing flags */ { > > > #ifdef HAVE_VA_X11 > > > +#ifdef HAVE_X11_XV > > > + struct i965_driver_data *i965 = i965_driver_data(ctx); #endif > > > + > > > if (IS_VA_X11(ctx)) { > > > VARectangle src_rect, dst_rect; > > > > > > @@ -3914,8 +3948,15 @@ i965_PutSurface(VADriverContextP ctx, > > > dst_rect.width = destw; > > > dst_rect.height = desth; > > > > > > - return i965_put_surface_dri(ctx, surface, draw, &src_rect, > > > &dst_rect, > > > - cliprects, number_cliprects, flags); > > > +#ifdef HAVE_X11_XV > > > + /* Check whether using GPU or Overlay rendering mode. */ > > > + if (i965->rendermode_attrib->value == > > > VA_RENDER_MODE_LOCAL_OVERLAY) > > > + return i965_put_surface_xv(ctx, surface, draw, &src_rect, > > > &dst_rect, > > > + cliprects, number_cliprects, > > > flags); > > > + else > > > +#endif > > > > The surface buffer is sent to Xserver and then the Xserver uses the overlay > > to display the corresponding content. > > Does it use the flag of "VA_RENDRR_MODE_LOCAL_OVERLAY"? Maybe the > > VA_RENDER_MODE_EXTERNAL_OVERLAY is better. > > > > > + return i965_put_surface_dri(ctx, surface, draw, &src_rect, > > > &dst_rect, > > > + cliprects, > > > + number_cliprects, flags); > > > } > > > #endif > > > return VA_STATUS_ERROR_UNIMPLEMENTED; @@ -4887,6 +4928,14 @@ > > > struct { > > > i965_output_dri_terminate, > > > VA_DISPLAY_X11, > > > }, > > > + > > > +#ifdef HAVE_X11_XV > > > + { > > > + i965_output_xv_init, > > > + i965_output_xv_terminate, > > > + VA_DISPLAY_X11, > > > + }, > > > +#endif > > > #endif > > > }; > > > > > > diff --git a/src/i965_drv_video.h b/src/i965_drv_video.h index > > > a0e7790..637812d 100644 > > > --- a/src/i965_drv_video.h > > > +++ b/src/i965_drv_video.h > > > @@ -321,6 +321,7 @@ struct i965_driver_data > > > VADisplayAttribute *display_attributes; > > > unsigned int num_display_attributes; > > > VADisplayAttribute *rotation_attrib; > > > + VADisplayAttribute *rendermode_attrib; > > > VAContextID current_context_id; > > > > > > /* VA/DRI (X11) specific data */ @@ -328,6 +329,9 @@ struct > > > i965_driver_data > > > > > > /* VA/Wayland specific data */ > > > struct va_wl_output *wl_output; > > > + > > > + /* X11/XV (X11) specific data */ > > > + struct va_xv_output *xv_output; > > > }; > > > > > > #define NEW_CONFIG_ID() object_heap_allocate(&i965->config_heap); > > > diff --git a/src/i965_output_xv.c b/src/i965_output_xv.c new file > > > mode > > > 100644 index 0000000..8b361da > > > --- /dev/null > > > +++ b/src/i965_output_xv.c > > > @@ -0,0 +1,92 @@ > > > +/* > > > + * Copyright (C) 2012 Intel Corporation. All Rights Reserved. > > > + * > > > + * Permission is hereby granted, free of charge, to any person > > > +obtaining a > > > + * copy of this software and associated documentation files (the > > > + * "Software"), to deal in the Software without restriction, > > > +including > > > + * without limitation the rights to use, copy, modify, merge, > > > +publish, > > > + * distribute, sub license, and/or sell copies of the Software, and > > > +to > > > + * permit persons to whom the Software is furnished to do so, > > > +subject to > > > + * the following conditions: > > > + * > > > + * The above copyright notice and this permission notice (including > > > +the > > > + * next paragraph) shall be included in all copies or substantial > > > +portions > > > + * of the Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > > +EXPRESS > > > + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > > > NON-INFRINGEMENT. > > > + * IN NO EVENT SHALL PRECISION INSIGHT AND/OR ITS SUPPLIERS BE > > > +LIABLE FOR > > > + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF > > > +CONTRACT, > > > + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH > > > +THE > > > + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > > > + */ > > > + > > > +#include "sysdeps.h" > > > + > > > +#include <va/va_backend.h> > > > + > > > +#include "i965_defines.h" > > > +#include "i965_drv_video.h" > > > +#include "i965_output_xv.h" > > > + > > > +struct va_xv_output { > > > + int vaxv_port; > > > + unsigned int vaxv_format; > > > +}; > > > + > > > + > > > +bool > > > +i965_output_xv_init(VADriverContextP ctx) { > > > + struct i965_driver_data * const i965 = i965_driver_data(ctx); > > > + > > > + i965->xv_output = calloc(1, sizeof(struct va_xv_output)); > > > + if (!i965->xv_output) { > > > + goto error; > > > + } > > > + > > > + /* Need to addin implementation, currently just disable all those > > > + * overlay and render attributes. > > > + */ > > > + i965->num_display_attributes = 1; > > > + > > > + return true; > > > + > > > +error : > > > + i965->num_display_attributes = 1; > > > + i965_output_xv_terminate(ctx); > > > + > > > + /* Just return true to not break the i965_init */ > > > + return true; > > > +} > > > + > > > +void > > > +i965_output_xv_terminate(VADriverContextP ctx) { > > > + struct i965_driver_data * const i965 = i965_driver_data(ctx); > > > + > > > + if (!xv_output) > > > + return; > > > + > > > + free(xv_output); > > > + i965->xv_output = NULL; > > > +} > > > + > > > +VAStatus > > > +i965_put_surface_xv( > > > + VADriverContextP ctx, > > > + VASurfaceID surface, > > > + void *draw, > > > + const VARectangle *src_rect, > > > + const VARectangle *dst_rect, > > > + const VARectangle *cliprects, > > > + unsigned int num_cliprects, > > > + unsigned int flags > > > +) > > > +{ > > > + struct i965_driver_data * const i965 = i965_driver_data(ctx); > > > + struct va_xv_output * const xv_output = i965->xv_output; > > > + > > > + return VA_STATUS_ERROR_UNIMPLEMENTED; } > > > diff --git a/src/i965_output_xv.h b/src/i965_output_xv.h new file > > > mode > > > 100644 index 0000000..7d0cb91 > > > --- /dev/null > > > +++ b/src/i965_output_xv.h > > > @@ -0,0 +1,49 @@ > > > +/* > > > + * Copyright (C) 2012 Intel Corporation. All Rights Reserved. > > > + * > > > + * Permission is hereby granted, free of charge, to any person > > > +obtaining a > > > + * copy of this software and associated documentation files (the > > > + * "Software"), to deal in the Software without restriction, > > > +including > > > + * without limitation the rights to use, copy, modify, merge, > > > +publish, > > > + * distribute, sub license, and/or sell copies of the Software, and > > > +to > > > + * permit persons to whom the Software is furnished to do so, > > > +subject to > > > + * the following conditions: > > > + * > > > + * The above copyright notice and this permission notice (including > > > +the > > > + * next paragraph) shall be included in all copies or substantial > > > +portions > > > + * of the Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > > +EXPRESS > > > + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > > > NON-INFRINGEMENT. > > > + * IN NO EVENT SHALL PRECISION INSIGHT AND/OR ITS SUPPLIERS BE > > > +LIABLE FOR > > > + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF > > > +CONTRACT, > > > + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH > > > +THE > > > + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > > > + */ > > > + > > > +#ifndef I965_OUTPUT_XV_H > > > +#define I965_OUTPUT_XV_H > > > + > > > +#include <stdbool.h> > > > +#include <va/va_backend.h> > > > + > > > +bool > > > +i965_output_xv_init(VADriverContextP ctx); > > > + > > > +void > > > +i965_output_xv_terminate(VADriverContextP ctx); > > > + > > > +VAStatus > > > +i965_put_surface_xv( > > > + VADriverContextP ctx, > > > + VASurfaceID surface, > > > + void *draw, > > > + const VARectangle *src_rect, > > > + const VARectangle *dst_rect, > > > + const VARectangle *cliprects, > > > + unsigned int num_cliprects, > > > + unsigned int flags > > > +); > > > + > > > +#endif /* I965_OUTPUT_XV_H */ > > > > > > > _______________________________________________ > Libva mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/libva _______________________________________________ Libva mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libva
