On Thu, 4 Sep 2014 16:11:15 +0300 Rémi Denis-Courmont <[email protected]> wrote:
> From: Rémi Denis-Courmont <[email protected]> > > Using the not so new init and uninit callbacks, avcodec can now take > care of creating and destroying the VDPAU decoder instance. > > The application is still responsible for creating the VDPAU device > and allocating video surfaces. But it wull no longer needs to care > about any codec-specific aspects. > > This deprecates av_vdpau_alloc_context() and introduces a new > av_vdpau_create_context() accepting the VDPAU device (VdpDevice) and > VDPAU driver (VdpGetProcAddress) as explicit parameter. > > Note that av_vdpau_get_profile() is also deprecated since it becomes > totally useless for external use. > > [Discuss: It could be: > int av_vdpau_init_context(AVCodecContext *, VdpDevice, > VdpGetProcAddress *); > instead (that would enable capability checks). In that case, there > should probably be a deinitializer for the sake of symmetry: > void av_vdpau_deinit_context(AVCodecContext *); > instead of the current reliance on av_freep().] > --- > libavcodec/vdpau.c | 84 > ++++++++++++++++++++++++++++++++++++++++++++- > libavcodec/vdpau.h | 27 +++++++++++++-- > libavcodec/vdpau_internal.h | 16 +++++++++ > libavcodec/version.h | 3 ++ > 4 files changed, 127 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/vdpau.c b/libavcodec/vdpau.c > index 5406874..c285897 100644 > --- a/libavcodec/vdpau.c > +++ b/libavcodec/vdpau.c > @@ -22,7 +22,9 @@ > */ > > #include <limits.h> > +#include "libavutil/avassert.h" > #include "avcodec.h" > +#include "internal.h" > #include "h264.h" > #include "vc1.h" > > @@ -38,6 +40,67 @@ > * @{ > */ > > +static VdpStatus vdp_enosys(/* Unspecified, not void - leave empty */) > +{ > + return VDP_STATUS_NO_IMPLEMENTATION; > +} > + > +static void *ff_vdpau_get_proc_address(AVCodecContext *avctx, VdpFuncId id) > +{ > + AVVDPAUContext *hwctx = avctx->hwaccel_context; > + void *ret; > + > + av_assert0(hwctx->device != VDP_INVALID_HANDLE); > + > + if (hwctx->get_proc_address(hwctx->device, id, &ret) != VDP_STATUS_OK) > + ret = vdp_enosys; Isn't this very questionable? IMO initialization should just fail if a required function is missing. But if a C language lawyer can confirm that this use of vdp_enosys is indeed legal, why not (although I have my doubt). > + return ret; > +} > + > +int ff_vdpau_common_init(AVCodecContext *avctx, VdpDecoderProfile profile, > + uint32_t level, uint32_t surfaces) > +{ > + AVVDPAUContext *hwctx = avctx->hwaccel_context; > + struct vdpau_context *vdctx = avctx->internal->hwaccel_priv_data; > + VdpDecoderCreate *create; > + VdpStatus status; > + > +#if FF_API_DECODER_VDPAU > + if (hwctx->device == VDP_INVALID_HANDLE) { > + vdctx->decoder = hwctx->decoder; > + vdctx->render = hwctx->render; > + return 0; /* Decoder created by user */ > + } > +#endif > + > + create = ff_vdpau_get_proc_address(avctx, VDP_FUNC_ID_DECODER_CREATE); > + > + status = create(hwctx->device, profile, avctx->width, avctx->height, > + surfaces, &vdctx->decoder); > + if (status != VDP_STATUS_OK) > + return AVERROR(ENODEV); /* TODO: VDPAU to AV error codes */ > + > + vdctx->render = ff_vdpau_get_proc_address(avctx, > + VDP_FUNC_ID_DECODER_RENDER); > + return 0; > +} > + > +int ff_vdpau_common_uninit(AVCodecContext *avctx) > +{ > + struct vdpau_context *vdctx = avctx->internal->hwaccel_priv_data; > + VdpDecoderDestroy *destroy; > +#if FF_API_DECODER_VDPAU > + AVVDPAUContext *hwctx = avctx->hwaccel_context; > + > + if (hwctx->device == VDP_INVALID_HANDLE) > + return 0; /* Decoder to be destroyed by user */ > +#endif > + > + destroy = ff_vdpau_get_proc_address(avctx, VDP_FUNC_ID_DECODER_DESTROY); > + destroy(vdctx->decoder); > + return 0; > +} > + > int ff_vdpau_common_start_frame(struct vdpau_picture_context *pic_ctx, > av_unused const uint8_t *buffer, > av_unused uint32_t size) > @@ -88,6 +151,7 @@ int ff_vdpau_add_buffer(struct vdpau_picture_context > *pic_ctx, > return 0; > } > > +#if FF_API_DECODER_VDPAU > int av_vdpau_get_profile(AVCodecContext *avctx, VdpDecoderProfile *profile) > { > #define PROFILE(prof) \ > @@ -133,7 +197,25 @@ do { \ > > AVVDPAUContext *av_vdpau_alloc_context(void) > { > - return av_mallocz(sizeof(AVVDPAUContext)); > + AVVDPAUContext *hwctx = av_mallocz(sizeof(AVVDPAUContext)); > + if (!hwctx) > + return NULL; > + > + hwctx->device = VDP_INVALID_HANDLE; > + return hwctx; > +} > +#endif > + > +AVVDPAUContext *av_vdpau_create_context(VdpDevice device, > + VdpGetProcAddress *get_proc_address) > +{ > + AVVDPAUContext *hwctx = av_mallocz(sizeof(AVVDPAUContext)); > + if (!hwctx) > + return NULL; > + > + hwctx->device = device; > + hwctx->get_proc_address = get_proc_address; > + return hwctx; > } > > /* @}*/ > diff --git a/libavcodec/vdpau.h b/libavcodec/vdpau.h > index 75cb1bf..4a99834 100644 > --- a/libavcodec/vdpau.h > +++ b/libavcodec/vdpau.h > @@ -80,11 +80,13 @@ union AVVDPAUPictureInfo { > * AVVDPAUContext. > */ > typedef struct AVVDPAUContext { > +#if FF_API_DECODER_VDPAU > /** > * VDPAU decoder handle > * > * Set by user. > */ > + attribute_deprecated > VdpDecoder decoder; > > /** > @@ -92,8 +94,9 @@ typedef struct AVVDPAUContext { > * > * Set by the user. > */ > + attribute_deprecated > VdpDecoderRender *render; > - > +#endif > #if FF_API_BUFS_VDPAU > /** > * VDPAU picture information > @@ -128,14 +131,32 @@ typedef struct AVVDPAUContext { > attribute_deprecated > VdpBitstreamBuffer *bitstream_buffers; > #endif > + /***************************************************************** > + * No fields below this line are part of the public API. They > + * may not be used outside of libavcodec and can be changed and > + * removed at will. > + * No new public fields will be added to this structure. > + ***************************************************************** > + */ > + VdpDevice device; > + VdpGetProcAddress *get_proc_address; > } AVVDPAUContext; > > /** > + * Create an AVVDPAUContext. > + * > + * @return Newly-allocated AVVDPAUContext or NULL on failure. > + */ > +AVVDPAUContext *av_vdpau_create_context(VdpDevice device, > + VdpGetProcAddress *get_proc_address); > + > +#if FF_API_DECODER_VDPAU > +/** > * Allocate an AVVDPAUContext. > * > * @return Newly-allocated AVVDPAUContext or NULL on failure. > */ > -AVVDPAUContext *av_vdpau_alloc_context(void); > +attribute_deprecated AVVDPAUContext *av_vdpau_alloc_context(void); > > /** > * Get a decoder profile that should be used for initializing a VDPAU > decoder. > @@ -148,7 +169,9 @@ AVVDPAUContext *av_vdpau_alloc_context(void); > * > * @return 0 on success (non-negative), a negative AVERROR on failure. > */ > +attribute_deprecated > int av_vdpau_get_profile(AVCodecContext *avctx, VdpDecoderProfile *profile); > +#endif > > #if FF_API_CAP_VDPAU > /** @brief The videoSurface is used for rendering. */ > diff --git a/libavcodec/vdpau_internal.h b/libavcodec/vdpau_internal.h > index 2443e0a..379407e 100644 > --- a/libavcodec/vdpau_internal.h > +++ b/libavcodec/vdpau_internal.h > @@ -48,6 +48,18 @@ union AVVDPAUPictureInfo { > #include "vdpau.h" > #endif > > +struct vdpau_context { > + /** > + * VDPAU decoder handle > + */ > + VdpDecoder decoder; > + > + /** > + * VDPAU decoder render callback > + */ > + VdpDecoderRender *render; > +}; > + > struct vdpau_picture_context { > /** > * VDPAU picture information. > @@ -70,6 +82,10 @@ struct vdpau_picture_context { > VdpBitstreamBuffer *bitstream_buffers; > }; > > +int ff_vdpau_common_init(AVCodecContext *avctx, VdpDecoderProfile profile, > + uint32_t level, uint32_t surfaces); > +int ff_vdpau_common_uninit(AVCodecContext *avctx); > + > int ff_vdpau_common_start_frame(struct vdpau_picture_context *pic, > const uint8_t *buffer, uint32_t size); > int ff_vdpau_mpeg_end_frame(AVCodecContext *avctx); > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 8cc2fb0..41c7fcb 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -153,5 +153,8 @@ > #ifndef FF_API_AFD > #define FF_API_AFD (LIBAVCODEC_VERSION_MAJOR < 57) > #endif > +#ifndef FF_API_DECODER_VDPAU > +#define FF_API_DECODER_VDPAU (LIBAVCODEC_VERSION_MAJOR < 57) > +#endif > > #endif /* AVCODEC_VERSION_H */ The idea seems generally ok (and making libavcodec handle more aspects of hw decoding is welcome). But I have some doubts whether it will still be possible for me to handle preemption as well as I could, and I wouldn't appreciate if a small incremental improvement breaks all of my code. Also, think of the API users: do they have to update their vdpau code every month now? WTF? A solution to this might be not deprecating the "old" API. Then downstreams can wait it out until the API finally becomes sane after a bunch of more incremental improvements. This is true for the other hwaccels as well (since I always hear of hwaccel 1.x...). _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
