[FFmpeg-devel] [PATCH] vc1.c: add support for HWAccel (take 2)

[Gwenole Beauchesne]

On Wed, 18 Mar 2009, Kostya wrote:

> On Wed, Mar 18, 2009 at 06:06:31PM +0100, Gwenole Beauchesne wrote:
>> Hi,
>>
>> On Tue, 10 Mar 2009, Kostya wrote:
>>
>>>> @@ -4277,6 +4279,15 @@ static int vc1_decode_frame(AVCodecContext *avctx,
>>>>     s->me.qpel_put= s->dsp.put_qpel_pixels_tab;
>>>>     s->me.qpel_avg= s->dsp.avg_qpel_pixels_tab;
>>>>
>>>> +    if (avctx->hwaccel) {
>>>> +        if (avctx->hwaccel->start_frame(avctx, buf, buf_size) < 0)
>>>> +            return -1;
>>>> +        if (avctx->hwaccel->decode_slice(avctx, buf_start, (buf +
>>>> buf_size) - buf_start) < 0)
>>>> +            return -1;
>>>> +        if (avctx->hwaccel->end_frame(avctx) < 0)
>>>> +            return -1;
>>>> +    }
>>>> +    else
>>>
>>> <don't forget proper indent here>
>>
>> Well, that was an "optimization" to get a smaller patch (only '+') and
>> reduce the next one when removing VDPAU with only '-'s instead. ;-)
>> Anyway, reindented correctly as you can see in the attachment.
>
> Actually I don't like
>    }
>    else
> instead of
>    } else

Hey, please tell me exactly what you want. I had a quick glance at vc1.c 
and I see:

if ()
{
}
else
{
}

or

if(){
}else{
}

or

if () {
} else {
}

OK for sure for the latter?

>> (ii) you
>> can't swear either that all VA API bits can get in by the end of the week,
>
> I don't care about them but since you are the man behind AVHWAccel, why not
> convert existing instances to AVHWAccel?

I do maintain a separate tree for that. What I meant was inclusion of this 
shall not depend strictly on future integration of the other bits. 
Considering how long it takes for VA API to get in, I bet this will take 
at least as long for VDPAU...

> Also that does not mean you have to produce such patch immediately, I just
> want to ensure it won't be forgotten.

Things need to get in in-order. The initial deal was to factorize for VA 
API. Then, the VDPAU bits can be merged. Please don't mix the tasks.

> [...]
>> @@ -4277,8 +4279,16 @@ static int vc1_decode_frame(AVCodecContext *avctx,
>>      s->me.qpel_put= s->dsp.put_qpel_pixels_tab;
>>      s->me.qpel_avg= s->dsp.avg_qpel_pixels_tab;
>>
>> -    if ((CONFIG_VC1_VDPAU_DECODER || CONFIG_WMV3_VDPAU_DECODER)
>> -        &&s->avctx->codec->capabilities&CODEC_CAP_HWACCEL_VDPAU)
>> +    if (avctx->hwaccel) {
>> +        if (avctx->hwaccel->start_frame(avctx, buf, buf_size) < 0)
>> +            return -1;
>> +        if (avctx->hwaccel->decode_slice(avctx, buf_start, (buf + buf_size) - buf_start) < 0)
>> +            return -1;
>> +        if (avctx->hwaccel->end_frame(avctx) < 0)
>> +            return -1;
>> +    }
>> +    else if ((CONFIG_VC1_VDPAU_DECODER || CONFIG_WMV3_VDPAU_DECODER)
>> +             &&s->avctx->codec->capabilities&CODEC_CAP_HWACCEL_VDPAU)
>>          ff_vdpau_vc1_decode_picture(s, buf_start, (buf + buf_size) - buf_start);
>>      else {
>>          ff_er_frame_start(s);
>
> No need to swap conditions here.

What do you mean? The conditions are still in the same order: CONFIG_* 
then hw caps.