[FFmpeg-devel] [PATCH]VDPAU patch for h264 decoding, round 5

[Michael Niedermayer]

On Sat, Jan 03, 2009 at 12:52:09AM +0100, Carl Eugen Hoyos wrote:
> Hi!
>
> I tried to follow Diegos suggestions.

[...]
> @@ -2188,6 +2188,8 @@
>  
>      if(avctx->codec_id == CODEC_ID_SVQ3)
>          avctx->pix_fmt= PIX_FMT_YUVJ420P;
> +    else if(avctx->vdpau_acceleration)
> +        avctx->pix_fmt = PIX_FMT_VDPAU_H264;
>      else
>          avctx->pix_fmt= PIX_FMT_YUV420P;
>  

the = can be vertically aligned



> @@ -7289,6 +7291,8 @@
>      H264Context *hx;
>      int i;
>  
> +    if(avctx->vdpau_acceleration)
> +        return;
>      if(context_count == 1) {
>          decode_slice(avctx, &h);
>      } else {
> @@ -7416,8 +7420,20 @@
>                 && (avctx->skip_frame < AVDISCARD_NONREF || hx->nal_ref_idc)
>                 && (avctx->skip_frame < AVDISCARD_BIDIR  || hx->slice_type_nos!=FF_B_TYPE)
>                 && (avctx->skip_frame < AVDISCARD_NONKEY || hx->slice_type_nos==FF_I_TYPE)
> -               && avctx->skip_frame < AVDISCARD_ALL)
> +               && avctx->skip_frame < AVDISCARD_ALL) {
> +                if (ENABLE_H264_VDPAU_DECODER && avctx->vdpau_acceleration) {

> +                    if(h->is_avc) {
> +                        static const uint8_t start_code[] = {0x00, 0x00, 0x01};
> +                        ff_VDPAU_h264_add_data_chunk(h, start_code, sizeof(start_code));
> +                        ff_VDPAU_h264_add_data_chunk(h, &buf[buf_index - consumed], consumed );
> +                    }
> +                    else
> +                        // +/-3: Add back 00 00 01 to start of data
> +                        ff_VDPAU_h264_add_data_chunk(h, &buf[buf_index - consumed - 3], consumed + 3);

cant this always be
ff_VDPAU_h264_add_data_chunk(h, start_code, sizeof(start_code));
ff_VDPAU_h264_add_data_chunk(h, &buf[buf_index - consumed], consumed );
?

besides

}
else

is inconsistant style wise with the rest of h264.c


[...]
> +static void VDPAU_ensure_has_buffers(vdpau_render_state * render,
> +        int need_entries)
> +{
> +    int new_allocated;
> +
> +    if (render->bitstreamBuffersAlloced >= need_entries)
> +        return;
> +

> +    if (!render->bitstreamBuffersAlloced || !render->bitstreamBuffers)
> +        new_allocated = 4;
> +    else
> +        new_allocated = render->bitstreamBuffersAlloced * 2;

if(render->bitstreamBuffersAlloced && render->bitstreamBuffers)
    new_allocated = render->bitstreamBuffersAlloced * 2;
else
    new_allocated = 4;


> +
> +    render->bitstreamBuffers = av_realloc(
> +        render->bitstreamBuffers,
> +        new_allocated * sizeof(render->bitstreamBuffers[0])
> +    );
> +    render->bitstreamBuffersAlloced = new_allocated;
> +}

maybe the whole could be simplified by using av_fast_realloc()



> +
> +static void VDPAU_h264_set_reference_frames(H264Context *h)
> +{
> +    MpegEncContext * s = &h->s;
> +    vdpau_render_state * render, * render_ref;
> +    VdpReferenceFrameH264 * rf, * rf2;
> +    Picture * pic;
> +    int i, list;

> +    uint16t pic_frame_idx;

is there a special reason why this is exactly 16bit ?



> +
> +    render = (vdpau_render_state*)s->current_picture_ptr->data[2];

> +    assert(render != NULL);

superflous "!= NULL"


> +
> +    rf = &render->info.h264.referenceFrames[0];
> +#define H264_RF_COUNT FF_ARRAY_ELEMS(render->info.h264.referenceFrames)
> +
> +    for (list = 0; list < 2; ++list) {
> +        Picture **lp = list ? h->long_ref : h->short_ref;
> +        int ls = list ? h->long_ref_count : h->short_ref_count;
> +
> +        for (i = 0; i < ls; ++i) {
> +            pic = lp[i];
> +            if (!pic || !pic->reference)
> +                continue;
> +            pic_frame_idx = pic->long_ref ? pic->pic_id : pic->frame_num;
> +
> +            render_ref = (vdpau_render_state*)pic->data[2];
> +            assert(render_ref != NULL);
> +
> +            rf2 = &render->info.h264.referenceFrames[0];
> +            while (rf2 != rf) {
> +                if (
> +                    (rf2->surface == render_ref->surface)
> +                    && (rf2->is_long_term == pic->long_ref)
> +                    && (rf2->frame_idx == pic_frame_idx)
> +                )
> +                    break;
> +                ++rf2;
> +            }
> +            if (rf2 != rf) {

> +                rf2->top_is_reference |= (pic->reference & PICT_TOP_FIELD) ? VDP_TRUE : VDP_FALSE;
> +                rf2->bottom_is_reference |= (pic->reference & PICT_BOTTOM_FIELD) ? VDP_TRUE : VDP_FALSE;

vertical align

[...]
> +/** 
> + * \brief This structure is used as a CALL-BACK between the ffmpeg 
> + * decoder (vd_) and presentation (vo_) module.
> + * This is used for defining a video-frame containing surface,
> + * picture-parameter, bitstream informations etc which are passed 
> + * between ffmpeg decoder and its clients.
> + */
> +typedef struct{

> +    int magic;

unused


> +
> +    VdpVideoSurface surface; /** used as rendered surface, never changed.*/
> +

> +    int state; /** Holds MP_VDPAU_STATE_* values */

var1 ///< describe var1
var2 /// describes var3
var3

similarly for /** and /**<
IIRC


[...]

> +void ff_VDPAU_h264_add_data_chunk(H264Context *h, const uint8_t *buf,
> +        int buf_size);
> +void ff_VDPAU_h264_picture_complete(H264Context *h);

These should be doxcumented as well


[...]
> @@ -2298,6 +2305,13 @@
>       * - decoding: unused.
>       */
>      float rc_min_vbv_overflow_use;
> +
> +    /**
> +     * VDPAU Acceleration
> +     * - encoding: forbidden
> +     * - decoding: set by decoder
> +     */
> +    int vdpau_acceleration;
>  } AVCodecContext;
>  
>  /**

Has this any sense?
i mean the decoder is selected by -vcodec h264_vdpau not by setting this.
and a application can check the used codec type or even pix_fmt


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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20090103/1c9afe14/attachment.pgp>