[Libva] [PATCH v3 intel-driver 4/9] decoder: h264: fix submission of AVC_REF_IDX_STATE command.

[Gwenole Beauchesne]

2014-06-04 3:20 GMT+02:00 Zhao, Yakui <yakui.zhao at intel.com>:
> On Tue, 2014-06-03 at 10:43 -0600, Gwenole Beauchesne wrote:
>> If the RefPicListX[] entry has no valid picture_id associated to it,
>> then set the resulting state to 0xff. If that entry has no surface
>> buffer storage either, then compose a valid state that maps to the
>> first item in the reference frames list, as mandated by the PRM.
>>
>> Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne at intel.com>
>> ---
>>  src/i965_decoder_utils.c |   35 +++++++++++++++++++----------------
>>  1 file changed, 19 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/i965_decoder_utils.c b/src/i965_decoder_utils.c
>> index c686235..e0593ea 100644
>> --- a/src/i965_decoder_utils.c
>> +++ b/src/i965_decoder_utils.c
>> @@ -334,34 +334,37 @@ gen5_fill_avc_ref_idx_state(
>>      const GenFrameStore frame_store[MAX_GEN_REFERENCE_FRAMES]
>>  )
>>  {
>> -    unsigned int i, n, frame_idx;
>> -    int found;
>> +    int i, j;
>> +    bool found;
>>
>> -    for (i = 0, n = 0; i < ref_list_count; i++) {
>> +    for (i = 0; i < ref_list_count; i++) {
>>          const VAPictureH264 * const va_pic = &ref_list[i];
>>
>> -        if (va_pic->flags & VA_PICTURE_H264_INVALID)
>> +        if ((va_pic->flags & VA_PICTURE_H264_INVALID) ||
>> +            va_pic->picture_id == VA_INVALID_ID) {
>> +            state[i] = 0xff;
>>              continue;
>> +        }
>>
>> -        found = 0;
>> -        for (frame_idx = 0; frame_idx < MAX_GEN_REFERENCE_FRAMES; frame_idx++) {
>> -            const GenFrameStore * const fs = &frame_store[frame_idx];
>> -            if (fs->surface_id != VA_INVALID_ID &&
>> -                fs->surface_id == va_pic->picture_id) {
>> -                found = 1;
>> +        found = false;
>> +        for (j = 0; j < MAX_GEN_REFERENCE_FRAMES; j++) {
>> +            if ((found = frame_store[j].surface_id == va_pic->picture_id))
>
> It had better use the following style to improve the readability.
>      if (frame_store[j].surface_id == va_pic->picture_id) {
>           found = true;
>           break;
>      }

I actually planned to just drop the found variable. That's just
useless in this context. i.e. replace the if (found) with if (j !=
MAX_GEN_REFERENCE_FRAMES). WDYT?

For Patch3, I am OK to make the split for readability.

Regards,
Gwenole.

>
>>                  break;
>> -            }
>>          }
>>
>>          if (found) {
>> -            state[n++] = get_ref_idx_state_1(va_pic, frame_idx);
>> -        } else {
>> -            WARN_ONCE("Invalid Slice reference frame list !!!. It is not included in DPB \n");
>> +            const GenFrameStore * const fs = &frame_store[j];
>> +            assert(fs->frame_store_id == j); // Current architecture/assumption
>> +            state[i] = get_ref_idx_state_1(va_pic, fs->frame_store_id);
>> +        }
>> +        else {
>> +            WARN_ONCE("Invalid RefPicListX[] entry!!! It is not included in DPB\n");
>> +            state[i] = get_ref_idx_state_1(va_pic, 0) | 0x80;
>>          }
>>      }
>>
>> -    for (; n < 32; n++)
>> -        state[n] = 0xff;
>> +    for (; i < 32; i++)
>> +        state[i] = 0xff;
>>  }
>>
>>  /* Emit Reference List Entries (Gen6+: SNB, IVB) */
>
>