[FFmpeg-devel,v1,2/3] Revert "lavfi/colorlevels: Add slice threading support"

Submitted by lance.lmwang@gmail.com on Oct. 23, 2019, 10:27 a.m.

Details

Message ID 20191023102743.19979-2-lance.lmwang@gmail.com
State New
Headers show

Commit Message

lance.lmwang@gmail.com Oct. 23, 2019, 10:27 a.m.
From: Limin Wang <lance.lmwang@gmail.com>

This reverts commit 360bee8ca49d94d5cc8b77106887d6d7250440fe

An alternative approach will be used to make the code easy to maintain

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/vf_colorlevels.c | 110 ++++++-----------------------------
 1 file changed, 19 insertions(+), 91 deletions(-)

Comments

lance.lmwang@gmail.com Oct. 24, 2019, 1:12 a.m.
The revert is for better code review, if it's ok, please merge the patch
into one. I am not sure if it is worth saving 100 lines of code.
 
 vf_colorlevels.c |  246 ++++++++++++++++---------------------------------------
 1 file changed, 76 insertions(+), 170 deletions(-)

On Wed, Oct 23, 2019 at 06:27:42PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> This reverts commit 360bee8ca49d94d5cc8b77106887d6d7250440fe
> 
> An alternative approach will be used to make the code easy to maintain
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_colorlevels.c | 110 ++++++-----------------------------
>  1 file changed, 19 insertions(+), 91 deletions(-)
> 
> diff --git a/libavfilter/vf_colorlevels.c b/libavfilter/vf_colorlevels.c
> index fadb39e004..5385a5e754 100644
> --- a/libavfilter/vf_colorlevels.c
> +++ b/libavfilter/vf_colorlevels.c
> @@ -105,68 +105,6 @@ static int config_input(AVFilterLink *inlink)
>      return 0;
>  }
>  
> -struct thread_data {
> -    const uint8_t *srcrow;
> -    uint8_t *dstrow;
> -    int dst_linesize;
> -    int src_linesize;
> -
> -    double coeff;
> -    uint8_t offset;
> -
> -    int h;
> -
> -    int imin;
> -    int omin;
> -};
> -
> -#define LOAD_COMMON\
> -    ColorLevelsContext *s = ctx->priv;\
> -    const struct thread_data *td = arg;\
> -\
> -    int process_h = td->h;\
> -    const int slice_start = (process_h *  jobnr   ) / nb_jobs;\
> -    const int slice_end   = (process_h * (jobnr+1)) / nb_jobs;\
> -    int x, y;\
> -    const uint8_t *srcrow = td->srcrow;\
> -    uint8_t *dstrow = td->dstrow;\
> -    const int step = s->step;\
> -    const uint8_t offset = td->offset;\
> -\
> -    int imin = td->imin;\
> -    int omin = td->omin;\
> -    double coeff = td->coeff;\
> -
> -static int colorlevel_slice_8(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs)
> -{
> -    LOAD_COMMON
> -
> -    for (y = slice_start; y < slice_end; y++) {
> -        const uint8_t *src = srcrow + y * td->src_linesize;
> -        uint8_t *dst = dstrow + y * td->dst_linesize;
> -
> -        for (x = 0; x < s->linesize; x += step)
> -            dst[x + offset] = av_clip_uint8((src[x + offset] - imin) * coeff + omin);
> -    }
> -
> -    return 0;
> -}
> -
> -static int colorlevel_slice_16(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs)
> -{
> -    LOAD_COMMON
> -
> -    for (y = slice_start; y < slice_end; y++) {
> -        const uint16_t *src = (const uint16_t *)(srcrow + y * td->src_linesize);
> -        uint16_t *dst = (uint16_t *)(dstrow + y * td->dst_linesize);
> -
> -        for (x = 0; x < s->linesize; x += step)
> -            dst[x + offset] = av_clip_uint16((src[x + offset] - imin) * coeff + omin);
> -    }
> -
> -    return 0;
> -}
> -
>  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  {
>      AVFilterContext *ctx = inlink->dst;
> @@ -199,7 +137,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>              int omin = lrint(r->out_min * UINT8_MAX);
>              int omax = lrint(r->out_max * UINT8_MAX);
>              double coeff;
> -            struct thread_data td;
>  
>              if (imin < 0) {
>                  imin = UINT8_MAX;
> @@ -225,19 +162,15 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  
>              srcrow = in->data[0];
>              coeff = (omax - omin) / (double)(imax - imin);
> -
> -            td.srcrow        = srcrow;
> -            td.dstrow        = dstrow;
> -            td.dst_linesize  = out->linesize[0];
> -            td.src_linesize  = in->linesize[0];
> -            td.coeff         = coeff;
> -            td.offset        = offset;
> -            td.h             = inlink->h;
> -            td.imin          = imin;
> -            td.omin          = omin;
> -
> -            ctx->internal->execute(ctx, colorlevel_slice_8, &td, NULL,
> -                                   FFMIN(inlink->h, ff_filter_get_nb_threads(ctx)));
> +            for (y = 0; y < inlink->h; y++) {
> +                const uint8_t *src = srcrow;
> +                uint8_t *dst = dstrow;
> +
> +                for (x = 0; x < s->linesize; x += step)
> +                    dst[x + offset] = av_clip_uint8((src[x + offset] - imin) * coeff + omin);
> +                dstrow += out->linesize[0];
> +                srcrow += in->linesize[0];
> +            }
>          }
>          break;
>      case 2:
> @@ -251,7 +184,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>              int omin = lrint(r->out_min * UINT16_MAX);
>              int omax = lrint(r->out_max * UINT16_MAX);
>              double coeff;
> -            struct thread_data td;
>  
>              if (imin < 0) {
>                  imin = UINT16_MAX;
> @@ -277,19 +209,15 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>  
>              srcrow = in->data[0];
>              coeff = (omax - omin) / (double)(imax - imin);
> -
> -            td.srcrow        = srcrow;
> -            td.dstrow        = dstrow;
> -            td.dst_linesize  = out->linesize[0];
> -            td.src_linesize  = in->linesize[0];
> -            td.coeff         = coeff;
> -            td.offset        = offset;
> -            td.h             = inlink->h;
> -            td.imin          = imin;
> -            td.omin          = omin;
> -
> -            ctx->internal->execute(ctx, colorlevel_slice_16, &td, NULL,
> -                                   FFMIN(inlink->h, ff_filter_get_nb_threads(ctx)));
> +            for (y = 0; y < inlink->h; y++) {
> +                const uint16_t *src = (const uint16_t*)srcrow;
> +                uint16_t *dst = (uint16_t *)dstrow;
> +
> +                for (x = 0; x < s->linesize; x += step)
> +                    dst[x + offset] = av_clip_uint16((src[x + offset] - imin) * coeff + omin);
> +                dstrow += out->linesize[0];
> +                srcrow += in->linesize[0];
> +            }
>          }
>      }
>  
> @@ -324,5 +252,5 @@ AVFilter ff_vf_colorlevels = {
>      .query_formats = query_formats,
>      .inputs        = colorlevels_inputs,
>      .outputs       = colorlevels_outputs,
> -    .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | AVFILTER_FLAG_SLICE_THREADS,
> +    .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC,
>  };
> -- 
> 2.21.0
>

Patch hide | download patch | download mbox

diff --git a/libavfilter/vf_colorlevels.c b/libavfilter/vf_colorlevels.c
index fadb39e004..5385a5e754 100644
--- a/libavfilter/vf_colorlevels.c
+++ b/libavfilter/vf_colorlevels.c
@@ -105,68 +105,6 @@  static int config_input(AVFilterLink *inlink)
     return 0;
 }
 
-struct thread_data {
-    const uint8_t *srcrow;
-    uint8_t *dstrow;
-    int dst_linesize;
-    int src_linesize;
-
-    double coeff;
-    uint8_t offset;
-
-    int h;
-
-    int imin;
-    int omin;
-};
-
-#define LOAD_COMMON\
-    ColorLevelsContext *s = ctx->priv;\
-    const struct thread_data *td = arg;\
-\
-    int process_h = td->h;\
-    const int slice_start = (process_h *  jobnr   ) / nb_jobs;\
-    const int slice_end   = (process_h * (jobnr+1)) / nb_jobs;\
-    int x, y;\
-    const uint8_t *srcrow = td->srcrow;\
-    uint8_t *dstrow = td->dstrow;\
-    const int step = s->step;\
-    const uint8_t offset = td->offset;\
-\
-    int imin = td->imin;\
-    int omin = td->omin;\
-    double coeff = td->coeff;\
-
-static int colorlevel_slice_8(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs)
-{
-    LOAD_COMMON
-
-    for (y = slice_start; y < slice_end; y++) {
-        const uint8_t *src = srcrow + y * td->src_linesize;
-        uint8_t *dst = dstrow + y * td->dst_linesize;
-
-        for (x = 0; x < s->linesize; x += step)
-            dst[x + offset] = av_clip_uint8((src[x + offset] - imin) * coeff + omin);
-    }
-
-    return 0;
-}
-
-static int colorlevel_slice_16(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs)
-{
-    LOAD_COMMON
-
-    for (y = slice_start; y < slice_end; y++) {
-        const uint16_t *src = (const uint16_t *)(srcrow + y * td->src_linesize);
-        uint16_t *dst = (uint16_t *)(dstrow + y * td->dst_linesize);
-
-        for (x = 0; x < s->linesize; x += step)
-            dst[x + offset] = av_clip_uint16((src[x + offset] - imin) * coeff + omin);
-    }
-
-    return 0;
-}
-
 static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 {
     AVFilterContext *ctx = inlink->dst;
@@ -199,7 +137,6 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
             int omin = lrint(r->out_min * UINT8_MAX);
             int omax = lrint(r->out_max * UINT8_MAX);
             double coeff;
-            struct thread_data td;
 
             if (imin < 0) {
                 imin = UINT8_MAX;
@@ -225,19 +162,15 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 
             srcrow = in->data[0];
             coeff = (omax - omin) / (double)(imax - imin);
-
-            td.srcrow        = srcrow;
-            td.dstrow        = dstrow;
-            td.dst_linesize  = out->linesize[0];
-            td.src_linesize  = in->linesize[0];
-            td.coeff         = coeff;
-            td.offset        = offset;
-            td.h             = inlink->h;
-            td.imin          = imin;
-            td.omin          = omin;
-
-            ctx->internal->execute(ctx, colorlevel_slice_8, &td, NULL,
-                                   FFMIN(inlink->h, ff_filter_get_nb_threads(ctx)));
+            for (y = 0; y < inlink->h; y++) {
+                const uint8_t *src = srcrow;
+                uint8_t *dst = dstrow;
+
+                for (x = 0; x < s->linesize; x += step)
+                    dst[x + offset] = av_clip_uint8((src[x + offset] - imin) * coeff + omin);
+                dstrow += out->linesize[0];
+                srcrow += in->linesize[0];
+            }
         }
         break;
     case 2:
@@ -251,7 +184,6 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
             int omin = lrint(r->out_min * UINT16_MAX);
             int omax = lrint(r->out_max * UINT16_MAX);
             double coeff;
-            struct thread_data td;
 
             if (imin < 0) {
                 imin = UINT16_MAX;
@@ -277,19 +209,15 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 
             srcrow = in->data[0];
             coeff = (omax - omin) / (double)(imax - imin);
-
-            td.srcrow        = srcrow;
-            td.dstrow        = dstrow;
-            td.dst_linesize  = out->linesize[0];
-            td.src_linesize  = in->linesize[0];
-            td.coeff         = coeff;
-            td.offset        = offset;
-            td.h             = inlink->h;
-            td.imin          = imin;
-            td.omin          = omin;
-
-            ctx->internal->execute(ctx, colorlevel_slice_16, &td, NULL,
-                                   FFMIN(inlink->h, ff_filter_get_nb_threads(ctx)));
+            for (y = 0; y < inlink->h; y++) {
+                const uint16_t *src = (const uint16_t*)srcrow;
+                uint16_t *dst = (uint16_t *)dstrow;
+
+                for (x = 0; x < s->linesize; x += step)
+                    dst[x + offset] = av_clip_uint16((src[x + offset] - imin) * coeff + omin);
+                dstrow += out->linesize[0];
+                srcrow += in->linesize[0];
+            }
         }
     }
 
@@ -324,5 +252,5 @@  AVFilter ff_vf_colorlevels = {
     .query_formats = query_formats,
     .inputs        = colorlevels_inputs,
     .outputs       = colorlevels_outputs,
-    .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC | AVFILTER_FLAG_SLICE_THREADS,
+    .flags         = AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC,
 };