[FFmpeg-devel] [PATCH]VDPAU support for MPEG-4 ASP

[Carl Eugen Hoyos]

On Saturday 05 September 2009 10:17:08 Reimar D?ffinger wrote:
> On Sat, Sep 05, 2009 at 01:08:46AM +0200, Carl Eugen Hoyos wrote:
> > Attached patch adds support for decoding MPEG-4 ASP on Nvidia's latest
> > VDPAU hardware.
>
> This brings again up the same question: why do we have an extra codec
> for these formats when the acceleration could just be selected via
> pixfmt?

How would this work when ffmpeg will support hardware accelerated decoding (I 
mean the resulting pix_fmt would then be the same for software decoding and 
hardware decoding)?

> > +// greatest common divisor
> > +static int gcd(int u, int v)
>
> av_gcd.

Thank you.

> > +        int d = gcd(s->pp_time, s->pb_time);
> > +        render->info.mpeg4.trd[0] = s->pp_time / d;
> > +        render->info.mpeg4.trb[0] = s->pb_time / d;
>
> av_reduce?

Sorry, I don't understand how it would simplify the code?

> > +        render->info.mpeg4.trd[1] = (s->time - s->pb_time + s->pp_time
> > +                                       + (s->pb_time >> 1))/s->pb_time
> > +               - (s->time - s->pb_time + (s->pb_time >> 1))/s->pb_time;
> > +        render->info.mpeg4.trb[1] = (s->time + (s->pb_time >>
> > 1))/s->pb_time +                     - (s->time - s->pb_time +
> > (s->pb_time >> 1))/s->pb_time;
>
> Is it really necessary to round and divide twice?
> At least this needs an explaining comment.

Nvidia's original version looks like this:
        cur_time = s->time;
        fwd_time = cur_time - s->pb_time;
        bwd_time = fwd_time +  s->pp_time;
        frame_time = cur_time - fwd_time;

        trd[0] =  s->pp_time;
        trb[0] =  s->pb_time;
        d = gcd(trb[0], trd[0]);
        trb[0] /= d;
        trd[0] /= d;

        trd[1] = (bwd_time + (frame_time >> 1))/frame_time -
                              (fwd_time + (frame_time >> 1))/frame_time;
        trb[1] = (cur_time + (frame_time >> 1))/frame_time -
                              (fwd_time + (frame_time >> 1))/frame_time;

I hoped my version is a little simpler.

> > +    render->info.mpeg4.vop_coding_type                   = s->pict_type
> > - FF_I_TYPE;
>
> Hm, not sure if that doesn't assume a bit too much.

Agree.

New patch attached, Carl Eugen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patchvdpauC3.diff
Type: text/x-diff
Size: 9411 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090905/d0281a9d/attachment.diff>