[FFmpeg-devel,V3,2/2] hwaccel_mpeg: check reference pictures are available

Submitted by Zhong Li on Oct. 12, 2017, 8:20 a.m.

Details

Message ID 20171012082045.21146-2-zhong.li@intel.com
State New
Headers show

Commit Message

Zhong Li Oct. 12, 2017, 8:20 a.m.
some reference pictures are not existed but valiad surface_ids are set,
(for example, the second field of the first key frame can be
P_Picture_Type but there is no preivous frame.)
then may cause some driver problems (e.g, segment fault on  mesa/AMD driver).

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 libavcodec/dxva2_mpeg2.c  |  4 ++--
 libavcodec/vaapi_mpeg2.c  |  6 ++++--
 libavcodec/vdpau_mpeg12.c | 15 ++++++++++-----
 3 files changed, 16 insertions(+), 9 deletions(-)

Comments

Michael Niedermayer Oct. 12, 2017, 8:21 p.m.
On Thu, Oct 12, 2017 at 04:20:45PM +0800, Zhong Li wrote:
> some reference pictures are not existed but valiad surface_ids are set,
> (for example, the second field of the first key frame can be
> P_Picture_Type but there is no preivous frame.)
> then may cause some driver problems (e.g, segment fault on  mesa/AMD driver).
> 
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
[...]

> diff --git a/libavcodec/vdpau_mpeg12.c b/libavcodec/vdpau_mpeg12.c
> index b657007ee7..6d73d45f1a 100644
> --- a/libavcodec/vdpau_mpeg12.c
> +++ b/libavcodec/vdpau_mpeg12.c
> @@ -45,13 +45,18 @@ static int vdpau_mpeg_start_frame(AVCodecContext *avctx,
>  
>      switch (s->pict_type) {
>      case AV_PICTURE_TYPE_B:
> -        ref = ff_vdpau_get_surface_id(s->next_picture.f);
> -        assert(ref != VDP_INVALID_HANDLE);
> -        info->backward_reference = ref;
> +        if (s->next_picture_ptr) {
> +            ref = ff_vdpau_get_surface_id(s->next_picture.f);
> +            assert(ref != VDP_INVALID_HANDLE);
> +            info->backward_reference = ref;
> +        }
>          /* fall through to forward prediction */

>      case AV_PICTURE_TYPE_P:
> -        ref = ff_vdpau_get_surface_id(s->last_picture.f);
> -        info->forward_reference  = ref;
> +        if (s->last_picture_ptr) {
> +            ref = ff_vdpau_get_surface_id(s->last_picture.f);

> +            assert(ref != VDP_INVALID_HANDLE);

please use av_assert*()

[...]
Carl Eugen Hoyos Oct. 12, 2017, 8:34 p.m.
2017-10-12 10:20 GMT+02:00 Zhong Li <zhong.li@intel.com>:
> some reference pictures are not existed but valiad surface_ids are set,
> (for example, the second field of the first key frame can be
> P_Picture_Type but there is no preivous frame.)
> then may cause some driver problems

> (e.g, segment fault on  mesa/AMD driver).

Please also confirm that these driver bugs are fixed.

Carl Eugen
Zhong Li Oct. 13, 2017, 5:24 a.m.
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Friday, October 13, 2017 4:21 AM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V3 2/2] hwaccel_mpeg: check reference
> pictures are available
> 
> On Thu, Oct 12, 2017 at 04:20:45PM +0800, Zhong Li wrote:
> > some reference pictures are not existed but valiad surface_ids are
> > set, (for example, the second field of the first key frame can be
> > P_Picture_Type but there is no preivous frame.) then may cause some
> > driver problems (e.g, segment fault on  mesa/AMD driver).
> >
> > Signed-off-by: Zhong Li <zhong.li@intel.com>
> > ---
> [...]
> 
> > diff --git a/libavcodec/vdpau_mpeg12.c b/libavcodec/vdpau_mpeg12.c
> > index b657007ee7..6d73d45f1a 100644
> > --- a/libavcodec/vdpau_mpeg12.c
> > +++ b/libavcodec/vdpau_mpeg12.c
> > @@ -45,13 +45,18 @@ static int
> vdpau_mpeg_start_frame(AVCodecContext
> > *avctx,
> >
> >      switch (s->pict_type) {
> >      case AV_PICTURE_TYPE_B:
> > -        ref = ff_vdpau_get_surface_id(s->next_picture.f);
> > -        assert(ref != VDP_INVALID_HANDLE);
> > -        info->backward_reference = ref;
> > +        if (s->next_picture_ptr) {
> > +            ref = ff_vdpau_get_surface_id(s->next_picture.f);
> > +            assert(ref != VDP_INVALID_HANDLE);
> > +            info->backward_reference = ref;
> > +        }
> >          /* fall through to forward prediction */
> 
> >      case AV_PICTURE_TYPE_P:
> > -        ref = ff_vdpau_get_surface_id(s->last_picture.f);
> > -        info->forward_reference  = ref;
> > +        if (s->last_picture_ptr) {
> > +            ref = ff_vdpau_get_surface_id(s->last_picture.f);
> 
> > +            assert(ref != VDP_INVALID_HANDLE);
> 
> please use av_assert*()

Ok, will update.
> 
> [...]
> --
> Michael     GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> When you are offended at any man's fault, turn to yourself and study your
> own failings. Then you will forget your anger. -- Epictetus

Patch hide | download patch | download mbox

diff --git a/libavcodec/dxva2_mpeg2.c b/libavcodec/dxva2_mpeg2.c
index b7c69378f0..9117a3d7b1 100644
--- a/libavcodec/dxva2_mpeg2.c
+++ b/libavcodec/dxva2_mpeg2.c
@@ -51,11 +51,11 @@  static void fill_picture_parameters(AVCodecContext *avctx,
     memset(pp, 0, sizeof(*pp));
     pp->wDecodedPictureIndex         = ff_dxva2_get_surface_index(avctx, ctx, current_picture->f);
     pp->wDeblockedPictureIndex       = 0;
-    if (s->pict_type != AV_PICTURE_TYPE_I)
+    if (s->pict_type != AV_PICTURE_TYPE_I && s->last_picture_ptr)
         pp->wForwardRefPictureIndex  = ff_dxva2_get_surface_index(avctx, ctx, s->last_picture.f);
     else
         pp->wForwardRefPictureIndex  = 0xffff;
-    if (s->pict_type == AV_PICTURE_TYPE_B)
+    if (s->pict_type == AV_PICTURE_TYPE_B && s->next_picture_ptr)
         pp->wBackwardRefPictureIndex = ff_dxva2_get_surface_index(avctx, ctx, s->next_picture.f);
     else
         pp->wBackwardRefPictureIndex = 0xffff;
diff --git a/libavcodec/vaapi_mpeg2.c b/libavcodec/vaapi_mpeg2.c
index 0d197c9692..942711b39f 100644
--- a/libavcodec/vaapi_mpeg2.c
+++ b/libavcodec/vaapi_mpeg2.c
@@ -73,10 +73,12 @@  static int vaapi_mpeg2_start_frame(AVCodecContext *avctx, av_unused const uint8_
 
     switch (s->pict_type) {
     case AV_PICTURE_TYPE_B:
-        pic_param.backward_reference_picture = ff_vaapi_get_surface_id(s->next_picture.f);
+        if (s->next_picture_ptr)
+            pic_param.backward_reference_picture = ff_vaapi_get_surface_id(s->next_picture.f);
         // fall-through
     case AV_PICTURE_TYPE_P:
-        pic_param.forward_reference_picture = ff_vaapi_get_surface_id(s->last_picture.f);
+        if (s->last_picture_ptr)
+            pic_param.forward_reference_picture = ff_vaapi_get_surface_id(s->last_picture.f);
         break;
     }
 
diff --git a/libavcodec/vdpau_mpeg12.c b/libavcodec/vdpau_mpeg12.c
index b657007ee7..6d73d45f1a 100644
--- a/libavcodec/vdpau_mpeg12.c
+++ b/libavcodec/vdpau_mpeg12.c
@@ -45,13 +45,18 @@  static int vdpau_mpeg_start_frame(AVCodecContext *avctx,
 
     switch (s->pict_type) {
     case AV_PICTURE_TYPE_B:
-        ref = ff_vdpau_get_surface_id(s->next_picture.f);
-        assert(ref != VDP_INVALID_HANDLE);
-        info->backward_reference = ref;
+        if (s->next_picture_ptr) {
+            ref = ff_vdpau_get_surface_id(s->next_picture.f);
+            assert(ref != VDP_INVALID_HANDLE);
+            info->backward_reference = ref;
+        }
         /* fall through to forward prediction */
     case AV_PICTURE_TYPE_P:
-        ref = ff_vdpau_get_surface_id(s->last_picture.f);
-        info->forward_reference  = ref;
+        if (s->last_picture_ptr) {
+            ref = ff_vdpau_get_surface_id(s->last_picture.f);
+            assert(ref != VDP_INVALID_HANDLE);
+            info->forward_reference  = ref;
+        }
     }
 
     info->slice_count                = 0;