2014-06-04 10:53 GMT+02:00 Xiang, Haihao <haihao.xi...@intel.com>: > On Tue, 2014-06-03 at 18:43 +0200, Gwenole Beauchesne wrote: >> Sometimes, a dummy frame comes from the codec layer and it is used >> as a reference, per the comment in the existing code. Even though >> this looks suspicious, keep this criterion but make sure to try >> allocating the VA surface, if needed, earlier in the function that >> sanity checks the parameters for decoding the current frame. >> >> This makes it possible to fail at a much earlier time, and actually >> make it possible to return a sensible error code to the upper layer. >> >> Also fix the reference_objects[] array elements to be an exact 1:1 >> match for ReferenceFrames[] array elements, including possible but >> unlikely holes in it. The former array holds object_surface structs >> corresponding to the VA surfaces present in the ReferenceFrames[] >> array and identified by VAPictureH264.picture_id. >> >> Signed-off-by: Gwenole Beauchesne <gwenole.beauche...@intel.com> >> --- >> src/i965_decoder_utils.c | 46 >> +++++++++++++++++++++++----------------------- >> 1 file changed, 23 insertions(+), 23 deletions(-) >> >> diff --git a/src/i965_decoder_utils.c b/src/i965_decoder_utils.c >> index 9ae7083..2570be9 100644 >> --- a/src/i965_decoder_utils.c >> +++ b/src/i965_decoder_utils.c >> @@ -477,12 +477,6 @@ intel_update_avc_frame_store_index(VADriverContextP ctx, >> int slot_found; >> struct object_surface *obj_surface = >> decode_state->reference_objects[i]; >> >> - /* >> - * Sometimes a dummy frame comes from the upper layer library, >> call i965_check_alloc_surface_bo() >> - * to ake sure the store buffer is allocated for this reference >> frame >> - */ >> - avc_ensure_surface_bo(ctx, decode_state, obj_surface, >> pic_param); >> - >> slot_found = 0; >> frame_idx = -1; >> /* Find a free frame store index */ >> @@ -606,6 +600,7 @@ intel_decoder_check_avc_parameter(VADriverContextP ctx, >> { >> struct i965_driver_data *i965 = i965_driver_data(ctx); >> VAPictureParameterBufferH264 *pic_param = (VAPictureParameterBufferH264 >> *)decode_state->pic_param->buffer; >> + VAStatus va_status; >> struct object_surface *obj_surface; >> int i; >> >> @@ -629,28 +624,33 @@ intel_decoder_check_avc_parameter(VADriverContextP ctx, >> } >> } >> >> - for (i = 0; i < 16; i++) { >> - if (pic_param->ReferenceFrames[i].flags & VA_PICTURE_H264_INVALID || >> - pic_param->ReferenceFrames[i].picture_id == VA_INVALID_SURFACE) >> - break; >> - else { >> - obj_surface = SURFACE(pic_param->ReferenceFrames[i].picture_id); >> - assert(obj_surface); >> + /* Fill in the reference objects array with the actual VA surface >> + objects with 1:1 correspondance with any entry in ReferenceFrames[], >> + i.e. including "holes" for invalid entries, that are expanded >> + to NULL in the reference_objects[] array */ >> + for (i = 0; i < ARRAY_ELEMS(pic_param->ReferenceFrames); i++) { >> + const VAPictureH264 * const va_pic = &pic_param->ReferenceFrames[i]; >> >> + obj_surface = NULL; >> + if (!(va_pic->flags & VA_PICTURE_H264_INVALID) && >> + va_pic->picture_id != VA_INVALID_ID) { >> + obj_surface = SURFACE(pic_param->ReferenceFrames[i].picture_id); >> if (!obj_surface) >> - goto error; >> + return VA_STATUS_ERROR_INVALID_SURFACE; >> >> - if (!obj_surface->bo) { /* a reference frame without store >> buffer */ >> - WARN_ONCE("Invalid reference frame!!!\n"); >> - } >> - >> - decode_state->reference_objects[i] = obj_surface; >> + /* >> + * Sometimes a dummy frame comes from the upper layer >> + * library, call i965_check_alloc_surface_bo() to make >> + * sure the store buffer is allocated for this reference >> + * frame >> + */ >> + va_status = avc_ensure_surface_bo(ctx, decode_state, >> obj_surface, >> + pic_param); > > Just noticed avc_ensure_surface_bo() always allocates the buffer store > with tiled flag if necessary. It is wrong for ironlake.
Ah, OK. I thought it was only for MPEG-2 case. I will provide a fix to that. Thanks, Gwenole. > > >> + if (va_status != VA_STATUS_SUCCESS) >> + return va_status; >> } >> + decode_state->reference_objects[i] = obj_surface; >> } >> - >> - for ( ; i < 16; i++) >> - decode_state->reference_objects[i] = NULL; >> - >> return VA_STATUS_SUCCESS; >> >> error: > > _______________________________________________ Libva mailing list Libva@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libva