Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v4] avfilter/vf_blackframe: add slice threading
@ 2025-12-23 16:44 Raja Rathour via ffmpeg-devel
  2025-12-27 19:53 ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
  2025-12-30 17:31 ` [FFmpeg-devel] [PATCH v5] " Raja Rathour via ffmpeg-devel
  0 siblings, 2 replies; 10+ messages in thread
From: Raja Rathour via ffmpeg-devel @ 2025-12-23 16:44 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Raja Rathour

---
 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

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] Re: [PATCH v4] avfilter/vf_blackframe: add slice threading
  2025-12-23 16:44 [FFmpeg-devel] [PATCH v4] avfilter/vf_blackframe: add slice threading Raja Rathour via ffmpeg-devel
@ 2025-12-27 19:53 ` Michael Niedermayer via ffmpeg-devel
  2025-12-30 17:31 ` [FFmpeg-devel] [PATCH v5] " Raja Rathour via ffmpeg-devel
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Niedermayer via ffmpeg-devel @ 2025-12-27 19:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Michael Niedermayer


[-- Attachment #1.1: Type: text/plain, Size: 3447 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH v5] avfilter/vf_blackframe: add slice threading
  2025-12-23 16:44 [FFmpeg-devel] [PATCH v4] avfilter/vf_blackframe: add slice threading Raja Rathour via ffmpeg-devel
  2025-12-27 19:53 ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
@ 2025-12-30 17:31 ` Raja Rathour via ffmpeg-devel
  2025-12-30 17:53   ` [FFmpeg-devel] [PATCH v6] " Raja Rathour via ffmpeg-devel
  1 sibling, 1 reply; 10+ messages in thread
From: Raja Rathour via ffmpeg-devel @ 2025-12-30 17:31 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Raja Rathour

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

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH v6] avfilter/vf_blackframe: add slice threading
  2025-12-30 17:31 ` [FFmpeg-devel] [PATCH v5] " Raja Rathour via ffmpeg-devel
@ 2025-12-30 17:53   ` Raja Rathour via ffmpeg-devel
  2025-12-30 19:06     ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
  0 siblings, 1 reply; 10+ messages in thread
From: Raja Rathour via ffmpeg-devel @ 2025-12-30 17:53 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Raja Rathour

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

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] Re: [PATCH v6] avfilter/vf_blackframe: add slice threading
  2025-12-30 17:53   ` [FFmpeg-devel] [PATCH v6] " Raja Rathour via ffmpeg-devel
@ 2025-12-30 19:06     ` Michael Niedermayer via ffmpeg-devel
  2025-12-30 19:40       ` [FFmpeg-devel] [PATCH v7] " Raja Rathour via ffmpeg-devel
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Niedermayer via ffmpeg-devel @ 2025-12-30 19:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Michael Niedermayer


[-- Attachment #1.1: Type: text/plain, Size: 995 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH v7] avfilter/vf_blackframe: add slice threading
  2025-12-30 19:06     ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
@ 2025-12-30 19:40       ` Raja Rathour via ffmpeg-devel
  2025-12-31  2:06         ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
  0 siblings, 1 reply; 10+ messages in thread
From: Raja Rathour via ffmpeg-devel @ 2025-12-30 19:40 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Raja Rathour

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

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] Re: [PATCH v7] avfilter/vf_blackframe: add slice threading
  2025-12-30 19:40       ` [FFmpeg-devel] [PATCH v7] " Raja Rathour via ffmpeg-devel
@ 2025-12-31  2:06         ` Michael Niedermayer via ffmpeg-devel
  2026-01-01  6:39           ` [FFmpeg-devel] [PATCH v8] " Raja Rathour via ffmpeg-devel
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Niedermayer via ffmpeg-devel @ 2025-12-31  2:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Michael Niedermayer


[-- Attachment #1.1: Type: text/plain, Size: 2152 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 163 bytes --]

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH v8] avfilter/vf_blackframe: add slice threading
  2025-12-31  2:06         ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
@ 2026-01-01  6:39           ` Raja Rathour via ffmpeg-devel
  2026-01-04 22:15             ` [FFmpeg-devel] " Marton Balint via ffmpeg-devel
  0 siblings, 1 reply; 10+ messages in thread
From: Raja Rathour via ffmpeg-devel @ 2026-01-01  6:39 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Raja Rathour

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

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] Re: [PATCH v8] avfilter/vf_blackframe: add slice threading
  2026-01-01  6:39           ` [FFmpeg-devel] [PATCH v8] " Raja Rathour via ffmpeg-devel
@ 2026-01-04 22:15             ` Marton Balint via ffmpeg-devel
  2026-01-07 14:38               ` Raja Rathour via ffmpeg-devel
  0 siblings, 1 reply; 10+ messages in thread
From: Marton Balint via ffmpeg-devel @ 2026-01-04 22:15 UTC (permalink / raw)
  To: Raja Rathour via ffmpeg-devel; +Cc: Marton Balint



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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] Re: [PATCH v8] avfilter/vf_blackframe: add slice threading
  2026-01-04 22:15             ` [FFmpeg-devel] " Marton Balint via ffmpeg-devel
@ 2026-01-07 14:38               ` Raja Rathour via ffmpeg-devel
  0 siblings, 0 replies; 10+ messages in thread
From: Raja Rathour via ffmpeg-devel @ 2026-01-07 14:38 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Raja Rathour

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
>
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-01-07 14:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-23 16:44 [FFmpeg-devel] [PATCH v4] avfilter/vf_blackframe: add slice threading Raja Rathour via ffmpeg-devel
2025-12-27 19:53 ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
2025-12-30 17:31 ` [FFmpeg-devel] [PATCH v5] " Raja Rathour via ffmpeg-devel
2025-12-30 17:53   ` [FFmpeg-devel] [PATCH v6] " Raja Rathour via ffmpeg-devel
2025-12-30 19:06     ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
2025-12-30 19:40       ` [FFmpeg-devel] [PATCH v7] " Raja Rathour via ffmpeg-devel
2025-12-31  2:06         ` [FFmpeg-devel] " Michael Niedermayer via ffmpeg-devel
2026-01-01  6:39           ` [FFmpeg-devel] [PATCH v8] " Raja Rathour via ffmpeg-devel
2026-01-04 22:15             ` [FFmpeg-devel] " Marton Balint via ffmpeg-devel
2026-01-07 14:38               ` Raja Rathour via ffmpeg-devel

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