On Fri, Jan 11, 2013 at 08:33:20PM +0200, Rémi Denis-Courmont wrote:
> --- a/libavcodec/vdpau.c
> +++ b/libavcodec/vdpau.c
> @@ -38,6 +38,55 @@
> +int ff_vdpau_common_end_frame(AVCodecContext *avctx)
> +{
> +    MpegEncContext * const s = avctx->priv_data;
> +    struct vdpau_context *hwctx = avctx->hwaccel_context;
> +
> +    if (hwctx->bitstream_buffers_used) {
> +        VdpVideoSurface surf = 
> ff_vdpau_get_surface_id(s->current_picture_ptr);
> +
> +        hwctx->render(hwctx->decoder, surf, (void *)&hwctx->info,
> +                      hwctx->bitstream_buffers_used, 
> hwctx->bitstream_buffers);

Is the cast necessary?

> +int ff_vdpau_add_buffer(AVCodecContext *avctx,
> +                        const uint8_t *buf, uint32_t size)
> +{
> +    struct vdpau_context *hwctx = avctx->hwaccel_context;
> +    VdpBitstreamBuffer *buffers = hwctx->bitstream_buffers;
> +
> +    buffers = av_fast_realloc(buffers, &hwctx->bitstream_buffers_allocated,
> +                      (hwctx->bitstream_buffers_used  + 1) * 
> sizeof(*buffers));

Indentation is off.

> +/* Obsolete non-hwaccel VDPAU support below... */
> +
>  void ff_vdpau_h264_set_reference_frames(MpegEncContext *s)
>  {

Should that obsolete support be dropped at the next version bump?

> --- a/libavcodec/vdpau.h
> +++ b/libavcodec/vdpau.h
> @@ -52,6 +52,74 @@
>  
> +/**
> + * This structure is used to share data between the Libav library and

s/Libav/libavcodec/, Libav is the project name.

> +typedef struct vdpau_context {
> +    /**
> +     * VDPAU decoder handle
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by user
> +     */
> +    VdpDecoder decoder;
> +
> +    /**
> +     * VDPAU decoder render callback
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by the user
> +     */
> +    VdpDecoderRender *render;
> +
> +    /**
> +     * VDPAU picture information
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by libavcodec
> +     */
> +    union VdpPictureInfo info;
> +
> +    /**
> +     * Allocated size of the bitstream_buffers table.
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by libavcodec
> +     */
> +    int bitstream_buffers_allocated;
> +
> +    /**
> +     * Useful bitstream buffers in the bitstream buffers table.
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by libavcodec
> +     */
> +    int bitstream_buffers_used;
> +
> +   /**
> +     * Table of bitstream buffers.
> +     * The user is responsible for freeing this buffer using av_freep().
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by libavcodec
> +     */
> +    VdpBitstreamBuffer *bitstream_buffers;
> +} AVVDPAUContext;

I'm not sure it makes sense to mark every single member with encoding
and decoding.  VDPAU is never used for encoding and if every single
struct member is set by libavcodec, you could better mention it once
at the top.

> --- a/libavcodec/vdpau_internal.h
> +++ b/libavcodec/vdpau_internal.h
> @@ -25,8 +25,23 @@
>  
> +/** Extract VdpVideoSurface from a Picture */
> +static inline VdpVideoSurface ff_vdpau_get_surface_id(Picture *pic)
> +{
> +    return (uintptr_t)pic->f.data[3];
> +}

uintptr_t vs. VdpVideoSurface - I'm confused ...

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to