[vlc-devel] [PATCH] avcodec: va: directly set data[3] with surface

[Thomas Guillem]

LGTM

On Tue, Jun 23, 2020, at 18:51, Alexandre Janniaux wrote:
> We never use data[0] for anything, nor does avcodec, and it can
> actually keep the same initialized value or be NULL.
> 
> In particular, it fixes avcodec decoder with vaapi vlc_va_t backend
> which was generating VASurfaceID of value 0 to count, leading to the
> following assertion:
> 
>     vlc: ../../modules/codec/avcodec/video.c:1492: lavc_va_GetFrame: 
> Assertion `frame->data[0] != NULL' failed.
>     [1]    154639 abort (core dumped)  ./build-native/vlc <sample> 
> --dec-dev=vaapi -vvv
> ---
>  modules/codec/avcodec/d3d11va.c | 2 +-
>  modules/codec/avcodec/dxva2.c   | 2 +-
>  modules/codec/avcodec/vaapi.c   | 2 +-
>  modules/codec/avcodec/video.c   | 5 +----
>  4 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/modules/codec/avcodec/d3d11va.c b/modules/codec/avcodec/d3d11va.c
> index aaa41fcd73a..c9793a55d1b 100644
> --- a/modules/codec/avcodec/d3d11va.c
> +++ b/modules/codec/avcodec/d3d11va.c
> @@ -227,7 +227,7 @@ static int Get(vlc_va_t *va, picture_t *pic, uint8_t **data)
>          va_surface_Release(va_surface);
>          return VLC_ENOITEM;
>      }
> -    *data = (uint8_t*)sys->hw_surface[va_surface_GetIndex(va_surface)];
> +    data[3] = (uint8_t*)sys->hw_surface[va_surface_GetIndex(va_surface)];
>      return VLC_SUCCESS;
>  }
>  
> diff --git a/modules/codec/avcodec/dxva2.c 
> b/modules/codec/avcodec/dxva2.c
> index aefcb554971..4a7b62814f9 100644
> --- a/modules/codec/avcodec/dxva2.c
> +++ b/modules/codec/avcodec/dxva2.c
> @@ -238,7 +238,7 @@ static int Get(vlc_va_t *va, picture_t *pic, 
> uint8_t **data)
>          va_surface_Release(va_surface);
>          return VLC_ENOITEM;
>      }
> -    *data = 
> (uint8_t*)DXVA2_PICCONTEXT_FROM_PICCTX(pic->context)->ctx.picsys.surface;
> +    data[3] = 
> (uint8_t*)DXVA2_PICCONTEXT_FROM_PICCTX(pic->context)->ctx.picsys.surface;
>      return VLC_SUCCESS;
>  }
>  
> diff --git a/modules/codec/avcodec/vaapi.c b/modules/codec/avcodec/vaapi.c
> index 41136dc85a5..01d2b350f1a 100644
> --- a/modules/codec/avcodec/vaapi.c
> +++ b/modules/codec/avcodec/vaapi.c
> @@ -170,7 +170,7 @@ static int Get(vlc_va_t *va, picture_t *pic, uint8_t **data)
>      vaapi_ctx->ctx.va_dpy = sys->hw_ctx.display;
>      vaapi_ctx->va_surface = va_surface;
>      vlc_vaapi_PicSetContext(pic, &vaapi_ctx->ctx);
> -    *data = (void *) (uintptr_t) vaapi_ctx->ctx.surface;
> +    data[3] = (void *) (uintptr_t) vaapi_ctx->ctx.surface;
>  
>      return VLC_SUCCESS;
>  }
> diff --git a/modules/codec/avcodec/video.c 
> b/modules/codec/avcodec/video.c
> index b74d77e0f54..d89635dbe8f 100644
> --- a/modules/codec/avcodec/video.c
> +++ b/modules/codec/avcodec/video.c
> @@ -1483,16 +1483,13 @@ static int lavc_va_GetFrame(struct 
> AVCodecContext *ctx, AVFrame *frame)
>      if (pic == NULL)
>          return -1;
>  
> +    /* data[3] will contains the format-specific surface handle. */
>      if (vlc_va_Get(va, pic, &frame->data[0]))
>      {
>          msg_Err(dec, "hardware acceleration picture allocation failed");
>          picture_Release(pic);
>          return -1;
>      }
> -    assert(frame->data[0] != NULL);
> -    /* data[0] must be non-NULL for libavcodec internal checks.
> -     * data[3] actually contains the format-specific surface handle. */
> -    frame->data[3] = frame->data[0];
>  
>      frame->buf[0] = av_buffer_create(frame->data[0], 0, 
> lavc_ReleaseFrame, pic, 0);
>      if (unlikely(frame->buf[0] == NULL))
> -- 
> 2.27.0
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel