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
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
