diff mbox series

[FFmpeg-devel,v2,2/2] fftools/ffplay: add hwaccel decoding support

Message ID tencent_5B85E615D0BE66C8A7C6AA92530DDC43CB06@qq.com
State New
Headers show
Series [FFmpeg-devel,v2,1/2] fftools/ffplay: add vulkan renderer via libplacebo | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Zhao Zhili Oct. 18, 2023, 4:55 p.m. UTC
From: Zhao Zhili <zhilizhao@tencent.com>

---
 fftools/ffplay.c          |  30 ++++++++++
 fftools/ffplay_renderer.c | 117 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+)

Comments

Zhao Zhili Oct. 18, 2023, 5:05 p.m. UTC | #1
On 2023/10/19 00:55, Zhao Zhili wrote:
> From: Zhao Zhili <zhilizhao@tencent.com>
>
> ---
>   fftools/ffplay.c          |  30 ++++++++++
>   fftools/ffplay_renderer.c | 117 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 147 insertions(+)

With this patchset, I got a heap-use-after-free crash in vulkan decoder 
after seek. See ticket 10628 for ASAN information.

https://trac.ffmpeg.org/ticket/10628
Lynne Oct. 19, 2023, 1:31 a.m. UTC | #2
Oct 18, 2023, 19:05 by quinkblack@foxmail.com:

>
> On 2023/10/19 00:55, Zhao Zhili wrote:
>
>> From: Zhao Zhili <zhilizhao@tencent.com>
>>
>> ---
>>  fftools/ffplay.c          |  30 ++++++++++
>>  fftools/ffplay_renderer.c | 117 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 147 insertions(+)
>>
>
> With this patchset, I got a heap-use-after-free crash in vulkan decoder after seek. See ticket 10628 for ASAN information.
>
> https://trac.ffmpeg.org/ticket/10628
>

Why are you reporting an issue for a patch that hasn't
even been merged yet?
Zhao Zhili Oct. 19, 2023, 1:41 a.m. UTC | #3
> 在 2023年10月19日,上午9:31,Lynne <dev@lynne.ee> 写道:
> 
> Oct 18, 2023, 19:05 by quinkblack@foxmail.com:
> 
>> 
>>> On 2023/10/19 00:55, Zhao Zhili wrote:
>>> 
>>> From: Zhao Zhili <zhilizhao@tencent.com>
>>> 
>>> ---
>>> fftools/ffplay.c          |  30 ++++++++++
>>> fftools/ffplay_renderer.c | 117 ++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 147 insertions(+)
>>> 
>> 
>> With this patchset, I got a heap-use-after-free crash in vulkan decoder after seek. See ticket 10628 for ASAN information.
>> 
>> https://trac.ffmpeg.org/ticket/10628
>> 
> 
> Why are you reporting an issue for a patch that hasn't
> even been merged yet?

The issue isn’t created by this patch, but triggered by this patch. If user call libavcodec in the same way as ffplay, they can get the same result, but harder for us to reproduce.

I think this is one of the reasons to let ffplay test our hardware decoders.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lynne Oct. 19, 2023, 3:31 a.m. UTC | #4
Oct 18, 2023, 18:55 by quinkblack@foxmail.com:

> From: Zhao Zhili <zhilizhao@tencent.com>
>
> ---
>  fftools/ffplay.c          |  30 ++++++++++
>  fftools/ffplay_renderer.c | 117 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 147 insertions(+)
>

This patch doesn't look correct, libplacebo and lavc have to
share the same Vulkan device.
Either create a hwcontext vulkan device and import it into libplacebo,
which you would then use for decoding, or use libplacebo's context and
initialize a lavu hwcontext from it.
Zhao Zhili Oct. 19, 2023, 3:51 a.m. UTC | #5
> On Oct 19, 2023, at 11:31, Lynne <dev@lynne.ee> wrote:
> 
> Oct 18, 2023, 18:55 by quinkblack@foxmail.com:
> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> ---
>> fftools/ffplay.c          |  30 ++++++++++
>> fftools/ffplay_renderer.c | 117 ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 147 insertions(+)
>> 
> 
> This patch doesn't look correct, libplacebo and lavc have to
> share the same Vulkan device.
> Either create a hwcontext vulkan device and import it into libplacebo,
> which you would then use for decoding, or use libplacebo's context and
> initialize a lavu hwcontext from it.

OK. Will fixed in next version.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 305d72d8b8..0f9584b57e 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -352,6 +352,7 @@  static int autorotate = 1;
 static int find_stream_info = 1;
 static int filter_nbthreads = 0;
 static int enable_vulkan = 0;
+static const char *hwaccel = NULL;
 
 /* current context */
 static int is_full_screen;
@@ -2557,6 +2558,24 @@  static int audio_open(void *opaque, AVChannelLayout *wanted_channel_layout, int
     return spec.size;
 }
 
+static int create_hwaccel(AVBufferRef **device_ctx)
+{
+    enum AVHWDeviceType type;
+    int ret;
+
+    *device_ctx = NULL;
+
+    if (!hwaccel)
+        return 0;
+
+    type = av_hwdevice_find_type_by_name(hwaccel);
+    if (type == AV_HWDEVICE_TYPE_NONE)
+        return AVERROR(ENOTSUP);
+
+    ret = av_hwdevice_ctx_create(device_ctx, type, NULL, NULL, 0);
+    return ret;
+}
+
 /* open a given stream. Return 0 if OK */
 static int stream_component_open(VideoState *is, int stream_index)
 {
@@ -2624,6 +2643,12 @@  static int stream_component_open(VideoState *is, int stream_index)
 
     av_dict_set(&opts, "flags", "+copy_opaque", AV_DICT_MULTIKEY);
 
+    if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
+        ret = create_hwaccel(&avctx->hw_device_ctx);
+        if (ret < 0)
+            goto fail;
+    }
+
     if ((ret = avcodec_open2(avctx, codec, &opts)) < 0) {
         goto fail;
     }
@@ -3625,6 +3650,7 @@  static const OptionDef options[] = {
         "read and decode the streams to fill missing information with heuristics" },
     { "filter_threads", HAS_ARG | OPT_INT | OPT_EXPERT, { &filter_nbthreads }, "number of filter threads per graph" },
     { "enable_vulkan", OPT_BOOL, { &enable_vulkan }, "enable vulkan render" },
+    { "hwaccel", HAS_ARG | OPT_STRING | OPT_EXPERT, { &hwaccel }, "use HW accelerated decoding" },
     { NULL, },
 };
 
@@ -3739,6 +3765,10 @@  int main(int argc, char **argv)
 #ifdef SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR
         SDL_SetHint(SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR, "0");
 #endif
+        if (hwaccel && !enable_vulkan) {
+            av_log(NULL, AV_LOG_INFO, "Enable vulkan renderer to support hwaccel %s\n", hwaccel);
+            enable_vulkan = 1;
+        }
         if (enable_vulkan) {
             vk_renderer = vk_get_renderer();
             if (vk_renderer) {
diff --git a/fftools/ffplay_renderer.c b/fftools/ffplay_renderer.c
index 796a964a7b..bf6ad7fbaf 100644
--- a/fftools/ffplay_renderer.c
+++ b/fftools/ffplay_renderer.c
@@ -50,6 +50,12 @@  typedef struct RendererContext {
     pl_tex tex[4];
 
     pl_log vk_log;
+
+    AVBufferRef *hw_device;
+    AVBufferRef *hw_frame;
+    int hw_failed;
+
+    AVFrame *vk_frame;
 } RendererContext;
 
 static void vk_log_cb(void *log_priv, enum pl_log_level level, const char *msg) {
@@ -133,6 +139,13 @@  static int create(VkRenderer *renderer, SDL_Window *window)
         ret = AVERROR_EXTERNAL;
         goto out;
     }
+
+    ctx->vk_frame = av_frame_alloc();
+    if (!ctx->vk_frame) {
+        ret = AVERROR(ENOMEM);
+        goto out;
+    }
+
     ret = 0;
 
 out:
@@ -140,6 +153,102 @@  out:
     return ret;
 }
 
+static int create_hw(VkRenderer *renderer, AVFrame *frame)
+{
+    RendererContext *ctx = (RendererContext *)renderer;
+    AVHWFramesContext *src_hw_frame = (AVHWFramesContext *)frame->hw_frames_ctx->data;
+    AVBufferRef *src_dev = src_hw_frame->device_ref;
+    int ret;
+
+    if (ctx->hw_failed)
+        return ctx->hw_failed;
+
+    if (!ctx->hw_device) {
+        ret = av_hwdevice_ctx_create_derived(&ctx->hw_device, AV_HWDEVICE_TYPE_VULKAN, src_dev, 0);
+        if (ret < 0) {
+            av_log(renderer, AV_LOG_ERROR, "Derive hwaccel failed, %s\n", av_err2str(ret));
+            ctx->hw_failed = ret;
+            return ret;
+        }
+    }
+
+    if (!ctx->hw_frame) {
+        AVHWFramesContext *hw_frame;
+
+        ctx->hw_frame = av_hwframe_ctx_alloc(ctx->hw_device);
+        if (!ctx->hw_frame) {
+            ctx->hw_failed = AVERROR(ENOMEM);
+            return AVERROR(ENOMEM);
+        }
+
+        hw_frame = (AVHWFramesContext *)ctx->hw_frame->data;
+        hw_frame->format = AV_PIX_FMT_VULKAN;
+        hw_frame->sw_format = src_hw_frame->sw_format;
+        hw_frame->width = frame->width;
+        hw_frame->height = frame->height;
+
+        ret = av_hwframe_ctx_init(ctx->hw_frame);
+        if (ret < 0) {
+            av_log(renderer, AV_LOG_ERROR, "Create hwframe context failed, %s\n", av_err2str(ret));
+            ctx->hw_failed = ret;
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
+static int transfer_frame(VkRenderer *renderer, AVFrame *frame)
+{
+    RendererContext *ctx = (RendererContext *)renderer;
+    int ret;
+
+    if (!frame->hw_frames_ctx)
+        return 0;
+
+    if (frame->format == AV_PIX_FMT_VULKAN)
+        return 0;
+
+    ret = create_hw(renderer, frame);
+    if (ret < 0 && ret != AVERROR(ENOSYS))
+        return ret;
+
+    // Try map data first
+    av_frame_unref(ctx->vk_frame);
+    if (ctx->hw_frame) {
+        ctx->vk_frame->hw_frames_ctx = av_buffer_ref(ctx->hw_frame);
+        ctx->vk_frame->format = AV_PIX_FMT_VULKAN;
+    }
+    ret = av_hwframe_map(ctx->vk_frame, frame, AV_HWFRAME_MAP_READ);
+    if (!ret) {
+        goto out;
+    }
+
+    if (ret != AVERROR(ENOSYS)) {
+        av_log(NULL, AV_LOG_FATAL, "Map data to vulkan failed: %s\n", av_err2str(ret));
+        return ret;
+    }
+
+    // Try transfer data
+    av_frame_unref(ctx->vk_frame);
+    if (ctx->hw_frame)
+        av_hwframe_get_buffer(ctx->hw_frame, ctx->vk_frame, 0);
+    ret = av_hwframe_transfer_data(ctx->vk_frame, frame, 0);
+    if (ret < 0) {
+        av_log(NULL, AV_LOG_FATAL, "Transfer data to vulkan failed: %s\n", av_err2str(ret));
+        return ret;
+    }
+
+out:
+    ret = av_frame_copy_props(ctx->vk_frame, frame);
+    if (ret < 0)
+        return ret;
+    av_frame_unref(frame);
+    av_frame_move_ref(frame, ctx->vk_frame);
+
+    return 0;
+}
+
 static int display(VkRenderer *renderer, AVFrame *frame)
 {
     struct pl_swapchain_frame swap_frame = {0};
@@ -148,6 +257,10 @@  static int display(VkRenderer *renderer, AVFrame *frame)
     RendererContext *ctx = (RendererContext *)renderer;
     int ret = 0;
 
+    ret = transfer_frame(renderer, frame);
+    if (ret < 0)
+        return ret;
+
     if (!pl_map_avframe_ex(ctx->pl_vk->gpu, &pl_frame, pl_avframe_params(
             .frame = frame,
             .tex = ctx->tex))) {
@@ -194,6 +307,10 @@  static void destroy(VkRenderer *renderer)
     PFN_vkDestroySurfaceKHR vkDestroySurfaceKHR;
     RendererContext *ctx = (RendererContext *)renderer;
 
+    av_buffer_unref(&ctx->hw_frame);
+    av_buffer_unref(&ctx->hw_device);
+    av_frame_free(&ctx->vk_frame);
+
     for (int i = 0; i < FF_ARRAY_ELEMS(ctx->tex); i++)
         pl_tex_destroy(ctx->pl_vk->gpu, &ctx->tex[i]);
     pl_renderer_destroy(&ctx->renderer);