* [FFmpeg-devel] [RFC PATCH] avfilter/video: Protect frame_pool from multi-threads access
@ 2024-05-10 8:57 Zhao Zhili
2024-05-11 4:45 ` Zhao Zhili
0 siblings, 1 reply; 3+ messages in thread
From: Zhao Zhili @ 2024-05-10 8:57 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Zhao Zhili
From: Zhao Zhili <zhilizhao@tencent.com>
Fix crash with
./ffplay -f lavfi -i movie=foo.mp4,drawtext=text=bar
---
libavfilter/avfilter.c | 2 ++
libavfilter/avfilter_internal.h | 2 ++
libavfilter/video.c | 14 ++++++++------
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 049e4f62ca..44da809c15 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -176,6 +176,7 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad,
li = av_mallocz(sizeof(*li));
if (!li)
return AVERROR(ENOMEM);
+ ff_mutex_init(&li->frame_pool_mutex, NULL);
link = &li->l;
src->outputs[srcpad] = dst->inputs[dstpad] = link;
@@ -203,6 +204,7 @@ static void link_free(AVFilterLink **link)
ff_framequeue_free(&li->fifo);
ff_frame_pool_uninit(&li->frame_pool);
+ ff_mutex_destroy(&li->frame_pool_mutex);
av_channel_layout_uninit(&(*link)->ch_layout);
av_freep(link);
diff --git a/libavfilter/avfilter_internal.h b/libavfilter/avfilter_internal.h
index 2c31c3e7de..eae3a699c3 100644
--- a/libavfilter/avfilter_internal.h
+++ b/libavfilter/avfilter_internal.h
@@ -27,6 +27,7 @@
#include <stdint.h>
+#include "libavutil/thread.h"
#include "avfilter.h"
#include "framequeue.h"
@@ -34,6 +35,7 @@ typedef struct FilterLinkInternal {
AVFilterLink l;
struct FFFramePool *frame_pool;
+ AVMutex frame_pool_mutex;
/**
* Queue of frames waiting to be filtered.
diff --git a/libavfilter/video.c b/libavfilter/video.c
index 89d0797ab5..30eba76cbd 100644
--- a/libavfilter/video.c
+++ b/libavfilter/video.c
@@ -70,17 +70,17 @@ AVFrame *ff_default_get_video_buffer2(AVFilterLink *link, int w, int h, int alig
return frame;
}
+ ff_mutex_lock(&li->frame_pool_mutex);
if (!li->frame_pool) {
li->frame_pool = ff_frame_pool_video_init(av_buffer_allocz, w, h,
link->format, align);
if (!li->frame_pool)
- return NULL;
+ goto out;
} else {
if (ff_frame_pool_get_video_config(li->frame_pool,
&pool_width, &pool_height,
- &pool_format, &pool_align) < 0) {
- return NULL;
- }
+ &pool_format, &pool_align) < 0)
+ goto out;
if (pool_width != w || pool_height != h ||
pool_format != link->format || pool_align != align) {
@@ -89,18 +89,20 @@ AVFrame *ff_default_get_video_buffer2(AVFilterLink *link, int w, int h, int alig
li->frame_pool = ff_frame_pool_video_init(av_buffer_allocz, w, h,
link->format, align);
if (!li->frame_pool)
- return NULL;
+ goto out;
}
}
frame = ff_frame_pool_get(li->frame_pool);
if (!frame)
- return NULL;
+ goto out;
frame->sample_aspect_ratio = link->sample_aspect_ratio;
frame->colorspace = link->colorspace;
frame->color_range = link->color_range;
+out:
+ ff_mutex_unlock(&li->frame_pool_mutex);
return frame;
}
--
2.42.0
_______________________________________________
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] 3+ messages in thread* Re: [FFmpeg-devel] [RFC PATCH] avfilter/video: Protect frame_pool from multi-threads access
2024-05-10 8:57 [FFmpeg-devel] [RFC PATCH] avfilter/video: Protect frame_pool from multi-threads access Zhao Zhili
@ 2024-05-11 4:45 ` Zhao Zhili
2024-05-23 7:44 ` Anton Khirnov
0 siblings, 1 reply; 3+ messages in thread
From: Zhao Zhili @ 2024-05-11 4:45 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On May 10, 2024, at 16:57, Zhao Zhili <quinkblack@foxmail.com> wrote:
>
> From: Zhao Zhili <zhilizhao@tencent.com>
>
> Fix crash with
> ./ffplay -f lavfi -i movie=foo.mp4,drawtext=text=bar
This is an RFC because I’m not sure whether it’s a valid use case or not to call
ff_default_get_video_buffer2 from multiple threads. On one hand, it’s rare used
with multithreads. On the other hand, it can be hidden deep and dangerous.
> ---
> libavfilter/avfilter.c | 2 ++
> libavfilter/avfilter_internal.h | 2 ++
> libavfilter/video.c | 14 ++++++++------
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 049e4f62ca..44da809c15 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -176,6 +176,7 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad,
> li = av_mallocz(sizeof(*li));
> if (!li)
> return AVERROR(ENOMEM);
> + ff_mutex_init(&li->frame_pool_mutex, NULL);
> link = &li->l;
>
> src->outputs[srcpad] = dst->inputs[dstpad] = link;
> @@ -203,6 +204,7 @@ static void link_free(AVFilterLink **link)
>
> ff_framequeue_free(&li->fifo);
> ff_frame_pool_uninit(&li->frame_pool);
> + ff_mutex_destroy(&li->frame_pool_mutex);
> av_channel_layout_uninit(&(*link)->ch_layout);
>
> av_freep(link);
> diff --git a/libavfilter/avfilter_internal.h b/libavfilter/avfilter_internal.h
> index 2c31c3e7de..eae3a699c3 100644
> --- a/libavfilter/avfilter_internal.h
> +++ b/libavfilter/avfilter_internal.h
> @@ -27,6 +27,7 @@
>
> #include <stdint.h>
>
> +#include "libavutil/thread.h"
> #include "avfilter.h"
> #include "framequeue.h"
>
> @@ -34,6 +35,7 @@ typedef struct FilterLinkInternal {
> AVFilterLink l;
>
> struct FFFramePool *frame_pool;
> + AVMutex frame_pool_mutex;
>
> /**
> * Queue of frames waiting to be filtered.
> diff --git a/libavfilter/video.c b/libavfilter/video.c
> index 89d0797ab5..30eba76cbd 100644
> --- a/libavfilter/video.c
> +++ b/libavfilter/video.c
> @@ -70,17 +70,17 @@ AVFrame *ff_default_get_video_buffer2(AVFilterLink *link, int w, int h, int alig
> return frame;
> }
>
> + ff_mutex_lock(&li->frame_pool_mutex);
> if (!li->frame_pool) {
> li->frame_pool = ff_frame_pool_video_init(av_buffer_allocz, w, h,
> link->format, align);
> if (!li->frame_pool)
> - return NULL;
> + goto out;
> } else {
> if (ff_frame_pool_get_video_config(li->frame_pool,
> &pool_width, &pool_height,
> - &pool_format, &pool_align) < 0) {
> - return NULL;
> - }
> + &pool_format, &pool_align) < 0)
> + goto out;
>
> if (pool_width != w || pool_height != h ||
> pool_format != link->format || pool_align != align) {
> @@ -89,18 +89,20 @@ AVFrame *ff_default_get_video_buffer2(AVFilterLink *link, int w, int h, int alig
> li->frame_pool = ff_frame_pool_video_init(av_buffer_allocz, w, h,
> link->format, align);
> if (!li->frame_pool)
> - return NULL;
> + goto out;
> }
> }
>
> frame = ff_frame_pool_get(li->frame_pool);
> if (!frame)
> - return NULL;
> + goto out;
>
> frame->sample_aspect_ratio = link->sample_aspect_ratio;
> frame->colorspace = link->colorspace;
> frame->color_range = link->color_range;
>
> +out:
> + ff_mutex_unlock(&li->frame_pool_mutex);
> return frame;
> }
>
> --
> 2.42.0
>
_______________________________________________
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] 3+ messages in thread* Re: [FFmpeg-devel] [RFC PATCH] avfilter/video: Protect frame_pool from multi-threads access
2024-05-11 4:45 ` Zhao Zhili
@ 2024-05-23 7:44 ` Anton Khirnov
0 siblings, 0 replies; 3+ messages in thread
From: Anton Khirnov @ 2024-05-23 7:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Zhao Zhili (2024-05-11 06:45:12)
>
> > On May 10, 2024, at 16:57, Zhao Zhili <quinkblack@foxmail.com> wrote:
> >
> > From: Zhao Zhili <zhilizhao@tencent.com>
> >
> > Fix crash with
> > ./ffplay -f lavfi -i movie=foo.mp4,drawtext=text=bar
>
> This is an RFC because I’m not sure whether it’s a valid use case or not to call
> ff_default_get_video_buffer2 from multiple threads. On one hand, it’s rare used
> with multithreads. On the other hand, it can be hidden deep and dangerous.
Naively it sounds like something that should not happen, since lavfi
only has slice threading and slice threads should not allocate frames.
In your example it seems to happen because of frame threading in the
movie source, but that should ideally be handled inside that filter.
--
Anton Khirnov
_______________________________________________
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] 3+ messages in thread
end of thread, other threads:[~2024-05-23 7:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-10 8:57 [FFmpeg-devel] [RFC PATCH] avfilter/video: Protect frame_pool from multi-threads access Zhao Zhili
2024-05-11 4:45 ` Zhao Zhili
2024-05-23 7:44 ` Anton Khirnov
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