[FFmpeg-devel] [PATCH] AVHWAccel infrastructure v2 (take 3)

Gwenole Beauchesne gbeauchesne
Mon Jan 4 14:13:35 CET 2010


On Sun, 15 Nov 2009, Michael Niedermayer wrote:

>>> [...]
>>>> @@ -248,9 +249,29 @@ int avcodec_default_get_buffer(AVCodecContext *s,
>>>> AVFrame *pic){
>>>>          }
>>>>      }
>>>>
>>>> +    if (s->hwaccel &&
>>>> +        !ff_find_hwaccel_attribute(s, HWACCEL_ATTR_GET_PIXELS,
>>>> &alloc_pixels))
>>>> +        alloc_pixels = 0; /* SW surfaces are not needed */
>>>> +
>>>
>>> We have an existing API to let the user application choose the output
>>> format (get_format())
>>> your patch does this instead without explaining why the existing API is
>>> not used  ...
>>
>> get_format() deals with pixel formats. The aim was to get rid of HW accel
>> pixel formats because some other HW accelerator will need the pixel format
>> set to e.g. NV12 and we'd still need a way to identify this HW accelerator.
>
> If avcodec_decode_video() outputs NV12, get_format() would select NV12 and
> the user application would not even need to know if that was sw or hw decoded.
> if it does not output a normal pixel format then the userapp has to support
> that pixel format that is support the hw decoding counterpart
>
> thus really, a hardware accelerator that returns NV12 needs no changes at all
> not even any of the previous accelerator API additions
>
> of course a user app should be able to force sw or hw too by some means ...

Yes, this what I wanted to do with the hwaccel attributes. Even a way to 
force a specific hw accelerator if several ones are installed on the 
system.

>> Besides, with this iteration of hwaccel, the pixel format for HW will have
>> the same meaning as for SW: what pixel format the user actually wants for
>> real. e.g. if he wants to get the HW decoded pixels back, pixfmt will
>> identify this format, which will be the "standard" meaning, i.e. the same
>> as in SW mode.
>>
>>>>      if(buf->base[0]){
>>>>          pic->age= *picture_number - buf->last_pic_num;
>>>>          buf->last_pic_num= *picture_number;
>>>> +    }else if(s->hwaccel && !alloc_pixels){
>>>> +        pic->age= 256*256*256*64;
>>>> +        buf->last_pic_num= -256*256*256*64;
>>>> +        memset(buf->linesize, 0, sizeof(buf->linesize));
>>>> +        memset(buf->base, 0, sizeof(buf->base));
>>>> +        memset(buf->data, 0, sizeof(buf->data));
>>>> +        /*
>>>> +         * Allocate a dummy buffer so that we don't have to worry
>>>> +         * about hwaccel checks afterwards in this get_buffer() /
>>>> +         * release_buffer() machinery.
>>>> +         */
>>>> +        buf->base[0] = av_malloc(16);
>>>> +        buf->data[0] = buf->base[0];
>>>> +        buf->width   = s->width;
>>>> +        buf->height  = s->height;
>>>> +        buf->pix_fmt = s->pix_fmt;
>>>
>>> i would say base/data either contain pixels or could contain some struct
>>> representing a surface of the hwaccelerator
>>> why is neither of these the case?
>>
>> data[3] does contain that struct for the HW accelerator (vaapi_context,
>> vdpau_render_state). However, for the default mode of operations, that is
>> user wants to use hwaccel for decode+display, we don't have to allocate
>> data[0-2] pixels. This saves memory in that case. However, making it NULL
>> is possible but would require further checks in
>> get_buffer()/release_buffer().
>
> could it be that this problem arises from using [3] instead of [0] for the
> struct?
> (note i dont know if using [0] would not lead to more mess elsewhere ...)

I want to keep data[0-2] as the actual pixel data, when needed, and make 
data[3] hwaccel specific data. The hack presented in this hunk is only 
meaningful to save memory. Of course, we could keep all of them allocated 
but I doubt this is desired.

Regards,
Gwenole.



More information about the ffmpeg-devel mailing list