[Libva] [PATCH intel-driver 1/4] decoder: h264: don't deallocate surface storage of older frames.

[Gwenole Beauchesne]

Hi,

2014-05-12 2:55 GMT+02:00 Zhao Yakui <yakui.zhao at intel.com>:
> On Fri, 2014-05-09 at 08:34 -0600, Gwenole Beauchesne wrote:
>> Drop the optimization whereby surfaces that are no longer marked as
>> reference and that were already displayed are to be destroyed. This
>> is wrong mainly for two reasons:
>>
>> 1. The surface was displayed... once but it may still be needed for
>>    subsequent operations like displaying it again, using it for a
>>    transcode pipeline (encode) for instance, etc.
>>
>> 2. The new set of ReferenceFrames[] correspond to the active set of
>>    reference frames used for decoding the current slice. In presence
>>    of Multiview Coding (MVC), that could correspond to the current
>>    view, in view order index, but the surface may still be needed
>>    for decoding the next view with the same view_id, while also
>>    decoding other views with another set of reference frames for them.
>
> Hi, Gwenole
>
>     Thanks for working on the h264 reference frame list. And I have
> some concerns about the patch set:
>     1. When will the unused reference surface be destroyed?

As it ought to be: at vaDestroySurfaces() time.

>     2. How to find one free slot in decode_state->reference_surfaces
> array to store the corresponding reference frame?

It's the same algorithm. :)
At this time, the reference_surfaces[] contents exactly matches
ReferenceFrames[], hole for hole, if there is any. The former holds
object_surface, the latter holds the associated VAPictureH264. i.e.
pointer to object_surface [i] is really the reference_surface[i] that
matches VAPictureH264 at ReferenceFrames[i]. Previously, there could
be a mismatch between both.

Regards,
Gwenole.

>> ---
>>  src/i965_decoder_utils.c | 13 -------------
>>  1 file changed, 13 deletions(-)
>>
>> diff --git a/src/i965_decoder_utils.c b/src/i965_decoder_utils.c
>> index 2533381..9d537ff 100644
>> --- a/src/i965_decoder_utils.c
>> +++ b/src/i965_decoder_utils.c
>> @@ -382,19 +382,6 @@ intel_update_avc_frame_store_index(VADriverContextP ctx,
>>
>>          /* remove it from the internal DPB */
>>          if (!found) {
>> -            struct object_surface *obj_surface = frame_store[i].obj_surface;
>> -
>> -            obj_surface->flags &= ~SURFACE_REFERENCED;
>> -
>> -            if ((obj_surface->flags & SURFACE_ALL_MASK) == SURFACE_DISPLAYED) {
>> -                dri_bo_unreference(obj_surface->bo);
>> -                obj_surface->bo = NULL;
>> -                obj_surface->flags &= ~SURFACE_REF_DIS_MASK;
>> -            }
>> -
>> -            if (obj_surface->free_private_data)
>> -                obj_surface->free_private_data(&obj_surface->private_data);
>> -
>>              frame_store[i].surface_id = VA_INVALID_ID;
>>              frame_store[i].frame_store_id = -1;
>>              frame_store[i].obj_surface = NULL;
>
>