[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