From 422af1b948135e422a13e5d58bef32e4ee38f172 Mon Sep 17 00:00:00 2001 From: wm4 Date: Mon, 17 Mar 2014 18:23:24 +0100 Subject: vaapi: solve potential problem with ambiguous image formats VAAPI has some ambiguous image formats, like VA_FOURCC_I420, VA_FOURCC_IYUV, VA_FOURCC_YV12 (the latter exactly the same as the first two, just with swapped planes). There is potentially a problem when one specific VAAPI format was picked, and converting it to a mpv format and back to a VAAPI FourCC would result in a numerically different format (even if it's actually the same). Then it could e.g. happen that functions like va_surface_upload() reallocate the underlying VAImage, which would be inefficient. Change the code so that this can't happen. (Probably not a problem in practice with the current VAAPI usage.) --- video/vaapi.c | 59 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 27 insertions(+), 32 deletions(-) (limited to 'video') diff --git a/video/vaapi.c b/video/vaapi.c index df2ce70934..2b2b7819dd 100644 --- a/video/vaapi.c +++ b/video/vaapi.c @@ -238,17 +238,16 @@ static void va_surface_image_destroy(struct va_surface *surface) surface->is_derived = false; } -static VAImage *va_surface_image_alloc(struct mp_image *img, - VAImageFormat *format) +static int va_surface_image_alloc(struct mp_image *img, VAImageFormat *format) { struct va_surface *p = va_surface_in_mp_image(img); if (!format || !p) - return NULL; + return -1; VADisplay *display = p->display; if (p->image.image_id != VA_INVALID_ID && p->image.format.fourcc == format->fourcc) - return &p->image; + return 0; va_surface_image_destroy(p); @@ -270,24 +269,29 @@ static VAImage *va_surface_image_alloc(struct mp_image *img, status = vaCreateImage(p->display, format, img->w, img->h, &p->image); if (!CHECK_VA_STATUS(p->ctx, "vaCreateImage()")) { p->image.image_id = VA_INVALID_ID; - return NULL; + return -1; } } - return &p->image; + return 0; } // img must be a VAAPI surface; make sure its internal VAImage is allocated // to a format corresponding to imgfmt (or return an error). int va_surface_alloc_imgfmt(struct mp_image *img, int imgfmt) { - struct va_surface *surface = va_surface_in_mp_image(img); - if (!surface) + struct va_surface *p = va_surface_in_mp_image(img); + if (!p) return -1; + // Multiple FourCCs can refer to the same imgfmt, so check by doing the + // surjective conversion first. + if (p->image.image_id != VA_INVALID_ID && + va_fourcc_to_imgfmt(p->image.format.fourcc) == imgfmt) + return 0; VAImageFormat *format = - va_image_format_from_imgfmt(surface->ctx->image_formats, imgfmt); + va_image_format_from_imgfmt(p->ctx->image_formats, imgfmt); if (!format) return -1; - if (!va_surface_image_alloc(img, format)) + if (va_surface_image_alloc(img, format) < 0) return -1; return 0; } @@ -333,11 +337,7 @@ int va_surface_upload(struct mp_image *va_dst, struct mp_image *sw_src) if (!p) return -1; - VAImageFormat *format = - va_image_format_from_imgfmt(p->ctx->image_formats, sw_src->imgfmt); - if (!format) - return -1; - if (!va_surface_image_alloc(va_dst, format)) + if (va_surface_alloc_imgfmt(va_dst, sw_src->imgfmt) < 0) return -1; struct mp_image img; @@ -359,7 +359,6 @@ int va_surface_upload(struct mp_image *va_dst, struct mp_image *sw_src) } static struct mp_image *try_download(struct mp_image *src, - VAImageFormat *format, struct mp_image_pool *pool) { VAStatus status; @@ -367,15 +366,12 @@ static struct mp_image *try_download(struct mp_image *src, if (!p) return NULL; - enum mp_imgfmt imgfmt = va_fourcc_to_imgfmt(format->fourcc); - if (imgfmt == IMGFMT_NONE) - return NULL; + VAImage *image = &p->image; - if (!va_surface_image_alloc(src, format)) + if (image->image_id == VA_INVALID_ID || + !va_fourcc_to_imgfmt(image->format.fourcc)) return NULL; - VAImage *image = &p->image; - if (!p->is_derived) { status = vaGetImage(p->display, p->id, 0, 0, src->w, src->h, image->image_id); @@ -386,15 +382,15 @@ static struct mp_image *try_download(struct mp_image *src, struct mp_image *dst = NULL; struct mp_image tmp; if (va_image_map(p->ctx, image, &tmp)) { - assert(tmp.imgfmt == imgfmt); - dst = pool ? mp_image_pool_get(pool, imgfmt, tmp.w, tmp.h) - : mp_image_alloc(imgfmt, tmp.w, tmp.h); + dst = pool ? mp_image_pool_get(pool, tmp.imgfmt, tmp.w, tmp.h) + : mp_image_alloc(tmp.imgfmt, tmp.w, tmp.h); mp_image_copy(dst, &tmp); va_image_unmap(p->ctx, image); } return dst; } +// Return a software copy of the IMGFMT_VAAPI src image. // pool is optional (used for allocating returned images). struct mp_image *va_surface_download(struct mp_image *src, struct mp_image_pool *pool) @@ -407,17 +403,16 @@ struct mp_image *va_surface_download(struct mp_image *src, if (!CHECK_VA_STATUS(ctx, "vaSyncSurface()")) return NULL; - VAImage *image = &p->image; - if (image->image_id != VA_INVALID_ID) { - struct mp_image *mpi = try_download(src, &image->format, pool); - if (mpi) - return mpi; - } + struct mp_image *mpi = try_download(src, pool); + if (mpi) + return mpi; // We have no clue which format will work, so try them all. for (int i = 0; i < ctx->image_formats->num; i++) { VAImageFormat *format = &ctx->image_formats->entries[i]; - struct mp_image *mpi = try_download(src, format, pool); + if (va_surface_image_alloc(src, format) < 0) + continue; + mpi = try_download(src, pool); if (mpi) return mpi; } -- cgit v1.2.3