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 < [email protected]> 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 <[email protected]> > > --- > > 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 -- [email protected] > > To unsubscribe send an email to [email protected] > > > _______________________________________________ > ffmpeg-devel mailing list -- [email protected] > To unsubscribe send an email to [email protected] > _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
