[FFmpeg-devel] [PATCH] lavfi/cropdetect: support more pixel formats

[Stefano Sabatini]

On date Friday 2013-07-05 15:01:37 +0000, Paul B Mahol encoded:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  doc/filters.texi            |  2 +-
>  libavfilter/vf_cropdetect.c | 60 +++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index f713bb9..e1e13df 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2529,7 +2529,7 @@ The filter accepts the following options:
>  
>  @item limit
>  Set higher black value threshold, which can be optionally specified
> -from nothing (0) to everything (255). An intensity value greater
> +from nothing (0) to everything (65535). An intensity value greater
>  to the set value is considered non-black. Default value is 24.

This means the user needs to know the colorspace used in the filter,
which is not robust at all.

A possibility would be to express the value in 1/255 units and
introduce an relative parameter expressed as a floating point value in
the 0-1 range or an expression like:
gt(val, min_val+0.2*(max_val-min_val))

>  @item round
> diff --git a/libavfilter/vf_cropdetect.c b/libavfilter/vf_cropdetect.c
> index 28ae887..e5fbc0a 100644
> --- a/libavfilter/vf_cropdetect.c
> +++ b/libavfilter/vf_cropdetect.c
> @@ -25,6 +25,7 @@
>  
>  #include "libavutil/imgutils.h"
>  #include "libavutil/internal.h"
> +#include "libavutil/intreadwrite.h"
>  #include "libavutil/opt.h"
>  
>  #include "avfilter.h"
> @@ -45,11 +46,46 @@ typedef struct {
>  static int query_formats(AVFilterContext *ctx)
>  {
>      static const enum AVPixelFormat pix_fmts[] = {
> +        AV_PIX_FMT_GRAY8,   AV_PIX_FMT_GRAY16,
> +        AV_PIX_FMT_YUV410P,
> +        AV_PIX_FMT_YUV411P, AV_PIX_FMT_YUVJ411P,
>          AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUVJ420P,
>          AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUVJ422P,
> +        AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUVJ440P,
>          AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVJ444P,
> -        AV_PIX_FMT_YUV411P, AV_PIX_FMT_GRAY8,
> +        AV_PIX_FMT_YUVA420P,
> +        AV_PIX_FMT_YUVA422P,
> +        AV_PIX_FMT_YUVA444P,
> +        AV_PIX_FMT_YUV420P9,
> +        AV_PIX_FMT_YUV422P9,
> +        AV_PIX_FMT_YUV444P9,
> +        AV_PIX_FMT_YUVA420P9,
> +        AV_PIX_FMT_YUVA422P9,
> +        AV_PIX_FMT_YUVA444P9,
> +        AV_PIX_FMT_YUV420P10,
> +        AV_PIX_FMT_YUV422P10,
> +        AV_PIX_FMT_YUV444P10,
> +        AV_PIX_FMT_YUVA420P10,
> +        AV_PIX_FMT_YUVA422P10,
> +        AV_PIX_FMT_YUVA444P10,
> +        AV_PIX_FMT_YUV420P12,
> +        AV_PIX_FMT_YUV422P12,
> +        AV_PIX_FMT_YUV444P12,
> +        AV_PIX_FMT_YUV420P14,
> +        AV_PIX_FMT_YUV422P14,
> +        AV_PIX_FMT_YUV444P14,
> +        AV_PIX_FMT_YUV420P16,
> +        AV_PIX_FMT_YUV422P16,
> +        AV_PIX_FMT_YUV444P16,
> +        AV_PIX_FMT_YUVA420P16,
> +        AV_PIX_FMT_YUVA422P16,
> +        AV_PIX_FMT_YUVA444P16,
>          AV_PIX_FMT_NV12,    AV_PIX_FMT_NV21,
> +        AV_PIX_FMT_RGB24,   AV_PIX_FMT_BGR24,
> +        AV_PIX_FMT_RGBA,    AV_PIX_FMT_BGRA,
> +        AV_PIX_FMT_RGB0,    AV_PIX_FMT_BGR0,
> +        AV_PIX_FMT_RGB48,   AV_PIX_FMT_BGR48,
> +        AV_PIX_FMT_RGBA64,  AV_PIX_FMT_BGRA64,
>          AV_PIX_FMT_NONE
>      };
>  
> @@ -57,9 +93,9 @@ static int query_formats(AVFilterContext *ctx)
>      return 0;
>  }
>  
> -static int checkline(void *ctx, const unsigned char *src, int stride, int len, int bpp)
> +static int64_t checkline(void *ctx, const uint8_t *src, int stride, int len, int bpp)
>  {
> -    int total = 0;
> +    int64_t total = 0;
>      int div = len;
>  
>      switch (bpp) {
> @@ -69,6 +105,12 @@ static int checkline(void *ctx, const unsigned char *src, int stride, int len, i
>              src += stride;
>          }
>          break;
> +    case 2:
> +        while (--len >= 0) {
> +            total += AV_RN16(&src[0]);
> +            src += stride;
> +        }
> +        break;
>      case 3:
>      case 4:
>          while (--len >= 0) {
> @@ -77,10 +119,18 @@ static int checkline(void *ctx, const unsigned char *src, int stride, int len, i
>          }
>          div *= 3;
>          break;
> +    case 6:
> +    case 8:
> +        while (--len >= 0) {
> +            total += AV_RN16(&src[0]) + AV_RN16(&src[2]) + AV_RN16(&src[4]);
> +            src += stride;
> +        }
> +        div *= 3;
> +        break;

was the 3/4 path ever used before?

I suggest to add a note about RGB colorspace, or even better to add a
distinct RGB path.

[...]
-- 
FFmpeg = Fabulous & Foolish Murdering Proud Enlightening Geek