[PATCH v4] avfilter/vf_blackframe: add slice threading
--- libavfilter/vf_blackframe.c | 80 ++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 9 deletions(-) diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..f09d564bf7 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,6 +32,7 @@ #include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h" @@ -45,6 +46,14 @@ typedef struct BlackFrameContext { unsigned int last_keyframe; ///< frame number of the last received key-frame } BlackFrameContext; +typedef struct ThreadData { + const uint8_t *data; // Pointer to the image data + int linesize; // How wide is the memory line + int bthresh; // The black threshold + int width; // Image width + unsigned int *counts; // POINTER to the array where threads write results +} ThreadData; + static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV411P, @@ -55,22 +64,72 @@ static const enum AVPixelFormat pix_fmts[] = { snprintf(buf, sizeof(buf), format, value); \ av_dict_set(metadata, key, buf, 0) +static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) +{ + ThreadData *td = arg; + // --- C90 HARD FIX: ALL DECLARATIONS AT THE VERY TOP --- + int slice_start, slice_end, x, y; + const uint8_t *p; + unsigned int local_nblack = 0; + // ------------------------------------------------------ + + // Safety check + if (!td || !td->data || !td->counts) return 0; + + slice_start = (ctx->inputs[0]->h * jobnr) / nb_jobs; + slice_end = (ctx->inputs[0]->h * (jobnr+1)) / nb_jobs; + + p = td->data + slice_start * td->linesize; + + for (y = slice_start; y < slice_end; y++) { + for (x = 0; x < td->width; x++) + local_nblack += p[x] < td->bthresh; + p += td->linesize; + } + + // Save my private count + td->counts[jobnr] = local_nblack; + return 0; +} + static int filter_frame(AVFilterLink *inlink, AVFrame *frame) { AVFilterContext *ctx = inlink->dst; BlackFrameContext *s = ctx->priv; - int x, i; + // --- C90 HARD FIX: ALL DECLARATIONS AT THE VERY TOP --- int pblack = 0; - uint8_t *p = frame->data[0]; AVDictionary **metadata; char buf[32]; - - for (i = 0; i < frame->height; i++) { - for (x = 0; x < inlink->w; x++) - s->nblack += p[x] < s->bthresh; - p += frame->linesize[0]; + ThreadData td; + int nb_threads; + unsigned int *thread_counts; + int nb_jobs; + int i; // <--- This was the MSVC killer. Moved to top. + // ------------------------------------------------------ + + nb_threads = ff_filter_get_nb_threads(ctx); + + thread_counts = av_calloc(nb_threads, sizeof(*thread_counts)); + if (!thread_counts) { + av_log(ctx, AV_LOG_ERROR, "Failed to allocate thread_counts\n"); + return AVERROR(ENOMEM); } + td.data = frame->data[0]; + td.linesize = frame->linesize[0]; + td.width = inlink->w; + td.bthresh = s->bthresh; + td.counts = thread_counts; + + nb_jobs = FFMIN(frame->height, nb_threads); + + ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs); + + s->nblack = 0; + for (i = 0; i < nb_jobs; i++) { + s->nblack += thread_counts[i]; + } + if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame; @@ -89,6 +148,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) s->frame++; s->nblack = 0; + + av_free(thread_counts); + return ff_filter_frame(inlink->dst->outputs[0], frame); } @@ -118,9 +180,9 @@ const FFFilter ff_vf_blackframe = { .p.name = "blackframe", .p.description = NULL_IF_CONFIG_SMALL("Detect frames that are (almost) black."), .p.priv_class = &blackframe_class, - .p.flags = AVFILTER_FLAG_METADATA_ONLY, + .p.flags = AVFILTER_FLAG_METADATA_ONLY | AVFILTER_FLAG_SLICE_THREADS, .priv_size = sizeof(BlackFrameContext), FILTER_INPUTS(avfilter_vf_blackframe_inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_PIXFMTS_ARRAY(pix_fmts), -}; +}; \ No newline at end of file -- 2.48.1
Hi Raja On Tue, Dec 23, 2025 at 10:14:41PM +0530, Raja Rathour via ffmpeg-devel wrote:
--- libavfilter/vf_blackframe.c | 80 ++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 9 deletions(-)
diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..f09d564bf7 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,6 +32,7 @@
#include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h" @@ -45,6 +46,14 @@ typedef struct BlackFrameContext { unsigned int last_keyframe; ///< frame number of the last received key-frame } BlackFrameContext;
+typedef struct ThreadData { + const uint8_t *data; // Pointer to the image data + int linesize; // How wide is the memory line + int bthresh; // The black threshold + int width; // Image width + unsigned int *counts; // POINTER to the array where threads write results +} ThreadData; + static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV411P, @@ -55,22 +64,72 @@ static const enum AVPixelFormat pix_fmts[] = { snprintf(buf, sizeof(buf), format, value); \ av_dict_set(metadata, key, buf, 0)
+static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) +{ + ThreadData *td = arg;
+ // --- C90 HARD FIX: ALL DECLARATIONS AT THE VERY TOP ---
remove useless comments, like this
+ int slice_start, slice_end, x, y; + const uint8_t *p; + unsigned int local_nblack = 0; + // ------------------------------------------------------ +
+ // Safety check + if (!td || !td->data || !td->counts) return 0;
explain how these are possible conditions
+ + slice_start = (ctx->inputs[0]->h * jobnr) / nb_jobs; + slice_end = (ctx->inputs[0]->h * (jobnr+1)) / nb_jobs; + + p = td->data + slice_start * td->linesize; + + for (y = slice_start; y < slice_end; y++) { + for (x = 0; x < td->width; x++) + local_nblack += p[x] < td->bthresh; + p += td->linesize; + } + + // Save my private count + td->counts[jobnr] = local_nblack; + return 0; +} + static int filter_frame(AVFilterLink *inlink, AVFrame *frame) { AVFilterContext *ctx = inlink->dst; BlackFrameContext *s = ctx->priv; - int x, i; + // --- C90 HARD FIX: ALL DECLARATIONS AT THE VERY TOP --- int pblack = 0; - uint8_t *p = frame->data[0]; AVDictionary **metadata; char buf[32]; - - for (i = 0; i < frame->height; i++) { - for (x = 0; x < inlink->w; x++) - s->nblack += p[x] < s->bthresh; - p += frame->linesize[0]; + ThreadData td; + int nb_threads;
+ unsigned int *thread_counts;
please use a descriptive name that describes whats in the array thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle
This patch adds slice threading support to the blackframe filter, improving performance on multi-core systems. Changes in v5: - Removed meta-comments regarding C90 compliance as requested. - Removed redundant null checks for ThreadData pointers. - Renamed the thread results array to 'nb_black_pixels_per_slice' for clarity. Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 73 ++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index cb566d103d..fb531b8a4c 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -39,19 +39,20 @@ typedef struct BlackFrameContext { const AVClass *class; - int bamount; ///< black amount - int bthresh; ///< black threshold - unsigned int frame; ///< frame number - unsigned int nblack; ///< number of black pixels counted so far - unsigned int last_keyframe; ///< frame number of the last received key-frame + int bamount; + int bthresh; + unsigned int frame; + unsigned int nblack; + unsigned int last_keyframe; } BlackFrameContext; typedef struct ThreadData { - const uint8_t *data; // Pointer to the image data - int linesize; // How wide is the memory line - int bthresh; // The black threshold - int width; // Image width - unsigned int *counts; // POINTER to the array where threads write results + const uint8_t *data; + int linesize; + int bthresh; + int width; + int height; // Added height for cleaner slice math + unsigned int *counts; } ThreadData; static const enum AVPixelFormat pix_fmts[] = { @@ -67,25 +68,21 @@ static const enum AVPixelFormat pix_fmts[] = { static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) { ThreadData *td = arg; - // Calculate vertical slice - int slice_start = (ctx->inputs[0]->h * jobnr) / nb_jobs; - int slice_end = (ctx->inputs[0]->h * (jobnr+1)) / nb_jobs; - - // Safety check for pointers - if (!td || !td->data || !td->counts) return 0; - - const uint8_t *p = td->data + slice_start * td->linesize; - unsigned int local_nblack = 0; + int slice_start = (td->height * jobnr) / nb_jobs; + int slice_end = (td->height * (jobnr+1)) / nb_jobs; int x, y; + const uint8_t *p; + unsigned int black_pixels_count = 0; + + p = td->data + slice_start * td->linesize; for (y = slice_start; y < slice_end; y++) { for (x = 0; x < td->width; x++) - local_nblack += p[x] < td->bthresh; + black_pixels_count += p[x] < td->bthresh; p += td->linesize; } - // Save my private count - td->counts[jobnr] = local_nblack; + td->counts[jobnr] = black_pixels_count; return 0; } @@ -97,39 +94,31 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) AVDictionary **metadata; char buf[32]; ThreadData td; + int nb_threads, nb_jobs, i; + unsigned int *nb_black_pixels_per_slice; + + nb_threads = ff_filter_get_nb_threads(ctx); - // 1. Get thread count - int nb_threads = ff_filter_get_nb_threads(ctx); - - // Allocate memory for thread results - unsigned int *thread_counts = av_calloc(nb_threads, sizeof(*thread_counts)); - if (!thread_counts) { - av_log(ctx, AV_LOG_ERROR, "Failed to allocate thread_counts\n"); + nb_black_pixels_per_slice = av_calloc(nb_threads, sizeof(*nb_black_pixels_per_slice)); + if (!nb_black_pixels_per_slice) return AVERROR(ENOMEM); - } - // 3. Prepare the data package td.data = frame->data[0]; td.linesize = frame->linesize[0]; td.width = inlink->w; + td.height = inlink->h; td.bthresh = s->bthresh; - td.counts = thread_counts; + td.counts = nb_black_pixels_per_slice; - // 4. Run the threads! - // We calculate the exact number of jobs we are about to run - int nb_jobs = FFMIN(frame->height, nb_threads); + nb_jobs = FFMIN(td.height, nb_threads); ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs); - // 5. THE REDUCE STEP: Sum up the results s->nblack = 0; - // Only sum up the jobs that actually ran - for (int i = 0; i < nb_jobs; i++) { - s->nblack += thread_counts[i]; + for (i = 0; i < nb_jobs; i++) { + s->nblack += nb_black_pixels_per_slice[i]; } - // --- FROM HERE DOWN, THE CODE IS THE SAME AS THE ORIGINAL --- - if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame; @@ -149,7 +138,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) s->frame++; s->nblack = 0; - av_free(thread_counts); + av_free(nb_black_pixels_per_slice); return ff_filter_frame(inlink->dst->outputs[0], frame); } -- 2.48.1
v6: Rebased on master to resolve automated Patchwork application failures. This patch adds slice threading support to the blackframe filter, improving performance on multi-core systems. Changes in v5: - Removed meta-comments regarding C90 compliance as requested. - Removed redundant null checks for ThreadData pointers. - Renamed the thread results array to 'nb_black_pixels_per_slice' for clarity. Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 73 ++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index cb566d103d..fb531b8a4c 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -39,19 +39,20 @@ typedef struct BlackFrameContext { const AVClass *class; - int bamount; ///< black amount - int bthresh; ///< black threshold - unsigned int frame; ///< frame number - unsigned int nblack; ///< number of black pixels counted so far - unsigned int last_keyframe; ///< frame number of the last received key-frame + int bamount; + int bthresh; + unsigned int frame; + unsigned int nblack; + unsigned int last_keyframe; } BlackFrameContext; typedef struct ThreadData { - const uint8_t *data; // Pointer to the image data - int linesize; // How wide is the memory line - int bthresh; // The black threshold - int width; // Image width - unsigned int *counts; // POINTER to the array where threads write results + const uint8_t *data; + int linesize; + int bthresh; + int width; + int height; // Added height for cleaner slice math + unsigned int *counts; } ThreadData; static const enum AVPixelFormat pix_fmts[] = { @@ -67,25 +68,21 @@ static const enum AVPixelFormat pix_fmts[] = { static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) { ThreadData *td = arg; - // Calculate vertical slice - int slice_start = (ctx->inputs[0]->h * jobnr) / nb_jobs; - int slice_end = (ctx->inputs[0]->h * (jobnr+1)) / nb_jobs; - - // Safety check for pointers - if (!td || !td->data || !td->counts) return 0; - - const uint8_t *p = td->data + slice_start * td->linesize; - unsigned int local_nblack = 0; + int slice_start = (td->height * jobnr) / nb_jobs; + int slice_end = (td->height * (jobnr+1)) / nb_jobs; int x, y; + const uint8_t *p; + unsigned int black_pixels_count = 0; + + p = td->data + slice_start * td->linesize; for (y = slice_start; y < slice_end; y++) { for (x = 0; x < td->width; x++) - local_nblack += p[x] < td->bthresh; + black_pixels_count += p[x] < td->bthresh; p += td->linesize; } - // Save my private count - td->counts[jobnr] = local_nblack; + td->counts[jobnr] = black_pixels_count; return 0; } @@ -97,39 +94,31 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) AVDictionary **metadata; char buf[32]; ThreadData td; + int nb_threads, nb_jobs, i; + unsigned int *nb_black_pixels_per_slice; + + nb_threads = ff_filter_get_nb_threads(ctx); - // 1. Get thread count - int nb_threads = ff_filter_get_nb_threads(ctx); - - // Allocate memory for thread results - unsigned int *thread_counts = av_calloc(nb_threads, sizeof(*thread_counts)); - if (!thread_counts) { - av_log(ctx, AV_LOG_ERROR, "Failed to allocate thread_counts\n"); + nb_black_pixels_per_slice = av_calloc(nb_threads, sizeof(*nb_black_pixels_per_slice)); + if (!nb_black_pixels_per_slice) return AVERROR(ENOMEM); - } - // 3. Prepare the data package td.data = frame->data[0]; td.linesize = frame->linesize[0]; td.width = inlink->w; + td.height = inlink->h; td.bthresh = s->bthresh; - td.counts = thread_counts; + td.counts = nb_black_pixels_per_slice; - // 4. Run the threads! - // We calculate the exact number of jobs we are about to run - int nb_jobs = FFMIN(frame->height, nb_threads); + nb_jobs = FFMIN(td.height, nb_threads); ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs); - // 5. THE REDUCE STEP: Sum up the results s->nblack = 0; - // Only sum up the jobs that actually ran - for (int i = 0; i < nb_jobs; i++) { - s->nblack += thread_counts[i]; + for (i = 0; i < nb_jobs; i++) { + s->nblack += nb_black_pixels_per_slice[i]; } - // --- FROM HERE DOWN, THE CODE IS THE SAME AS THE ORIGINAL --- - if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame; @@ -149,7 +138,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) s->frame++; s->nblack = 0; - av_free(thread_counts); + av_free(nb_black_pixels_per_slice); return ff_filter_frame(inlink->dst->outputs[0], frame); } -- 2.48.1
On Tue, Dec 30, 2025 at 11:23:44PM +0530, Raja Rathour via ffmpeg-devel wrote:
v6: Rebased on master to resolve automated Patchwork application failures. This patch adds slice threading support to the blackframe filter, improving performance on multi-core systems.
Changes in v5: - Removed meta-comments regarding C90 compliance as requested. - Removed redundant null checks for ThreadData pointers. - Renamed the thread results array to 'nb_black_pixels_per_slice' for clarity.
Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 73 ++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 42 deletions(-)
This patch is on top of the previous patchset, not a patchset that has the suggested changes done to it [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB During times of universal deceit, telling the truth becomes a revolutionary act. -- George Orwell
v7: Squashed all changes into a single clean commit rebased on master. Thank you for the guidance on the patch structure and history. This version is a standalone replacement of the original filter logic and incorporates all requested technical improvements: - Renamed the results array to 'nb_black_pixels_per_slice' for clarity. - Removed redundant comments and safety checks. - Fixed the patchset structure to ensure it applies cleanly as a single unit. Thank you, Raja Rathour Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 79 ++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 14 deletions(-) diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..fb531b8a4c 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,19 +32,29 @@ #include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h" typedef struct BlackFrameContext { const AVClass *class; - int bamount; ///< black amount - int bthresh; ///< black threshold - unsigned int frame; ///< frame number - unsigned int nblack; ///< number of black pixels counted so far - unsigned int last_keyframe; ///< frame number of the last received key-frame + int bamount; + int bthresh; + unsigned int frame; + unsigned int nblack; + unsigned int last_keyframe; } BlackFrameContext; +typedef struct ThreadData { + const uint8_t *data; + int linesize; + int bthresh; + int width; + int height; // Added height for cleaner slice math + unsigned int *counts; +} ThreadData; + static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV411P, @@ -55,22 +65,60 @@ static const enum AVPixelFormat pix_fmts[] = { snprintf(buf, sizeof(buf), format, value); \ av_dict_set(metadata, key, buf, 0) +static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) +{ + ThreadData *td = arg; + int slice_start = (td->height * jobnr) / nb_jobs; + int slice_end = (td->height * (jobnr+1)) / nb_jobs; + int x, y; + const uint8_t *p; + unsigned int black_pixels_count = 0; + + p = td->data + slice_start * td->linesize; + + for (y = slice_start; y < slice_end; y++) { + for (x = 0; x < td->width; x++) + black_pixels_count += p[x] < td->bthresh; + p += td->linesize; + } + + td->counts[jobnr] = black_pixels_count; + return 0; +} + static int filter_frame(AVFilterLink *inlink, AVFrame *frame) { AVFilterContext *ctx = inlink->dst; BlackFrameContext *s = ctx->priv; - int x, i; int pblack = 0; - uint8_t *p = frame->data[0]; AVDictionary **metadata; char buf[32]; + ThreadData td; + int nb_threads, nb_jobs, i; + unsigned int *nb_black_pixels_per_slice; + + nb_threads = ff_filter_get_nb_threads(ctx); + + nb_black_pixels_per_slice = av_calloc(nb_threads, sizeof(*nb_black_pixels_per_slice)); + if (!nb_black_pixels_per_slice) + return AVERROR(ENOMEM); + + td.data = frame->data[0]; + td.linesize = frame->linesize[0]; + td.width = inlink->w; + td.height = inlink->h; + td.bthresh = s->bthresh; + td.counts = nb_black_pixels_per_slice; + + nb_jobs = FFMIN(td.height, nb_threads); + + ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs); - for (i = 0; i < frame->height; i++) { - for (x = 0; x < inlink->w; x++) - s->nblack += p[x] < s->bthresh; - p += frame->linesize[0]; + s->nblack = 0; + for (i = 0; i < nb_jobs; i++) { + s->nblack += nb_black_pixels_per_slice[i]; } - + if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame; @@ -89,6 +137,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) s->frame++; s->nblack = 0; + + av_free(nb_black_pixels_per_slice); + return ff_filter_frame(inlink->dst->outputs[0], frame); } @@ -118,9 +169,9 @@ const FFFilter ff_vf_blackframe = { .p.name = "blackframe", .p.description = NULL_IF_CONFIG_SMALL("Detect frames that are (almost) black."), .p.priv_class = &blackframe_class, - .p.flags = AVFILTER_FLAG_METADATA_ONLY, + .p.flags = AVFILTER_FLAG_METADATA_ONLY | AVFILTER_FLAG_SLICE_THREADS, .priv_size = sizeof(BlackFrameContext), FILTER_INPUTS(avfilter_vf_blackframe_inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_PIXFMTS_ARRAY(pix_fmts), -}; +}; \ No newline at end of file -- 2.48.1
On Wed, Dec 31, 2025 at 01:10:18AM +0530, Raja Rathour via ffmpeg-devel wrote:
v7: Squashed all changes into a single clean commit rebased on master.
Thank you for the guidance on the patch structure and history. This version is a standalone replacement of the original filter logic and incorporates all requested technical improvements:
- Renamed the results array to 'nb_black_pixels_per_slice' for clarity. - Removed redundant comments and safety checks. - Fixed the patchset structure to ensure it applies cleanly as a single unit.
Thank you, Raja Rathour
Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 79 ++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 14 deletions(-)
diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..fb531b8a4c 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,19 +32,29 @@
#include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h"
typedef struct BlackFrameContext { const AVClass *class; - int bamount; ///< black amount - int bthresh; ///< black threshold - unsigned int frame; ///< frame number - unsigned int nblack; ///< number of black pixels counted so far - unsigned int last_keyframe; ///< frame number of the last received key-frame + int bamount; + int bthresh; + unsigned int frame; + unsigned int nblack; + unsigned int last_keyframe; } BlackFrameContext;
unrelated to threads
+typedef struct ThreadData { + const uint8_t *data; + int linesize; + int bthresh; + int width;
+ int height; // Added height for cleaner slice math
///< [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
v8: Restored original struct comments (unrelated to threads) and updated height comment to Doxygen style (///<) as requested. Thank you for the detailed feedback on the patch structure and project style. Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 69 ++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..5b198e6594 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,6 +32,7 @@ #include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h" @@ -45,6 +46,15 @@ typedef struct BlackFrameContext { unsigned int last_keyframe; ///< frame number of the last received key-frame } BlackFrameContext; +typedef struct ThreadData { + const uint8_t *data; + int linesize; + int bthresh; + int width; + int height; ///< height for cleaner slice math + unsigned int *counts; +} ThreadData; + static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV411P, @@ -55,22 +65,60 @@ static const enum AVPixelFormat pix_fmts[] = { snprintf(buf, sizeof(buf), format, value); \ av_dict_set(metadata, key, buf, 0) +static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) +{ + ThreadData *td = arg; + int slice_start = (td->height * jobnr) / nb_jobs; + int slice_end = (td->height * (jobnr+1)) / nb_jobs; + int x, y; + const uint8_t *p; + unsigned int black_pixels_count = 0; + + p = td->data + slice_start * td->linesize; + + for (y = slice_start; y < slice_end; y++) { + for (x = 0; x < td->width; x++) + black_pixels_count += p[x] < td->bthresh; + p += td->linesize; + } + + td->counts[jobnr] = black_pixels_count; + return 0; +} + static int filter_frame(AVFilterLink *inlink, AVFrame *frame) { AVFilterContext *ctx = inlink->dst; BlackFrameContext *s = ctx->priv; - int x, i; int pblack = 0; - uint8_t *p = frame->data[0]; AVDictionary **metadata; char buf[32]; + ThreadData td; + int nb_threads, nb_jobs, i; + unsigned int *nb_black_pixels_per_slice; + + nb_threads = ff_filter_get_nb_threads(ctx); + + nb_black_pixels_per_slice = av_calloc(nb_threads, sizeof(*nb_black_pixels_per_slice)); + if (!nb_black_pixels_per_slice) + return AVERROR(ENOMEM); + + td.data = frame->data[0]; + td.linesize = frame->linesize[0]; + td.width = inlink->w; + td.height = inlink->h; + td.bthresh = s->bthresh; + td.counts = nb_black_pixels_per_slice; + + nb_jobs = FFMIN(td.height, nb_threads); + + ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs); - for (i = 0; i < frame->height; i++) { - for (x = 0; x < inlink->w; x++) - s->nblack += p[x] < s->bthresh; - p += frame->linesize[0]; + s->nblack = 0; + for (i = 0; i < nb_jobs; i++) { + s->nblack += nb_black_pixels_per_slice[i]; } - + if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame; @@ -89,6 +137,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) s->frame++; s->nblack = 0; + + av_free(nb_black_pixels_per_slice); + return ff_filter_frame(inlink->dst->outputs[0], frame); } @@ -118,9 +169,9 @@ const FFFilter ff_vf_blackframe = { .p.name = "blackframe", .p.description = NULL_IF_CONFIG_SMALL("Detect frames that are (almost) black."), .p.priv_class = &blackframe_class, - .p.flags = AVFILTER_FLAG_METADATA_ONLY, + .p.flags = AVFILTER_FLAG_METADATA_ONLY | AVFILTER_FLAG_SLICE_THREADS, .priv_size = sizeof(BlackFrameContext), FILTER_INPUTS(avfilter_vf_blackframe_inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_PIXFMTS_ARRAY(pix_fmts), -}; +}; \ No newline at end of file -- 2.48.1
On Thu, 1 Jan 2026, Raja Rathour via ffmpeg-devel wrote:
v8: Restored original struct comments (unrelated to threads) and updated height comment to Doxygen style (///<) as requested.
Thank you for the detailed feedback on the patch structure and project style.
Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 69 ++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..5b198e6594 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,6 +32,7 @@
#include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h" @@ -45,6 +46,15 @@ typedef struct BlackFrameContext { unsigned int last_keyframe; ///< frame number of the last received key-frame } BlackFrameContext;
+typedef struct ThreadData { + const uint8_t *data; + int linesize; + int bthresh; + int width; + int height; ///< height for cleaner slice math + unsigned int *counts;
You can use a single stdatomic atomic int for this and avoid the allocation. Also I suggest you open a pull request at code.ffmpeg.org. Regards, Marton
+} ThreadData; + static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV411P, @@ -55,22 +65,60 @@ static const enum AVPixelFormat pix_fmts[] = { snprintf(buf, sizeof(buf), format, value); \ av_dict_set(metadata, key, buf, 0)
+static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) +{ + ThreadData *td = arg; + int slice_start = (td->height * jobnr) / nb_jobs; + int slice_end = (td->height * (jobnr+1)) / nb_jobs; + int x, y; + const uint8_t *p; + unsigned int black_pixels_count = 0; + + p = td->data + slice_start * td->linesize; + + for (y = slice_start; y < slice_end; y++) { + for (x = 0; x < td->width; x++) + black_pixels_count += p[x] < td->bthresh; + p += td->linesize; + } + + td->counts[jobnr] = black_pixels_count; + return 0; +} + static int filter_frame(AVFilterLink *inlink, AVFrame *frame) { AVFilterContext *ctx = inlink->dst; BlackFrameContext *s = ctx->priv; - int x, i; int pblack = 0; - uint8_t *p = frame->data[0]; AVDictionary **metadata; char buf[32]; + ThreadData td; + int nb_threads, nb_jobs, i; + unsigned int *nb_black_pixels_per_slice; + + nb_threads = ff_filter_get_nb_threads(ctx); + + nb_black_pixels_per_slice = av_calloc(nb_threads, sizeof(*nb_black_pixels_per_slice)); + if (!nb_black_pixels_per_slice) + return AVERROR(ENOMEM); + + td.data = frame->data[0]; + td.linesize = frame->linesize[0]; + td.width = inlink->w; + td.height = inlink->h; + td.bthresh = s->bthresh; + td.counts = nb_black_pixels_per_slice; + + nb_jobs = FFMIN(td.height, nb_threads); + + ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs);
- for (i = 0; i < frame->height; i++) { - for (x = 0; x < inlink->w; x++) - s->nblack += p[x] < s->bthresh; - p += frame->linesize[0]; + s->nblack = 0; + for (i = 0; i < nb_jobs; i++) { + s->nblack += nb_black_pixels_per_slice[i]; } - + if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame;
@@ -89,6 +137,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
s->frame++; s->nblack = 0; + + av_free(nb_black_pixels_per_slice); + return ff_filter_frame(inlink->dst->outputs[0], frame); }
@@ -118,9 +169,9 @@ const FFFilter ff_vf_blackframe = { .p.name = "blackframe", .p.description = NULL_IF_CONFIG_SMALL("Detect frames that are (almost) black."), .p.priv_class = &blackframe_class, - .p.flags = AVFILTER_FLAG_METADATA_ONLY, + .p.flags = AVFILTER_FLAG_METADATA_ONLY | AVFILTER_FLAG_SLICE_THREADS, .priv_size = sizeof(BlackFrameContext), FILTER_INPUTS(avfilter_vf_blackframe_inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_PIXFMTS_ARRAY(pix_fmts), -}; +}; \ No newline at end of file -- 2.48.1
_______________________________________________ ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
Hi Marton, Thank you for the feedback. As you suggested, I have replaced the heap allocation (av_calloc) with a single stdatomic (atomic_uint) counter. This simplifies the logic and removes the memory overhead. I have also opened the formal Pull Request on code.ffmpeg.org as requested: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21408 Summary of changes in v9: - Switched to stdatomic for the global pixel counter. - Enabled AVFILTER_FLAG_SLICE_THREADS. - Cleaned up the commit history to include only this feature. - Verified that all FATE tests are passing on the PR. Best regards, Raja Rathour On Mon, Jan 5, 2026 at 3:46 AM Marton Balint via ffmpeg-devel < ffmpeg-devel@ffmpeg.org> wrote:
On Thu, 1 Jan 2026, Raja Rathour via ffmpeg-devel wrote:
v8: Restored original struct comments (unrelated to threads) and updated height comment to Doxygen style (///<) as requested.
Thank you for the detailed feedback on the patch structure and project style.
Signed-off-by: Raja Rathour <imraja729@gmail.com> --- libavfilter/vf_blackframe.c | 69 ++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/libavfilter/vf_blackframe.c b/libavfilter/vf_blackframe.c index f0aa53e133..5b198e6594 100644 --- a/libavfilter/vf_blackframe.c +++ b/libavfilter/vf_blackframe.c @@ -32,6 +32,7 @@
#include "libavutil/internal.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "avfilter.h" #include "filters.h" #include "video.h" @@ -45,6 +46,15 @@ typedef struct BlackFrameContext { unsigned int last_keyframe; ///< frame number of the last received key-frame } BlackFrameContext;
+typedef struct ThreadData { + const uint8_t *data; + int linesize; + int bthresh; + int width; + int height; ///< height for cleaner slice math + unsigned int *counts;
You can use a single stdatomic atomic int for this and avoid the allocation.
Also I suggest you open a pull request at code.ffmpeg.org.
Regards, Marton
+} ThreadData; + static const enum AVPixelFormat pix_fmts[] = { AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NV12, AV_PIX_FMT_NV21, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV411P, @@ -55,22 +65,60 @@ static const enum AVPixelFormat pix_fmts[] = { snprintf(buf, sizeof(buf), format, value); \ av_dict_set(metadata, key, buf, 0)
+static int blackframe_slice(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs) +{ + ThreadData *td = arg; + int slice_start = (td->height * jobnr) / nb_jobs; + int slice_end = (td->height * (jobnr+1)) / nb_jobs; + int x, y; + const uint8_t *p; + unsigned int black_pixels_count = 0; + + p = td->data + slice_start * td->linesize; + + for (y = slice_start; y < slice_end; y++) { + for (x = 0; x < td->width; x++) + black_pixels_count += p[x] < td->bthresh; + p += td->linesize; + } + + td->counts[jobnr] = black_pixels_count; + return 0; +} + static int filter_frame(AVFilterLink *inlink, AVFrame *frame) { AVFilterContext *ctx = inlink->dst; BlackFrameContext *s = ctx->priv; - int x, i; int pblack = 0; - uint8_t *p = frame->data[0]; AVDictionary **metadata; char buf[32]; + ThreadData td; + int nb_threads, nb_jobs, i; + unsigned int *nb_black_pixels_per_slice; + + nb_threads = ff_filter_get_nb_threads(ctx); + + nb_black_pixels_per_slice = av_calloc(nb_threads, sizeof(*nb_black_pixels_per_slice)); + if (!nb_black_pixels_per_slice) + return AVERROR(ENOMEM); + + td.data = frame->data[0]; + td.linesize = frame->linesize[0]; + td.width = inlink->w; + td.height = inlink->h; + td.bthresh = s->bthresh; + td.counts = nb_black_pixels_per_slice; + + nb_jobs = FFMIN(td.height, nb_threads); + + ff_filter_execute(ctx, blackframe_slice, &td, NULL, nb_jobs);
- for (i = 0; i < frame->height; i++) { - for (x = 0; x < inlink->w; x++) - s->nblack += p[x] < s->bthresh; - p += frame->linesize[0]; + s->nblack = 0; + for (i = 0; i < nb_jobs; i++) { + s->nblack += nb_black_pixels_per_slice[i]; } - + if (frame->flags & AV_FRAME_FLAG_KEY) s->last_keyframe = s->frame;
@@ -89,6 +137,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
s->frame++; s->nblack = 0; + + av_free(nb_black_pixels_per_slice); + return ff_filter_frame(inlink->dst->outputs[0], frame); }
@@ -118,9 +169,9 @@ const FFFilter ff_vf_blackframe = { .p.name = "blackframe", .p.description = NULL_IF_CONFIG_SMALL("Detect frames that are (almost) black."), .p.priv_class = &blackframe_class, - .p.flags = AVFILTER_FLAG_METADATA_ONLY, + .p.flags = AVFILTER_FLAG_METADATA_ONLY | AVFILTER_FLAG_SLICE_THREADS, .priv_size = sizeof(BlackFrameContext), FILTER_INPUTS(avfilter_vf_blackframe_inputs), FILTER_OUTPUTS(ff_video_default_filterpad), FILTER_PIXFMTS_ARRAY(pix_fmts), -}; +}; \ No newline at end of file -- 2.48.1
_______________________________________________ ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
_______________________________________________ ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
participants (3)
-
Marton Balint -
Michael Niedermayer -
Raja Rathour