Message ID | tencent_A8311C468B86661DD09F26DB27DA2ABAD306@qq.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v2] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
andriy/make_armv7_RPi4 | success | Make finished |
andriy/make_fate_armv7_RPi4 | success | Make fate finished |
andriy/make_aarch64_jetson | success | Make finished |
andriy/make_fate_aarch64_jetson | success | Make fate finished |
27 Jan 2022, 09:38 by quinkblack@foxmail.com: > MoltenVK doesn't reset instance when CreateInstance() failed, > then DestroyInstance() leads to segmentation fault. MoltenVK's bug > has been fixed by [1], which doesn't available on homebrew yet. > Regardless MoltenVK's bug, we shouldn't call DestroyInstance() > in the case of CreateInstance() failed, so reset instance making > sense. > > [1] https://github.com/KhronosGroup/MoltenVK/commit/86a1fbdb8 > --- > libavutil/hwcontext_vulkan.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c > index 2e219511c9..d3bd37a6a7 100644 > --- a/libavutil/hwcontext_vulkan.c > +++ b/libavutil/hwcontext_vulkan.c > @@ -719,6 +719,8 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts) > if (ret != VK_SUCCESS) { > av_log(ctx, AV_LOG_ERROR, "Instance creation failure: %s\n", > vk_ret2str(ret)); > + /* Workaround MoltenVK's bug which doesn't reset instance. */ > + hwctx->inst = VK_NULL_HANDLE; > err = AVERROR_EXTERNAL; > goto fail; > } > Not a fan of working around this implementation bug. Couldn't homebrew just update?
> On Jan 27, 2022, at 5:07 PM, Lynne <dev@lynne.ee> wrote: > > 27 Jan 2022, 09:38 by quinkblack@foxmail.com: > >> MoltenVK doesn't reset instance when CreateInstance() failed, >> then DestroyInstance() leads to segmentation fault. MoltenVK's bug >> has been fixed by [1], which doesn't available on homebrew yet. >> Regardless MoltenVK's bug, we shouldn't call DestroyInstance() >> in the case of CreateInstance() failed, so reset instance making >> sense. >> >> [1] https://github.com/KhronosGroup/MoltenVK/commit/86a1fbdb8 >> --- >> libavutil/hwcontext_vulkan.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c >> index 2e219511c9..d3bd37a6a7 100644 >> --- a/libavutil/hwcontext_vulkan.c >> +++ b/libavutil/hwcontext_vulkan.c >> @@ -719,6 +719,8 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts) >> if (ret != VK_SUCCESS) { >> av_log(ctx, AV_LOG_ERROR, "Instance creation failure: %s\n", >> vk_ret2str(ret)); >> + /* Workaround MoltenVK's bug which doesn't reset instance. */ >> + hwctx->inst = VK_NULL_HANDLE; >> err = AVERROR_EXTERNAL; >> goto fail; >> } >> > > Not a fan of working around this implementation bug. > Couldn't homebrew just update? I don’t have strong opinion on the patch, feel free to drop it. It’s a cheap workaround and it takes a lot of time for me to figure out the crash. Other people may confront the same issue, then the patch can be seen as a reference. > _______________________________________________ > 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 --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index 2e219511c9..d3bd37a6a7 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -719,6 +719,8 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts) if (ret != VK_SUCCESS) { av_log(ctx, AV_LOG_ERROR, "Instance creation failure: %s\n", vk_ret2str(ret)); + /* Workaround MoltenVK's bug which doesn't reset instance. */ + hwctx->inst = VK_NULL_HANDLE; err = AVERROR_EXTERNAL; goto fail; }