[FFmpeg-devel] [PATCH][VAAPI][2/6] Add common data structures and helpers (take 9)

[Gwenole Beauchesne]

Le 12 mars 09 ? 01:27, Michael Niedermayer a ?crit :

>> +{
>> +    const struct vaapi_context *va_context = avctx->hwaccel_context;
>> +    struct vaapi_hwaccel_data_private * const p = pic- 
>> >hwaccel_data_private;
>
> the structs should be named in a way that makes it obvious which is
> "global" and which is per frame like
> hwaccel_frame/surface/..._private or so

The current names are obvious.
- vaapi_context derived from and accessed from  
AVCodecContext.hwaccel_context
- vaapi_hwaccel_data_private derived from and accessed from  
Picture.hwaccel_data_private

>> +    slice_param->slice_data_size        = size;
>> +    slice_param->slice_data_offset      = p->slice_data_size;
>> +    slice_param->slice_data_flag        = VA_SLICE_DATA_FLAG_ALL;
>
> extra useless whitespace

Oh, it's great, you are now focusing on cosmetics, so this means the  
rest is correct!

> [...]
>> +    av_freep(&p->bitplane_buffer);
>> +    av_freep(&p->slice_buf_ids);
>> +    p->n_slice_buf_ids     = 0;
>> +    p->slice_buf_ids_alloc = 0;
>> +    av_freep(&p->slice_params);
>> +    p->slice_count         = 0;
>> +    p->slice_params_alloc  = 0;
>
> you can group the av_freep() together

Yes, I surely can, but it's not desirable, I grouped them per relative  
function (inner working) not by "vehicle" (av_freep()) achieving that.  
Besides, that order turns out to also be the order of the struct  
elements.

The most logical behaviour is to reset collaterals within the same  
group. e.g. n_slice_buf_ids and slice_buf_ids_alloc with  
slice_buf_ids. The other suggestion cannot make any logical sense.  
e.g. imagine you'd want to destroy a house with a loader, would you  
start from the bottom?