[Libva] [PATCH] [Intel-driver] Decoding:Add more check of H264 slice param to avoid GPU hang caused by the incorrect parameter

Zhao, Yakui yakui.zhao at intel.com
Tue Jul 22 19:45:44 PDT 2014


On Tue, 2014-07-22 at 20:20 -0600, Xiang, Haihao wrote:
> On Wed, 2014-07-23 at 10:10 +0800, Zhao, Yakui wrote:
> > From: Zhao Yakui <yakui.zhao at intel.com>
> > 
> > This is to fix the GPU hang in https://bugs.freedesktop.org/show_bug.cgi?id=76363
> > 
> > Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
> > Tested-by: ValdikSS <iam at valdikss.org.ru>
> > ---
> >  src/gen6_mfd.c  |   18 +++++++++++++++++-
> >  src/gen75_mfd.c |   16 ++++++++++++++++
> >  src/gen7_mfd.c  |   16 ++++++++++++++++
> >  src/gen8_mfd.c  |   17 +++++++++++++++++
> 
> Maybe ILK also needs such check for H.264 decoding.
> 
> >  4 files changed, 66 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/gen6_mfd.c b/src/gen6_mfd.c
> > index b3a8ef4..18c6baa 100755
> > --- a/src/gen6_mfd.c
> > +++ b/src/gen6_mfd.c
> > @@ -899,6 +899,7 @@ gen6_mfd_avc_decode_picture(VADriverContextP ctx,
> >      VASliceParameterBufferH264 *slice_param, *next_slice_param, *next_slice_group_param;
> >      dri_bo *slice_data_bo;
> >      int i, j;
> > +    int incorrect_slice;
> >  
> >      assert(decode_state->pic_param && decode_state->pic_param->buffer);
> >      pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
> > @@ -913,6 +914,8 @@ gen6_mfd_avc_decode_picture(VADriverContextP ctx,
> >      gen6_mfd_avc_img_state(ctx, decode_state, gen6_mfd_context);
> >      gen6_mfd_avc_qm_state(ctx, decode_state, gen6_mfd_context);
> >  
> > +    incorrect_slice = 0;
> > +
> >      for (j = 0; j < decode_state->num_slice_params; j++) {
> >          assert(decode_state->slice_params && decode_state->slice_params[j]->buffer);
> >          slice_param = (VASliceParameterBufferH264 *)decode_state->slice_params[j]->buffer;
> > @@ -941,15 +944,28 @@ gen6_mfd_avc_decode_picture(VADriverContextP ctx,
> >              else
> >                  next_slice_param = next_slice_group_param;
> >  
> > +            if (next_slice_param != NULL) {
> > +                /* If the mb position of next_slice is less than or equal to the current slice,
> > +                 * ignore the following sequence.
> > +                 */
> 
> 
> Why not just ignore the illegal slice ? It is possible there are some
> legal slices after the illegal slice. We can add a function to find the
> next slice and this function can be shared by GENx 

If it only ignores the illegal slice and continue the following slice,
it needs to redesign the loop framework. 

In fact as the incorrect parameter is passed by the upper, I will use
another mechanism that only return the FAILURE to upper instead of
continuing the current frame so that the user-middle knows/fixes the
error.

OK. I will update the patch.

> 
> 
> > +                if (next_slice_param->first_mb_in_slice <= slice_param->first_mb_in_slice) {
> > +                    next_slice_param = NULL;
> > +                    incorrect_slice = 1;
> > +                }
> > +            }
> >              gen6_mfd_avc_directmode_state(ctx, decode_state, pic_param, slice_param, gen6_mfd_context);
> >              gen6_mfd_avc_slice_state(ctx, pic_param, slice_param, next_slice_param, gen6_mfd_context);
> >              gen6_mfd_avc_ref_idx_state(ctx, pic_param, slice_param, gen6_mfd_context);
> >              gen6_mfd_avc_weightoffset_state(ctx, pic_param, slice_param, gen6_mfd_context);
> >              gen6_mfd_avc_bsd_object(ctx, pic_param, slice_param, slice_data_bo, gen6_mfd_context);
> >              slice_param++;
> > +
> > +            if (incorrect_slice)
> > +                goto slice_param_done;
> >          }
> >      }
> > -    
> > +
> > +slice_param_done:
> >      gen6_mfd_avc_phantom_slice_last(ctx, pic_param, gen6_mfd_context);
> >      intel_batchbuffer_end_atomic(batch);
> >      intel_batchbuffer_flush(batch);
> > diff --git a/src/gen75_mfd.c b/src/gen75_mfd.c
> > index b14db61..9a92f5c 100644
> > --- a/src/gen75_mfd.c
> > +++ b/src/gen75_mfd.c
> > @@ -1119,6 +1119,7 @@ gen75_mfd_avc_decode_picture(VADriverContextP ctx,
> >      VASliceParameterBufferH264 *slice_param, *next_slice_param, *next_slice_group_param;
> >      dri_bo *slice_data_bo;
> >      int i, j;
> > +    int incorrect_slice;
> >  
> >      assert(decode_state->pic_param && decode_state->pic_param->buffer);
> >      pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
> > @@ -1134,6 +1135,7 @@ gen75_mfd_avc_decode_picture(VADriverContextP ctx,
> >      gen75_mfd_avc_picid_state(ctx, decode_state, gen7_mfd_context);
> >      gen75_mfd_avc_img_state(ctx, decode_state, gen7_mfd_context);
> >  
> > +    incorrect_slice = 0;
> >      for (j = 0; j < decode_state->num_slice_params; j++) {
> >          assert(decode_state->slice_params && decode_state->slice_params[j]->buffer);
> >          slice_param = (VASliceParameterBufferH264 *)decode_state->slice_params[j]->buffer;
> > @@ -1158,15 +1160,29 @@ gen75_mfd_avc_decode_picture(VADriverContextP ctx,
> >              else
> >                  next_slice_param = next_slice_group_param;
> >  
> > +            if (next_slice_param != NULL) {
> > +                /* If the mb position of next_slice is less than or equal to the current slice,
> > +                 * ignore the following sequence.
> > +                 */
> > +                if (next_slice_param->first_mb_in_slice <= slice_param->first_mb_in_slice) {
> > +                    next_slice_param = NULL;
> > +                    incorrect_slice = 1;
> > +                }
> > +            }
> > +
> >              gen75_mfd_avc_directmode_state(ctx, decode_state, pic_param, slice_param, gen7_mfd_context);
> >              gen75_mfd_avc_ref_idx_state(ctx, pic_param, slice_param, gen7_mfd_context);
> >              gen75_mfd_avc_weightoffset_state(ctx, pic_param, slice_param, gen7_mfd_context);
> >              gen75_mfd_avc_slice_state(ctx, pic_param, slice_param, next_slice_param, gen7_mfd_context);
> >              gen75_mfd_avc_bsd_object(ctx, pic_param, slice_param, slice_data_bo, next_slice_param, gen7_mfd_context);
> >              slice_param++;
> > +
> > +            if (incorrect_slice)
> > +                goto slice_param_done;
> >          }
> >      }
> >  
> > +slice_param_done:
> >      intel_batchbuffer_end_atomic(batch);
> >      intel_batchbuffer_flush(batch);
> >  }
> > diff --git a/src/gen7_mfd.c b/src/gen7_mfd.c
> > index 46a07a0..a677ed8 100755
> > --- a/src/gen7_mfd.c
> > +++ b/src/gen7_mfd.c
> > @@ -817,6 +817,7 @@ gen7_mfd_avc_decode_picture(VADriverContextP ctx,
> >      VASliceParameterBufferH264 *slice_param, *next_slice_param, *next_slice_group_param;
> >      dri_bo *slice_data_bo;
> >      int i, j;
> > +    int incorrect_slice = 0;
> >  
> >      assert(decode_state->pic_param && decode_state->pic_param->buffer);
> >      pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
> > @@ -831,6 +832,7 @@ gen7_mfd_avc_decode_picture(VADriverContextP ctx,
> >      gen7_mfd_avc_qm_state(ctx, decode_state, gen7_mfd_context);
> >      gen7_mfd_avc_img_state(ctx, decode_state, gen7_mfd_context);
> >  
> > +    incorrect_slice = 0;
> >      for (j = 0; j < decode_state->num_slice_params; j++) {
> >          assert(decode_state->slice_params && decode_state->slice_params[j]->buffer);
> >          slice_param = (VASliceParameterBufferH264 *)decode_state->slice_params[j]->buffer;
> > @@ -855,15 +857,29 @@ gen7_mfd_avc_decode_picture(VADriverContextP ctx,
> >              else
> >                  next_slice_param = next_slice_group_param;
> >  
> > +            if (next_slice_param != NULL) {
> > +                /* If the mb position of next_slice is less than or equal to the current slice,
> > +                 * ignore the following sequence.
> > +                 */
> > +                if (next_slice_param->first_mb_in_slice <= slice_param->first_mb_in_slice) {
> > +                    next_slice_param = NULL;
> > +                    incorrect_slice = 1;
> > +                }
> > +            }
> > +
> >              gen7_mfd_avc_directmode_state(ctx, decode_state, pic_param, slice_param, gen7_mfd_context);
> >              gen7_mfd_avc_ref_idx_state(ctx, pic_param, slice_param, gen7_mfd_context);
> >              gen7_mfd_avc_weightoffset_state(ctx, pic_param, slice_param, gen7_mfd_context);
> >              gen7_mfd_avc_slice_state(ctx, pic_param, slice_param, next_slice_param, gen7_mfd_context);
> >              gen7_mfd_avc_bsd_object(ctx, pic_param, slice_param, slice_data_bo, next_slice_param, gen7_mfd_context);
> >              slice_param++;
> > +
> > +            if (incorrect_slice)
> > +                goto slice_param_done;
> >          }
> >      }
> >  
> > +slice_param_done:
> >      intel_batchbuffer_end_atomic(batch);
> >      intel_batchbuffer_flush(batch);
> >  }
> > diff --git a/src/gen8_mfd.c b/src/gen8_mfd.c
> > index d08dd43..9d59a1e 100644
> > --- a/src/gen8_mfd.c
> > +++ b/src/gen8_mfd.c
> > @@ -882,6 +882,7 @@ gen8_mfd_avc_decode_picture(VADriverContextP ctx,
> >      VASliceParameterBufferH264 *slice_param, *next_slice_param, *next_slice_group_param;
> >      dri_bo *slice_data_bo;
> >      int i, j;
> > +    int incorrect_slice;
> >  
> >      assert(decode_state->pic_param && decode_state->pic_param->buffer);
> >      pic_param = (VAPictureParameterBufferH264 *)decode_state->pic_param->buffer;
> > @@ -897,6 +898,8 @@ gen8_mfd_avc_decode_picture(VADriverContextP ctx,
> >      gen8_mfd_avc_picid_state(ctx, decode_state, gen7_mfd_context);
> >      gen8_mfd_avc_img_state(ctx, decode_state, gen7_mfd_context);
> >  
> > +    incorrect_slice = 0;
> > +
> >      for (j = 0; j < decode_state->num_slice_params; j++) {
> >          assert(decode_state->slice_params && decode_state->slice_params[j]->buffer);
> >          slice_param = (VASliceParameterBufferH264 *)decode_state->slice_params[j]->buffer;
> > @@ -921,15 +924,29 @@ gen8_mfd_avc_decode_picture(VADriverContextP ctx,
> >              else
> >                  next_slice_param = next_slice_group_param;
> >  
> > +            if (next_slice_param != NULL) {
> > +                /* If the mb position of next_slice is less than or equal to the current slice,
> > +                 * ignore the following sequence.
> > +                 */
> > +                if (next_slice_param->first_mb_in_slice <= slice_param->first_mb_in_slice) {
> > +                    next_slice_param = NULL;
> > +                    incorrect_slice = 1;
> > +                }
> > +            }
> > +
> >              gen8_mfd_avc_directmode_state(ctx, decode_state, pic_param, slice_param, gen7_mfd_context);
> >              gen8_mfd_avc_ref_idx_state(ctx, pic_param, slice_param, gen7_mfd_context);
> >              gen8_mfd_avc_weightoffset_state(ctx, pic_param, slice_param, gen7_mfd_context);
> >              gen8_mfd_avc_slice_state(ctx, pic_param, slice_param, next_slice_param, gen7_mfd_context);
> >              gen8_mfd_avc_bsd_object(ctx, pic_param, slice_param, slice_data_bo, next_slice_param, gen7_mfd_context);
> >              slice_param++;
> > +
> > +            if (incorrect_slice)
> > +                goto slice_param_done;
> >          }
> >      }
> >  
> > +slice_param_done:
> >      intel_batchbuffer_end_atomic(batch);
> >      intel_batchbuffer_flush(batch);
> >  }
> 
> 




More information about the Libva mailing list