hwdec_vaapi: try format upload lazily

Uploading all available formats has proven to be problematic because
more unusual ones can crash the driver, even when no upload is
necessary. Check the upload only if needed to avoid issues with broken
drivers.

This also might speedup the init process.

Fixes: #14956
Fixes: #15030

filters/f_hwtransfer.c

@@ -15,6 +15,7 @@
 #include "user_filters.h"
 
 struct priv {
+    struct mp_hwdec_ctx *ctx;
     AVBufferRef *av_device_ctx;
 
     AVBufferRef *hw_pool;
@@ -129,23 +130,51 @@ static bool select_format(struct priv *p, int input_fmt,
     int *upload_fmts = &p->upload_fmts[p->fmt_upload_index[index]];
     int num_upload_fmts = p->fmt_upload_num[index];
 
-    // Select the best input format from the available upload formats.
+    if (p->ctx->try_upload) {
-    int hw_input_fmt = mp_imgfmt_select_best_list(upload_fmts, num_upload_fmts, input_fmt);
+        upload_fmts = talloc_array_ptrtype(NULL, upload_fmts, num_upload_fmts);
-
+        memcpy(upload_fmts, &p->upload_fmts[p->fmt_upload_index[index]], num_upload_fmts * sizeof(int));
-    // If the input format is not directly uploadable, conversion will be needed.
-    // Attempt to convert directly to the output format to avoid double conversion.
-    if (input_fmt != hw_input_fmt) {
-        int upload_output_fmt = mp_imgfmt_select_best_list(upload_fmts, num_upload_fmts, hw_output_fmt);
-        // Use this format only if it avoids double conversion, i.e., if we can
-        // upload the output format directly. If that's not the case, just use
-        // the best format for the input format selected earlier, as double
-        // conversion is unavoidable anyway. This approach prefers a closer
-        // conversion before upload and do remaining conversion during upload,
-        // which may be hardware-accelerated.
-        if (upload_output_fmt == hw_output_fmt)
-            hw_input_fmt = upload_output_fmt;
     }
 
+    // Try only upload formats, if all of them fail, give up. We could attempt
+    // using a different hw_output_fmt, can be extended later, if needed.
+    int hw_input_fmt = IMGFMT_NONE;
+    while (num_upload_fmts) {
+        // Select the best input format from the available upload formats.
+        hw_input_fmt = mp_imgfmt_select_best_list(upload_fmts, num_upload_fmts, input_fmt);
+
+        // If the input format is not directly uploadable, conversion will be needed.
+        // Attempt to convert directly to the output format to avoid double conversion.
+        if (input_fmt != hw_input_fmt) {
+            int upload_output_fmt = mp_imgfmt_select_best_list(upload_fmts, num_upload_fmts, hw_output_fmt);
+            // Use this format only if it avoids double conversion, i.e., if we can
+            // upload the output format directly. If that's not the case, just use
+            // the best format for the input format selected earlier, as double
+            // conversion is unavoidable anyway. This approach prefers a closer
+            // conversion before upload and do remaining conversion during upload,
+            // which may be hardware-accelerated.
+            if (upload_output_fmt == hw_output_fmt)
+                hw_input_fmt = upload_output_fmt;
+        }
+
+        if (!hw_input_fmt || !p->ctx->try_upload)
+            break;
+
+        if (p->ctx->try_upload(p->ctx->try_upload_priv, hw_input_fmt, hw_output_fmt))
+            break;
+
+        for (int i = 0; i < num_upload_fmts; i++) {
+            if (upload_fmts[i] != hw_input_fmt)
+                continue;
+            hw_input_fmt = IMGFMT_NONE;
+            MP_TARRAY_REMOVE_AT(upload_fmts, num_upload_fmts, i);
+            break;
+        }
+        assert(hw_input_fmt == IMGFMT_NONE);
+    }
+
+    if (p->ctx->try_upload)
+        talloc_free(upload_fmts);
+
     if (!hw_input_fmt)
         return false;
 
@@ -281,19 +310,6 @@ static bool vo_supports(struct mp_hwdec_ctx *ctx, int hw_fmt, int sw_fmt)
     return false;
 }
 
-static bool upload_supports(struct mp_hwdec_ctx *ctx, int fmt)
-{
-    if (!ctx->supported_hwupload_formats)
-        return true; // if unset, all formats are allowed
-
-    for (int i = 0; ctx->supported_hwupload_formats[i]; i++) {
-        if (ctx->supported_hwupload_formats[i] == fmt)
-            return true;
-    }
-
-    return false;
-}
-
 /**
  * Some hwcontexts do not implement constraints, and so cannot
  * report supported formats, so cobble something together from our
@@ -512,30 +528,16 @@ static bool probe_formats(struct mp_filter *f, int hw_imgfmt, bool use_conversio
 
                 p->fmt_upload_index[index] = p->num_upload_fmts;
 
-                int *upload_not_supported = NULL;
-                int num_upload_not_supported = 0;
-
                 MP_DBG(f, "  supports:");
                 for (int i = 0; fmts[i] != AV_PIX_FMT_NONE; i++) {
                     int fmt = pixfmt2imgfmt(fmts[i]);
                     if (!fmt)
                         continue;
-                    if (!upload_supports(ctx, fmt)) {
-                        MP_TARRAY_APPEND(NULL, upload_not_supported, num_upload_not_supported, fmt);
-                        continue;
-                    }
                     MP_DBG(f, " %s", mp_imgfmt_to_name(fmt));
                     MP_TARRAY_APPEND(p, p->upload_fmts, p->num_upload_fmts, fmt);
                 }
-                if (num_upload_not_supported) {
-                    MP_DBG(f, "\n  upload not supported:");
-                    for (int i = 0; i < num_upload_not_supported; ++i)
-                        MP_DBG(f, " %s", mp_imgfmt_to_name(upload_not_supported[i]));
-                }
                 MP_DBG(f, "\n");
 
-                talloc_free(upload_not_supported);
-
                 p->fmt_upload_num[index] =
                     p->num_upload_fmts - p->fmt_upload_index[index];
 
@@ -551,6 +553,7 @@ static bool probe_formats(struct mp_filter *f, int hw_imgfmt, bool use_conversio
     p->av_device_ctx = av_buffer_ref(ctx->av_device_ref);
     if (!p->av_device_ctx)
         return false;
+    p->ctx = ctx;
     p->get_conversion_filter = ctx->get_conversion_filter;
 
     /*

video/hwdec.h

@@ -7,6 +7,7 @@
 #include "options/m_option.h"
 
 struct mp_image_pool;
+enum mp_imgfmt;
 
 struct mp_conversion_filter {
     // Name of the conversion filter.
@@ -28,9 +29,12 @@ struct mp_hwdec_ctx {
     // HW format used by the hwdec
     int hw_imgfmt;
 
-    // List of support software formats when doing hwuploads.
+    // Callback to test if the format can be uploaded.
     // If NULL, all possible hwuploads are assumed to be supported.
-    const int *supported_hwupload_formats;
+    bool (*try_upload)(void *p, enum mp_imgfmt src_fmt, enum mp_imgfmt dst_fmt);
+
+    // Private data for try_upload.
+    void *try_upload_priv;
 
     // Getter for conversion filter description, or NULL.
     // This will be used for hardware conversion of frame formats.

video/out/hwdec/hwdec_vaapi.c

@@ -107,8 +107,6 @@ struct priv_owner {
     struct mp_vaapi_ctx *ctx;
     VADisplay *display;
     int *formats;
-    int *hwupload_formats;
-    int num_hwupload_formats;
     bool probing_formats; // temporary during init
 
     struct dmabuf_interop dmabuf_interop;
@@ -163,6 +161,8 @@ static struct mp_conversion_filter *get_conversion_filter_desc(int target_imgfmt
     return desc;
 }
 
+static bool try_format_upload(void *priv, enum mp_imgfmt src_fmt, enum mp_imgfmt dst_fmt);
+
 static int init(struct ra_hwdec *hw)
 {
     struct priv_owner *p = hw->priv;
@@ -218,7 +218,8 @@ static int init(struct ra_hwdec *hw)
 
     p->ctx->hwctx.hw_imgfmt = IMGFMT_VAAPI;
     p->ctx->hwctx.supported_formats = p->formats;
-    p->ctx->hwctx.supported_hwupload_formats = p->hwupload_formats;
+    p->ctx->hwctx.try_upload = try_format_upload;
+    p->ctx->hwctx.try_upload_priv = p;
     p->ctx->hwctx.driver_name = hw->driver->name;
     p->ctx->hwctx.get_conversion_filter = get_conversion_filter_desc;
     p->ctx->hwctx.conversion_config = hwconfig;
@@ -439,26 +440,32 @@ err:
     av_buffer_unref(&fref);
 }
 
-static void try_format_upload(struct ra_hwdec *hw, enum AVPixelFormat pixfmt)
+static bool try_format_upload(void *priv, enum mp_imgfmt src_fmt, enum mp_imgfmt dst_fmt)
 {
-    int mp_fmt = pixfmt2imgfmt(pixfmt);
+    struct priv_owner *p = priv;
-    if (!mp_fmt || IMGFMT_IS_HWACCEL(mp_fmt))
-        return;
 
-    // Arbitrarily use the first format we have for the hw_subfmt.
+    if (IMGFMT_IS_HWACCEL(src_fmt))
-    struct priv_owner *p = hw->priv;
+        return true;
-    if (!p->formats || !p->formats[0])
-        return;
 
-    struct mp_image *src = mp_image_alloc(mp_fmt, 2, 2);
+    bool ret = false;
+
+    struct mp_image *src = mp_image_alloc(src_fmt, 16, 16);
     if (!src)
-        return;
+        goto end;
 
     AVBufferRef *hw_pool = av_hwframe_ctx_alloc(p->ctx->av_device_ref);
-    mp_update_av_hw_frames_pool(&hw_pool, p->ctx->av_device_ref, IMGFMT_VAAPI,
+    if (!hw_pool)
-                                p->formats[0], src->w, src->h, false);
+        goto end;
+
+    if (!mp_update_av_hw_frames_pool(&hw_pool, p->ctx->av_device_ref, IMGFMT_VAAPI,
+                                     dst_fmt, src->w, src->h, false))
+    {
+        goto end;
+    }
 
     struct mp_image *dst = mp_av_pool_image_hw_upload(hw_pool, src);
+    if (!dst)
+        goto end;
 
     VADisplay *display = p->display;
     VADRMPRIMESurfaceDescriptor desc = {0};
@@ -469,15 +476,18 @@ static void try_format_upload(struct ra_hwdec *hw, enum AVPixelFormat pixfmt)
     VAStatus status = vaExportSurfaceHandle(display, id, VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME_2,
                                             flags | VA_EXPORT_SURFACE_READ_ONLY, &desc);
 
-    if (status == VA_STATUS_SUCCESS)
+    if (status != VA_STATUS_SUCCESS)
-        MP_TARRAY_APPEND(p, p->hwupload_formats, p->num_hwupload_formats, mp_fmt);
+        goto end;
 
+    ret = true;
+
+end:
     close_file_descriptors(&desc);
     av_buffer_unref(&hw_pool);
     mp_image_unrefp(&dst);
     mp_image_unrefp(&src);
 
-    return;
+    return ret;
 }
 
 static void try_format_config(struct ra_hwdec *hw, AVVAAPIHWConfig *hwconfig)
@@ -524,11 +534,6 @@ static void try_format_config(struct ra_hwdec *hw, AVVAAPIHWConfig *hwconfig)
                     fmts[n] != AV_PIX_FMT_NONE; n++)
         try_format_pixfmt(hw, fmts[n]);
 
-    for (int n = 0; fmts &&
-                    fmts[n] != AV_PIX_FMT_NONE; n++)
-        try_format_upload(hw, fmts[n]);
-    MP_TARRAY_APPEND(p, p->hwupload_formats, p->num_hwupload_formats, 0); // sanity check
-
 err:
     av_hwframe_constraints_free(&fc);
     av_buffer_unref(&fref);