From 59478b0059f3af023eb3e9f9d3ac5fa537ce1caf Mon Sep 17 00:00:00 2001 From: Philip Langdale Date: Sat, 29 Jul 2023 22:43:58 +0800 Subject: [PATCH] hwtransfer: implement support for hw->hw format conversion Historically, we have not attempted to support hw->hw format conversion in the autoconvert logic. If a user needed to do these kinds of conversions, they needed to manually insert the hw format's conversion filter manually (eg: scale_vaapi). This was usually fine because the general rule is that any format supported by the hardware can be used as well as any other. ie: You would only need to do conversion if you have a specific goal in mind. However, we now have two situations where we can find ourselves with a hardware format being produced by a decoder that cannot be accepted by a VO via hwdec-interop: * dmabuf-wayland can only accept formats that the Wayland compositor accepts. In the case of GNOME, it can only accept a handful of RGB formats. * When decoding via VAAPI on Intel hardware, some of the more unusual video encodings (4:2:2, 10bit 4:4:4) lead to packed frame formats which gpu-next cannot handle, causing rendering to fail. In both these cases (at least when using VAAPI with dmabuf-wayland), if we could detect the failure case and insert a `scale_vaapi` filter, we could get successful output automatically. For `hwdec=drm`, there is currently no conversion filter, so dmabuf-wayland is still out of luck there. The basic approach to implementing this is to detect the case where we are evaluating a hardware format where the VO can accept the hardware format itself, but may not accept the underlying sw format. In the current code, we bypass autoconvert as soon as we see the hardware format is compatible. My first observation was that we actually have logic in autoconvert to detect when the input sw format is not in the list of allowed sw formats passed into the autoconverter. Unfortunately, we never populate this list, and the way you would expect to do that is vo-query-format returning sw format information, which it does not. We could define an extended vo-query-format-2, but we'd still need to implement the probing logic to fill it in. On the other hand, we already have the probing logic in the hwupload filter - and most recently I used that logic to implement conversion on upload. So if we could leverage that, we could both detect when hw->hw conversion is required, and pick the best target format. This exercise is then primarily one of detecting when we are in this case and letting that code run in a useful way. The hwupload filter is a bit awkward to work with today, and so I refactored a bunch of the set up code to actually make it more encapsulated. Now, instead of the caller instantiating it and then triggering the probe, we probe on creation and instantiate the correct underlying filter (hwupload vs conversion) automatically. --- filters/f_autoconvert.c | 48 +++++++++++++-- filters/f_hwtransfer.c | 110 +++++++++++++++++++++++----------- filters/f_hwtransfer.h | 17 +++--- video/hwdec.h | 5 ++ video/out/hwdec/hwdec_vaapi.c | 3 + 5 files changed, 135 insertions(+), 48 deletions(-) diff --git a/filters/f_autoconvert.c b/filters/f_autoconvert.c index b2db4e7f9e..739a45b496 100644 --- a/filters/f_autoconvert.c +++ b/filters/f_autoconvert.c @@ -150,7 +150,12 @@ static bool build_image_converter(struct mp_autoconvert *c, struct mp_log *log, for (int n = 0; n < p->num_imgfmts; n++) { bool samefmt = img->params.imgfmt == p->imgfmts[n]; bool samesubffmt = img->params.hw_subfmt == p->subfmts[n]; - if (samefmt && (samesubffmt || !p->subfmts[n])) { + /* + * In practice, `p->subfmts` is not usually populated today, in which + * case we must actively probe formats below to establish if the VO can + * accept the subfmt being used by the hwdec. + */ + if (samefmt && samesubffmt) { if (p->imgparams_set) { if (!mp_image_params_equal(&p->imgparams, &img->params)) break; @@ -183,26 +188,57 @@ static bool build_image_converter(struct mp_autoconvert *c, struct mp_log *log, bool dst_all_hw = true; bool dst_have_sw = false; + bool has_src_hw_fmt = false; for (int n = 0; n < num_fmts; n++) { bool is_hw = IMGFMT_IS_HWACCEL(fmts[n]); dst_all_hw &= is_hw; dst_have_sw |= !is_hw; + has_src_hw_fmt |= is_hw && fmts[n] == imgpar.imgfmt; } // Source is hw, some targets are sw -> try to download. bool hw_to_sw = !imgfmt_is_sw && dst_have_sw; - if (dst_all_hw && num_fmts > 0) { + if (has_src_hw_fmt) { + int src_fmt = img->params.hw_subfmt; + /* + * If the source format is a hardware format, and our output supports + * that hardware format, we prioritize preserving the use of that + * hardware format. In most cases, the sub format will also be supported + * and no conversion will be required, but in some cases, the hwdec + * may be able to output formats that the VO cannot display, and + * hardware format conversion becomes necessary. + */ + struct mp_hwupload upload = mp_hwupload_create(conv, imgpar.imgfmt, + src_fmt, + true); + if (upload.successful_init) { + if (upload.f) { + mp_info(log, "Converting %s[%s] -> %s[%s]\n", + mp_imgfmt_to_name(imgpar.imgfmt), + mp_imgfmt_to_name(src_fmt), + mp_imgfmt_to_name(imgpar.imgfmt), + mp_imgfmt_to_name(upload.selected_sw_imgfmt)); + filters[2] = upload.f; + } + hw_to_sw = false; + need_sws = false; + } else { + mp_err(log, "Failed to create HW uploader for format %s\n", + mp_imgfmt_to_name(src_fmt)); + } + } else if (dst_all_hw && num_fmts > 0) { bool upload_created = false; int sw_fmt = imgfmt_is_sw ? img->imgfmt : img->params.hw_subfmt; for (int i = 0; i < num_fmts; i++) { // We can probably use this! Very lazy and very approximate. - struct mp_hwupload *upload = mp_hwupload_create(conv, fmts[i]); - if (upload) { + struct mp_hwupload upload = mp_hwupload_create(conv, fmts[i], + sw_fmt, false); + if (upload.successful_init) { mp_info(log, "HW-uploading to %s\n", mp_imgfmt_to_name(fmts[i])); - filters[2] = upload->f; - hwupload_fmt = mp_hwupload_find_upload_format(upload, sw_fmt); + filters[2] = upload.f; + hwupload_fmt = upload.selected_sw_imgfmt; fmts = &hwupload_fmt; num_fmts = hwupload_fmt ? 1 : 0; hw_to_sw = false; diff --git a/filters/f_hwtransfer.c b/filters/f_hwtransfer.c index b281a15838..a8f01e4054 100644 --- a/filters/f_hwtransfer.c +++ b/filters/f_hwtransfer.c @@ -1,6 +1,7 @@ #include #include #include +#include #include "video/fmt-conversion.h" #include "video/hwdec.h" @@ -8,7 +9,10 @@ #include "video/mp_image_pool.h" #include "f_hwtransfer.h" +#include "f_output_chain.h" +#include "f_utils.h" #include "filter_internal.h" +#include "user_filters.h" struct priv { AVBufferRef *av_device_ctx; @@ -39,7 +43,9 @@ struct priv { int *map_fmts; int num_map_fmts; - struct mp_hwupload public; + // If the selected hwdec has a conversion filter available for converting + // between sw formats in hardware, the name will be set. NULL otherwise. + const char *conversion_filter_name; }; struct hwmap_pairs { @@ -66,6 +72,11 @@ static const struct hwmap_pairs hwmap_pairs[] = { /** * @brief Find the closest supported format when hw uploading * + * Return the best format suited for upload that is supported for a given input + * imgfmt. This returns the same as imgfmt if the format is natively supported, + * and otherwise a format that likely results in the least loss. + * Returns 0 if completely unsupported. + * * Some hardware types support implicit format conversion on upload. For these * types, it is possible for the set of formats that are accepts as inputs to * the upload process to differ from the set of formats that can be outputs of @@ -122,23 +133,6 @@ static bool select_format(struct priv *p, int input_fmt, return true; } -int mp_hwupload_find_upload_format(struct mp_hwupload *u, int imgfmt) -{ - struct priv *p = u->f->priv; - - int sw = 0, up = 0; - select_format(p, imgfmt, &sw, &up); - // In th case where the imgfmt is not natively supported, it must be - // converted, either before or during upload. If the imgfmt is supported as - // an hw input format, then prefer that, and if the upload has to do implicit - // conversion, that's fine. On the other hand, if the imgfmt is not a - // supported input format, then pick the output format as the conversion - // target to avoid doing two conversions (one before upload, and one during - // upload). Note that for most hardware types, there is no ability to convert - // during upload, and the two formats will always be the same. - return imgfmt == sw ? sw : up; -} - static void process(struct mp_filter *f) { struct priv *p = f->priv; @@ -266,17 +260,17 @@ static bool vo_supports(struct mp_hwdec_ctx *ctx, int hw_fmt, int sw_fmt) return false; } -static bool probe_formats(struct mp_hwupload *u, int hw_imgfmt) +static bool probe_formats(struct mp_filter *f, int hw_imgfmt) { - struct priv *p = u->f->priv; + struct priv *p = f->priv; p->hw_imgfmt = hw_imgfmt; p->num_fmts = 0; p->num_upload_fmts = 0; - struct mp_stream_info *info = mp_filter_find_stream_info(u->f); + struct mp_stream_info *info = mp_filter_find_stream_info(f); if (!info || !info->hwdec_devs) { - MP_ERR(u->f, "no hw context\n"); + MP_ERR(f, "no hw context\n"); return false; } @@ -312,7 +306,7 @@ static bool probe_formats(struct mp_hwupload *u, int hw_imgfmt) } if (!ctx) { - MP_INFO(u->f, "no support for this hw format\n"); + MP_INFO(f, "no support for this hw format\n"); return false; } @@ -339,14 +333,14 @@ static bool probe_formats(struct mp_hwupload *u, int hw_imgfmt) if (!imgfmt) continue; - MP_VERBOSE(u->f, "looking at format %s/%s\n", + MP_VERBOSE(f, "looking at format %s/%s\n", mp_imgfmt_to_name(hw_imgfmt), mp_imgfmt_to_name(imgfmt)); if (IMGFMT_IS_HWACCEL(imgfmt)) { // If the enumerated format is a hardware format, we don't need to // do any further probing. It will be supported. - MP_VERBOSE(u->f, " supports %s (a hardware format)\n", + MP_VERBOSE(f, " supports %s (a hardware format)\n", mp_imgfmt_to_name(imgfmt)); continue; } @@ -356,7 +350,7 @@ static bool probe_formats(struct mp_hwupload *u, int hw_imgfmt) if (!mp_update_av_hw_frames_pool(&frames, ctx->av_device_ref, hw_imgfmt, imgfmt, 128, 128, false)) { - MP_WARN(u->f, "failed to allocate pool\n"); + MP_WARN(f, "failed to allocate pool\n"); continue; } @@ -375,9 +369,9 @@ static bool probe_formats(struct mp_hwupload *u, int hw_imgfmt) int fmt = pixfmt2imgfmt(fmts[i]); if (!fmt) continue; - MP_VERBOSE(u->f, " supports %s\n", mp_imgfmt_to_name(fmt)); + MP_VERBOSE(f, " supports %s\n", mp_imgfmt_to_name(fmt)); if (!vo_supports(ctx, hw_imgfmt, fmt)) { - MP_VERBOSE(u->f, " ... not supported by VO\n"); + MP_VERBOSE(f, " ... not supported by VO\n"); continue; } MP_TARRAY_APPEND(p, p->upload_fmts, p->num_upload_fmts, fmt); @@ -396,32 +390,78 @@ static bool probe_formats(struct mp_hwupload *u, int hw_imgfmt) p->av_device_ctx = av_buffer_ref(ctx->av_device_ref); if (!p->av_device_ctx) return false; + p->conversion_filter_name = ctx->conversion_filter_name; return p->num_upload_fmts > 0; } -struct mp_hwupload *mp_hwupload_create(struct mp_filter *parent, int hw_imgfmt) +struct mp_hwupload mp_hwupload_create(struct mp_filter *parent, int hw_imgfmt, + int sw_imgfmt, bool src_is_same_hw) { + struct mp_hwupload u = {0,}; struct mp_filter *f = mp_filter_create(parent, &hwupload_filter); if (!f) - return NULL; + return u; struct priv *p = f->priv; - struct mp_hwupload *u = &p->public; - u->f = f; - mp_filter_add_pin(f, MP_PIN_IN, "in"); mp_filter_add_pin(f, MP_PIN_OUT, "out"); - if (!probe_formats(u, hw_imgfmt)) { + if (!probe_formats(f, hw_imgfmt)) { MP_INFO(f, "hardware format not supported\n"); goto fail; } + int hw_input_fmt = 0, hw_output_fmt = 0; + if (!select_format(p, sw_imgfmt, &hw_input_fmt, &hw_output_fmt)) { + MP_ERR(f, "Unable to find a compatible upload format for %s\n", + mp_imgfmt_to_name(sw_imgfmt)); + goto fail; + } + + if (src_is_same_hw) { + if (p->conversion_filter_name) { + /* + * If we are converting from one sw format to another within the same + * hw format, we will use that hw format's conversion filter rather + * than the actual hwupload filter. + */ + u.selected_sw_imgfmt = hw_output_fmt; + if (sw_imgfmt != u.selected_sw_imgfmt) { + enum AVPixelFormat pixfmt = imgfmt2pixfmt(u.selected_sw_imgfmt); + const char *avfmt_name = av_get_pix_fmt_name(pixfmt); + char *args[] = {"format", (char *)avfmt_name, NULL}; + MP_VERBOSE(f, "Hardware conversion: %s -> %s\n", + p->conversion_filter_name, avfmt_name); + struct mp_filter *sv = + mp_create_user_filter(parent, MP_OUTPUT_CHAIN_VIDEO, + p->conversion_filter_name, args); + u.f = sv; + talloc_free(f); + } + } + } else { + u.f = f; + /* + * In the case where the imgfmt is not natively supported, it must be + * converted, either before or during upload. If the imgfmt is supported + * as a hw input format, then prefer that, and if the upload has to do + * implicit conversion, that's fine. On the other hand, if the imgfmt is + * not a supported input format, then pick the output format as the + * conversion target to avoid doing two conversions (one before upload, + * and one during upload). Note that for most hardware types, there is + * no ability to convert during upload, and the two formats will always + * be the same. + */ + u.selected_sw_imgfmt = + sw_imgfmt == hw_input_fmt ? hw_input_fmt : hw_output_fmt; + } + + u.successful_init = true; return u; fail: talloc_free(f); - return NULL; + return u; } static void hwdownload_process(struct mp_filter *f) diff --git a/filters/f_hwtransfer.h b/filters/f_hwtransfer.h index 4f6c253193..dde9cf7137 100644 --- a/filters/f_hwtransfer.h +++ b/filters/f_hwtransfer.h @@ -4,16 +4,19 @@ // A filter which uploads sw frames to hw. Ignores hw frames. struct mp_hwupload { + // Indicates if the filter was successfully initialised, or not. + // If not, the state of other members is undefined. + bool successful_init; + + // The filter to use for uploads. NULL if none is required. struct mp_filter *f; + + // The underlying format of uploaded frames + int selected_sw_imgfmt; }; -struct mp_hwupload *mp_hwupload_create(struct mp_filter *parent, int hw_imgfmt); - -// Return the best format suited for upload that is supported for a given input -// imgfmt. This returns the same as imgfmt if the format is natively supported, -// and otherwise a format that likely results in the least loss. -// Returns 0 if completely unsupported. -int mp_hwupload_find_upload_format(struct mp_hwupload *u, int imgfmt); +struct mp_hwupload mp_hwupload_create(struct mp_filter *parent, int hw_imgfmt, + int sw_imgfmt, bool src_is_same_hw); // A filter which downloads sw frames from hw. Ignores sw frames. struct mp_hwdownload { diff --git a/video/hwdec.h b/video/hwdec.h index f3f2df06f2..44008dece0 100644 --- a/video/hwdec.h +++ b/video/hwdec.h @@ -18,6 +18,11 @@ struct mp_hwdec_ctx { const int *supported_formats; // HW format used by the hwdec int hw_imgfmt; + + // The name of this hwdec's matching conversion filter if available. + // This will be used for hardware conversion of frame formats. + // NULL otherwise. + const char *conversion_filter_name; }; // Used to communicate hardware decoder device handles from VO to video decoder. diff --git a/video/out/hwdec/hwdec_vaapi.c b/video/out/hwdec/hwdec_vaapi.c index 9dd1fcd79e..fafc331277 100644 --- a/video/out/hwdec/hwdec_vaapi.c +++ b/video/out/hwdec/hwdec_vaapi.c @@ -131,6 +131,8 @@ const static dmabuf_interop_init interop_inits[] = { NULL }; +const static char *conversion_filter_name = "scale_vaapi"; + static int init(struct ra_hwdec *hw) { struct priv_owner *p = hw->priv; @@ -177,6 +179,7 @@ static int init(struct ra_hwdec *hw) p->ctx->hwctx.hw_imgfmt = IMGFMT_VAAPI; p->ctx->hwctx.supported_formats = p->formats; p->ctx->hwctx.driver_name = hw->driver->name; + p->ctx->hwctx.conversion_filter_name = conversion_filter_name; hwdec_devices_add(hw->devs, &p->ctx->hwctx); return 0; }