[FFmpeg-devel] [PATCH][VAAPI][2/6] Add common data structures and helpers

[Michael Niedermayer]

On Thu, Feb 26, 2009 at 10:54:53AM +0100, Gwenole Beauchesne wrote:
> Hi,
>
> This patch adds common data structures and helper functions for VA API 
> support. This greatly simplify actual implementation.
>
> struct vaapi_render_state is now reduced to the strict minimum from a 
> public POV. Users are required to allocate this through 
> av_alloc_vaapi_render_state() as it is actually larger to hold all internal 
> data needed for decoding and that the user shouldn't care about.
> - av_alloc_vaapi_render_state()
> - av_free_vaapi_render_state()
>
> The buf/buf_size args to AVHWAccel::start_frame() are particularly useful 
> for optimization purposes. Whenever possible, slice data chunks are 
> directly copied to mapped HW memory. Otherwise, they are accumulated to a 
> temporary buffer that will be copied in end_frame().
> - vaapi_slice_data_prepare() [usually called in ::start_frame()]
> - vaapi_slice_data_append()  [usually called in ::decode_slice()]
> - vaapi_slice_data_commit()  [usually called in ::end_frame()]
>
> The same happens for slice control blocks. If we can predetermine the 
> number of slices, we can map the whole slice params buffer from HW memory 
> space. Otherwise, they are accumulated to a temporary buffer that is also 
> copied in end_frame().
> - vaapi_slice_params_prepare() [usually called in ::start_frame()]
> - vaapi_slice_params_next()    [usually called in ::decode_slice()]
> - vaapi_slice_params_commit()  [usually called in ::end_frame()]
>
> Other helpers include:
> - vaapi_common_end_frame(): common commit code to the HW accelerator
> - vaapi_render_picture(): doing the actual rendering
>
> Regards,
> Gwenole.

> commit 6bfd94e5df61badca034b34b3ba379f121db27a8
> Author: Gwenole Beauchesne <gbeauchesne at splitted-desktop.com>
> Date:   Wed Feb 25 15:19:36 2009 +0000
> 
>     Add common VA API data structures and helpers.
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index afa5fac..b5a0ad1 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -3,7 +3,7 @@ include $(SUBDIR)../config.mak
>  NAME = avcodec
>  FFLIBS = avutil
>  
> -HEADERS = avcodec.h opt.h vdpau.h xvmc.h
> +HEADERS = avcodec.h opt.h vaapi.h vdpau.h xvmc.h
>  
>  OBJS = allcodecs.o                                                      \
>         audioconvert.o                                                   \

i think the struct placed in data[0] should be designed so that it does not
need any vaapi headers, that is contain pointers to structs instead of
vaapi structs.
then this "data[0] struct" can just be placed in avcodec.h and vaapi.h wont
be needed.


[...]
> +#if DEBUG
> +#define D(x) x
> +#define bug(...) dprintf(NULL, __VA_ARGS__)
> +#else
> +#define D(x)
> +#endif

useless
dprintf() already is #defined away if debuging is disabled


> +
> +/**
> + * \addtogroup VAAPI_Decoding
> + *
> + * @{
> + */
> +

> +/**
> + * \brief This structure holds all temporary data for VA API
> + * decoding. It's an extension of the vaapi_render_state structure.
> + */

you mix doxy with and without \brief,
without is standard in ffmpeg and the first sentance is used a brief anyway


[...]
> +/**
> + * \brief This structure is used as a callback between the FFmpeg
> + * library and the client video application.
> + * This is defined by the client application prior to calling the
> + * FFmpeg decode functions. The members are considered read-only from
> + * an FFmpeg point of view.
> + */
> +struct vaapi_context {
> +    VADisplay   display;                ///< Window system dependent data
> +    VAConfigID  config_id;              ///< Configuration ID
> +    VAContextID context_id;             ///< Context ID (video decode pipeline)
> +};
> +
> +/**
> + * \brief This structure is used as a callback between the FFmpeg
> + * library and the client video application.
> + * This is created by the client application prior to calling the
> + * FFmpeg decode functions. You must call
> + * av_alloc_vaapi_render_state() to allocate this structure and fill
> + * both the surface and va_context fields.
> + */
> +struct vaapi_render_state {
> +    uint32_t    magic;                  ///< Magic number identifying the structure
> +    uint32_t    state;                  ///< Holds FF_VAAPI_STATE_* values
> +    struct vaapi_context *va_context;   ///< Pointer to display-dependent data
> +    VASurfaceID surface;                ///< Rendering surface, never changed
> +    // ... there are other (private) fields beyond this point
> +};
> +
> +/** \brief Allocate a new VA API render state structure. */
> +struct vaapi_render_state *av_alloc_vaapi_render_state(void);

do i understand correctly that the use provided get_buffer() is called and
then the user calls av_alloc_vaapi_render_state()?
this is ugly
either lavc should already call av_alloc_vaapi_render_state() before
get_buffer()
or it should be done in default_get_buffer() and the users get_buffer() should
call default_get_buffer()
or allocate it by avpicture_alloc()
i dunno which of these would be cleanest ...


> +
> +/** \brief Destroy VA API render state structure. */
> +void av_free_vaapi_render_state(struct vaapi_render_state *rds);
> +

same


> +/** \brief Extract the vaapi_render_state struct from an AVPicture. */
> +static inline struct vaapi_render_state *ff_get_vaapi_render_state(void *pic)

a AVPicture isnt void *

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090226/639fa764/attachment.pgp>