[libav-devel] [PATCH] hwaccel: Video Decoder Acceleration (VDA) support

Diego Biurrun diego at biurrun.de
Wed Nov 2 23:45:31 CET 2011


Another fresh patch with some of these suggestions already applied
will arrive on the ml soon.

On Wed, Nov 02, 2011 at 10:44:41PM +0100, Diego Biurrun wrote:
> 
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -45,6 +45,7 @@ RDFT-OBJS-$(CONFIG_HARDCODED_TABLES)   += sin_tables.o
>  OBJS-$(CONFIG_SINEWIN)                 += sinewin.o
>  OBJS-$(CONFIG_VAAPI)                   += vaapi.o
> +OBJS-$(CONFIG_VDA)                     += vda.o
>  OBJS-$(CONFIG_VDPAU)                   += vdpau.o
>  
> @@ -178,6 +179,7 @@ OBJS-$(CONFIG_H264_DECODER)            += h264.o                               \
>                                            mpegvideo.o error_resilience.o
>  OBJS-$(CONFIG_H264_VAAPI_HWACCEL)      += vaapi_h264.o
> +OBJS-$(CONFIG_H264_VDA_HWACCEL)        += vda_h264.o
>  OBJS-$(CONFIG_HUFFYUV_DECODER)         += huffyuv.o
>  OBJS-$(CONFIG_HUFFYUV_ENCODER)         += huffyuv.o

How much sense does VDA make without H.264 VDA?  It seems to me both
should just be enabled together.

> --- /dev/null
> +++ b/libavcodec/vda.c
> @@ -0,0 +1,264 @@
> +
> +static int64_t vda_pts_from_dictionary(CFDictionaryRef user_info)
> +{
> +    CFNumberRef pts;
> +    int64_t outValue = 0;
> +
> +    if (NULL == user_info)

Just

  if (!user_info)

is preferred.

> +static void vda_decoder_callback(void *vda_hw_ctx,
> +                                 CFDictionaryRef user_info,
> +                                 OSStatus status,
> +                                 uint32_t infoFlags,
> +                                 CVImageBufferRef image_buffer)
> +{
> +    struct vda_context *vda_ctx = (struct vda_context*)vda_hw_ctx;

This void* cast should be unnecessary.

> +    new_frame = (vda_frame*)av_mallocz(sizeof(vda_frame));

This void* cast is unnecessary.

> +int ff_vda_create_decoder(struct vda_context *vda_ctx,
> +                          uint8_t *extradata,
> +                          int extradata_size)
> +{
> +
> +    status = VDADecoderCreate(config_info,
> +                              NULL,
> +                              (VDADecoderOutputCallback *)vda_decoder_callback,
> +                              (void *)vda_ctx,
> +                              &vda_ctx->decoder );

Is the cast necessary?

> +int ff_vda_destroy_decoder(struct vda_context *vda_ctx)
> +{
> +    OSStatus status = kVDADecoderNoErr;
> +
> +    if (vda_ctx->decoder)
> +        status = VDADecoderDestroy(vda_ctx->decoder);
> +
> +    vda_clear_queue(vda_ctx);
> +
> +    if (vda_ctx->queue_mutex != NULL)

Same here, dropping the "!= NULL" is preferred.

> --- /dev/null
> +++ b/libavcodec/vda.h
> @@ -0,0 +1,138 @@
> +
> +#ifndef AVCODEC_VDA_H
> +#define AVCODEC_VDA_H
> +
> +#include <stdint.h>
> +
> +// emmintrin.h is unable to compile with -std=c99 -Werror=missing-prototypes
> +// http://openradar.appspot.com/8026390
> +#undef __GNUC_STDC_INLINE__
> +
> +#define Picture QuickdrawPicture
> +
> +#include <pthread.h>
> +#include <VideoDecodeAcceleration/VDADecoder.h>
> +
> +#include "avcodec.h"

The #define looks dangerous to me, we have a struct Picture, so I fear
for unintended sideeffects.

Also, can the emmintrin.h workaround be done somewhere else?  Where does
it get #included from?

> --- /dev/null
> +++ b/libavcodec/vda_internal.h
> @@ -0,0 +1,49 @@
> +
> +#ifndef AVCODEC_VDA_INTERNAL_H
> +#define AVCODEC_VDA_INTERNAL_H
> +
> +#include <CoreFoundation/CFDictionary.h>
> +#include <CoreFoundation/CFNumber.h>
> +#include <CoreFoundation/CFData.h>
> +#include <CoreFoundation/CFString.h>
> +
> +#include "h264.h"
> +#include "h264data.h"
> +#include "vda.h"
> +
> +/** Send a frame data to the hardware decoder. */
> +int ff_vda_decoder_decode(struct vda_context *vda_ctx,
> +                          uint8_t *bitstream,
> +                          int bitstream_size,
> +                          int64_t frame_pts);
> +
> +#endif /* AVCODEC_VDA_INTERNAL_H */

These #includes should be placed in the files that actually use them.

Diego


More information about the libav-devel mailing list