* [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault
@ 2022-01-27 8:11 Zhao Zhili
2022-01-27 8:18 ` Wu, Jianhua
0 siblings, 1 reply; 6+ messages in thread
From: Zhao Zhili @ 2022-01-27 8:11 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Zhao Zhili
MoltenVK doesn't reset instance pointer 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..ac8e00003a 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 pointer. */
+ hwctx->inst = (VkInstance) { 0 };
err = AVERROR_EXTERNAL;
goto fail;
}
--
2.31.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault
2022-01-27 8:11 [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault Zhao Zhili
@ 2022-01-27 8:18 ` Wu, Jianhua
2022-01-27 8:38 ` [FFmpeg-devel] [PATCH v2] " Zhao Zhili
2022-01-27 8:42 ` [FFmpeg-devel] [PATCH] " "zhilizhao(赵志立)"
0 siblings, 2 replies; 6+ messages in thread
From: Wu, Jianhua @ 2022-01-27 8:18 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Zhao Zhili wrote:
> Sent: Thursday, January 27, 2022 4:11 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Zhao Zhili <quinkblack@foxmail.com>
> Subject: [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround
> MoltenVK's bug which leads to segmentation fault
>
> MoltenVK doesn't reset instance pointer 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..ac8e00003a 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 pointer.
> */
> + hwctx->inst = (VkInstance) { 0 };
Hi,
It's no need to use the explicit cast and use hwctx->inst = VK_NULL_HANDLE instead, which is the null context defined by Vulkan spec.
Thanks,
Jianhua
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH v2] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault
2022-01-27 8:18 ` Wu, Jianhua
@ 2022-01-27 8:38 ` Zhao Zhili
2022-01-27 9:07 ` Lynne
2022-01-27 8:42 ` [FFmpeg-devel] [PATCH] " "zhilizhao(赵志立)"
1 sibling, 1 reply; 6+ messages in thread
From: Zhao Zhili @ 2022-01-27 8:38 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Zhao Zhili
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;
}
--
2.31.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault
2022-01-27 8:18 ` Wu, Jianhua
2022-01-27 8:38 ` [FFmpeg-devel] [PATCH v2] " Zhao Zhili
@ 2022-01-27 8:42 ` "zhilizhao(赵志立)"
1 sibling, 0 replies; 6+ messages in thread
From: "zhilizhao(赵志立)" @ 2022-01-27 8:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On Jan 27, 2022, at 4:18 PM, Wu, Jianhua <jianhua.wu-at-intel.com@ffmpeg.org> wrote:
>
> Zhao Zhili wrote:
>> Sent: Thursday, January 27, 2022 4:11 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: Zhao Zhili <quinkblack@foxmail.com>
>> Subject: [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround
>> MoltenVK's bug which leads to segmentation fault
>>
>> MoltenVK doesn't reset instance pointer 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..ac8e00003a 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 pointer.
>> */
>> + hwctx->inst = (VkInstance) { 0 };
>
> Hi,
>
> It's no need to use the explicit cast and use hwctx->inst = VK_NULL_HANDLE instead, which is the null context defined by Vulkan spec.
I didn’t notice that, thanks! Patch v2:
http://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/292181.html
>
> Thanks,
> Jianhua
>
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault
2022-01-27 8:38 ` [FFmpeg-devel] [PATCH v2] " Zhao Zhili
@ 2022-01-27 9:07 ` Lynne
2022-01-27 9:43 ` "zhilizhao(赵志立)"
0 siblings, 1 reply; 6+ messages in thread
From: Lynne @ 2022-01-27 9:07 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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?
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault
2022-01-27 9:07 ` Lynne
@ 2022-01-27 9:43 ` "zhilizhao(赵志立)"
0 siblings, 0 replies; 6+ messages in thread
From: "zhilizhao(赵志立)" @ 2022-01-27 9:43 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-27 9:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-27 8:11 [FFmpeg-devel] [PATCH] hwcontext_vulkan: workaround MoltenVK's bug which leads to segmentation fault Zhao Zhili
2022-01-27 8:18 ` Wu, Jianhua
2022-01-27 8:38 ` [FFmpeg-devel] [PATCH v2] " Zhao Zhili
2022-01-27 9:07 ` Lynne
2022-01-27 9:43 ` "zhilizhao(赵志立)"
2022-01-27 8:42 ` [FFmpeg-devel] [PATCH] " "zhilizhao(赵志立)"
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git