Re: [FFmpeg-devel] [PATCH] avfilter/f_ebur128: use correct type for chl

2019-04-29 Thread Clément Bœsch
On Mon, Apr 29, 2019 at 04:47:52PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/f_ebur128.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c
> index f613d8def2..f25d5f096e 100644
> --- a/libavfilter/f_ebur128.c
> +++ b/libavfilter/f_ebur128.c
> @@ -420,7 +420,7 @@ static int config_audio_output(AVFilterLink *outlink)
>  
>  for (i = 0; i < nb_channels; i++) {
>  /* channel weighting */
> -const uint16_t chl = 
> av_channel_layout_extract_channel(outlink->channel_layout, i);
> +const uint64_t chl = 
> av_channel_layout_extract_channel(outlink->channel_layout, i);
>  if (chl & (AV_CH_LOW_FREQUENCY|AV_CH_LOW_FREQUENCY_2)) {
>  ebur128->ch_weighting[i] = 0;
>  } else if (chl & BACK_MASK) {

LGTM, thanks

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_lut3d: increase MAX_LEVEL

2019-04-27 Thread Clément Bœsch
On Sat, Apr 27, 2019 at 12:29:44PM +0200, Paul B Mahol wrote:
> On 4/27/19, Clément Bœsch  wrote:
> > On Thu, Apr 25, 2019 at 04:47:00PM +0200, Paul B Mahol wrote:
> >> On 4/25/19, Reto Kromer  wrote:
> >> > Paul B Mahol wrote:
> >> >
> >> >>Found 65x65x65 3D LUT in wild
> >> >
> >> > FYI: 128x128x128 3D LUTs do also exist in film production.
> >> >
> >>
> >> Thank you, changed locally.
> >
> > This means one single malloc of 24M unless I'm mistaken. Are we sure
> > that's what we want?
> 
> So you want this to be dynamically allocated? How you want to access elements?

It is actually already dynamically allocated (the private context is), but
I suppose you meant individual allocs for each dimension. It really is an
open question. Maybe it's not an issue, it's just that I would just think
twice before doing such change because I would assume it would impact
performance on low-end machines due to more sparse memory accesses.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/subtitles: ignore extra '\r' at line endings

2019-04-27 Thread Clément Bœsch
On Thu, Apr 25, 2019 at 11:22:47PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/microdvddec.c | 2 ++
>  libavformat/subtitles.c   | 2 +-
>  libavformat/subtitles.h   | 2 +-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/microdvddec.c b/libavformat/microdvddec.c
> index ef6bcfbc73..598093309c 100644
> --- a/libavformat/microdvddec.c
> +++ b/libavformat/microdvddec.c
> @@ -130,6 +130,8 @@ static int microdvd_read_header(AVFormatContext *s)
>  continue;   \
>  }   \
>  p++
> +if (!*p)
> +continue;
>  SKIP_FRAME_ID;
>  SKIP_FRAME_ID;
>  if (!*p)

This looks unrelated and should probably be much earlier (just after `p =
line`?)

> diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
> index 93c9ef05cf..659c99d1cf 100644
> --- a/libavformat/subtitles.c
> +++ b/libavformat/subtitles.c
> @@ -417,7 +417,7 @@ ptrdiff_t ff_subtitles_read_line(FFTextReader *tr, char 
> *buf, size_t size)
>  buf[cur++] = c;
>  buf[cur] = '\0';
>  }
> -if (ff_text_peek_r8(tr) == '\r')
> +while (ff_text_peek_r8(tr) == '\r')
>  ff_text_r8(tr);
>  if (ff_text_peek_r8(tr) == '\n')
>  ff_text_r8(tr);
> diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
> index ca78db224d..6b418e3621 100644
> --- a/libavformat/subtitles.h
> +++ b/libavformat/subtitles.h
> @@ -188,7 +188,7 @@ static av_always_inline int ff_subtitles_next_line(const 
> char *ptr)
>  {
>  int n = strcspn(ptr, "\r\n");
>  ptr += n;
> -if (*ptr == '\r') {
> +while (*ptr == '\r') {
>  ptr++;
>  n++;

Rest should be fine if it passes FATE.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_lut3d: fix range domain processing for .cube format

2019-04-27 Thread Clément Bœsch
On Thu, Apr 25, 2019 at 02:57:26PM +0200, Paul B Mahol wrote:
> The ranges are for input, not for output.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/vf_lut3d.c | 73 +-
>  1 file changed, 51 insertions(+), 22 deletions(-)
> 

LGTM, thanks

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_lut3d: increase MAX_LEVEL

2019-04-27 Thread Clément Bœsch
On Thu, Apr 25, 2019 at 04:47:00PM +0200, Paul B Mahol wrote:
> On 4/25/19, Reto Kromer  wrote:
> > Paul B Mahol wrote:
> >
> >>Found 65x65x65 3D LUT in wild
> >
> > FYI: 128x128x128 3D LUTs do also exist in film production.
> >
> 
> Thank you, changed locally.

This means one single malloc of 24M unless I'm mistaken. Are we sure
that's what we want?

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_lut3d: add cineSpace 1D lut parsing

2019-04-20 Thread Clément Bœsch
On Fri, Apr 19, 2019 at 07:53:36PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi   |  2 ++
>  libavfilter/vf_lut3d.c | 77 ++
>  2 files changed, 79 insertions(+)
> 

LGTM

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_lut3d: add cineSpace 3D lut support

2019-04-20 Thread Clément Bœsch
On Fri, Apr 19, 2019 at 07:53:37PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi   |  2 +
>  libavfilter/vf_lut3d.c | 91 ++
>  2 files changed, 93 insertions(+)
> 

LGTM

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH V2] lavfi/vf_nlmeans: Improve the performance for nlmeans

2019-02-01 Thread Clément Bœsch
On Fri, Feb 01, 2019 at 05:50:37PM +0800, myp...@gmail.com wrote:
[...]
> > Ok, makes sense. Would you mind updating the comment to something like:
> >
> > /* Note: WEIGHT_LUT_SIZE must be larger than max_meaningful_diff
> >  * (log(255)*max(h)^2, which is approximately 50 with the current
> >  * maximum sigma of 30). The current value is arbitrary and could be
> >  * tweaked or defined dynamically. */
> > #define WEIGHT_LUT_SIZE 80
> >
> > I will test your patch tonight (let's say in about 10 hours given my
> > current timezone) and apply if everything works fine.
> >
> it's OK, and I think you can change   80  to 50 at the same time
> if the patch is Ok. Tks

I did change it to 50, added a comment, updated the description, and
pushed. I also made the dynamic change you suggested earlier on the weight
LUT.

Thanks for the patch,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V2] lavfi/vf_nlmeans: Improve the performance for nlmeans

2019-02-01 Thread Clément Bœsch
On Fri, Feb 01, 2019 at 05:19:53PM +0800, myp...@gmail.com wrote:
> On Fri, Feb 1, 2019 at 5:11 PM Clément Bœsch  wrote:
> >
> > On Fri, Feb 01, 2019 at 04:57:37PM +0800, myp...@gmail.com wrote:
> > [...]
> > > > > -#define WEIGHT_LUT_NBITS 9
> > > > > -#define WEIGHT_LUT_SIZE  (1< > > > > +#define WEIGHT_LUT_SIZE  (80) // need to >  300 * 300 *
> log(255)
> > > >
> > > > So the LUT is now 3.2MB?
> > > >
> > > > Why 300? 300*300*log(255) is closer to 500 000 than 800 000
> > > I just seleted a value > 300*300*log(255) (500 000 more precise) for
> > > this case at liberty in fact , the other option is use a dynamic
> > > allocation memory for weight_lut table size base on the
> > > max_meaningful_diff :), but maybe seems pretty obvious, I think 3M is
> > > not a big burden for nlmeans
> >
> > It's probably fine yes, I'm just confused at the comment: why does it
> > *needs* to be > 300 * 300 * log(255)?
> >
> > --
> ohhh, 300 = max(s) * 10 :), max(s) = 30, this is the reason.
> 
> In fact, max size of WEIGHT_LUT_SIZE ==  max (max_meaningful_diff), then we
> can avoid use pdiff_lut_scale in nlmeans, becasue now pdiff_lut_scale == 1.
> :)
> 
> and max( max_meaningful_diff ) = -log(-1/255.0) * h * h = log(255) * max
> (h) * max(h) = log(255) * max (10*s) * max(10*s)

Ok, makes sense. Would you mind updating the comment to something like:

/* Note: WEIGHT_LUT_SIZE must be larger than max_meaningful_diff
 * (log(255)*max(h)^2, which is approximately 50 with the current
 * maximum sigma of 30). The current value is arbitrary and could be
 * tweaked or defined dynamically. */
#define WEIGHT_LUT_SIZE 80

I will test your patch tonight (let's say in about 10 hours given my
current timezone) and apply if everything works fine.

Thanks

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V2] lavfi/vf_nlmeans: Improve the performance for nlmeans

2019-02-01 Thread Clément Bœsch
On Fri, Feb 01, 2019 at 04:57:37PM +0800, myp...@gmail.com wrote:
[...]
> > > -#define WEIGHT_LUT_NBITS 9
> > > -#define WEIGHT_LUT_SIZE  (1< > > +#define WEIGHT_LUT_SIZE  (80) // need to >  300 * 300 * log(255)
> >
> > So the LUT is now 3.2MB?
> >
> > Why 300? 300*300*log(255) is closer to 500 000 than 800 000
> I just seleted a value > 300*300*log(255) (500 000 more precise) for
> this case at liberty in fact , the other option is use a dynamic
> allocation memory for weight_lut table size base on the
> max_meaningful_diff :), but maybe seems pretty obvious, I think 3M is
> not a big burden for nlmeans

It's probably fine yes, I'm just confused at the comment: why does it
*needs* to be > 300 * 300 * log(255)?

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V2] lavfi/vf_nlmeans: Improve the performance for nlmeans

2019-02-01 Thread Clément Bœsch
On Fri, Feb 01, 2019 at 10:45:24AM +0800, Jun Zhao wrote:
> Remove the pdiff_lut_scale in nlmeans and increase weight_lut table size
> from 2^9 to 80, this change will avoid using pdiff_lut_scale in
> nlmeans_slice() for weight_lut table search, it's will improve the
> performance about 12%. (in 1080P size picture case).
> 
> Use the profiling command like:
> 
> perf stat -a -d -r 5 ./ffmpeg -i input -an -vf nlmeans=s=30 -vframes 10 \
> -f null /dev/null
> 
> without this change:
> when s=1.0(default value) 63s
>  s=30.0   72s
> 
> after this change:
>  s=1.0(default value) 56s
>  s=30.0   63s

Nice.

I assume this is tested on x86_64?

> 
> Reviewed-by: Carl Eugen Hoyos 
> Signed-off-by: Jun Zhao 
> ---
>  libavfilter/vf_nlmeans.c |   12 
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
> index 82e779c..72eb819 100644
> --- a/libavfilter/vf_nlmeans.c
> +++ b/libavfilter/vf_nlmeans.c
> @@ -43,8 +43,7 @@ struct weighted_avg {
>  float sum;
>  };
>  
> -#define WEIGHT_LUT_NBITS 9
> -#define WEIGHT_LUT_SIZE  (1< +#define WEIGHT_LUT_SIZE  (80) // need to >  300 * 300 * log(255)

So the LUT is now 3.2MB?

Why 300? 300*300*log(255) is closer to 500 000 than 800 000

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter/vf_paletteuse: don't constantly free and realloc internal frames

2019-01-17 Thread Clément Bœsch
On Tue, Jan 15, 2019 at 01:14:34AM -0300, James Almer wrote:
> Signed-off-by: James Almer 
> ---
>  libavfilter/vf_paletteuse.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
> index 5966f10685..289c663e8e 100644
> --- a/libavfilter/vf_paletteuse.c
> +++ b/libavfilter/vf_paletteuse.c
> @@ -814,7 +814,7 @@ static void set_processing_window(enum diff_mode 
> diff_mode,
>  int width  = cur_src->width;
>  int height = cur_src->height;
>  
> -if (prv_src && diff_mode == DIFF_MODE_RECTANGLE) {
> +if (prv_src->data[0] && diff_mode == DIFF_MODE_RECTANGLE) {
>  int y;
>  int x_end = cur_src->width  - 1,
>  y_end = cur_src->height - 1;
> @@ -911,11 +911,10 @@ static int apply_palette(AVFilterLink *inlink, AVFrame 
> *in, AVFrame **outf)
>  
>  set_processing_window(s->diff_mode, s->last_in, in,
>s->last_out, out, , , , );
> -av_frame_free(>last_in);
> -av_frame_free(>last_out);
> -s->last_in  = av_frame_clone(in);
> -s->last_out = av_frame_clone(out);
> -if (!s->last_in || !s->last_out ||
> +av_frame_unref(s->last_in);
> +av_frame_unref(s->last_out);
> +if (av_frame_ref(s->last_in, in) < 0 ||
> +av_frame_ref(s->last_out, out) < 0 ||
>  av_frame_make_writable(s->last_in) < 0) {
>  av_frame_free();
>  av_frame_free();
> @@ -1086,6 +1085,11 @@ static av_cold int init(AVFilterContext *ctx)
>  {
>  PaletteUseContext *s = ctx->priv;
>  
> +s->last_in  = av_frame_alloc();
> +s->last_out = av_frame_alloc();
> +if (!s->last_in || !s->last_out)
> +return AVERROR(ENOMEM);
> +
>  s->set_frame = set_frame_lut[s->color_search_method][s->dither];
>  
>  if (s->dither == DITHERING_BAYER) {

LGTM, thanks

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] palettegen: Fill with last color, not black

2019-01-17 Thread Clément Bœsch
On Wed, Jan 16, 2019 at 01:40:20PM +0100, Tomas Härdin wrote:
> Hi
> 
> I was helping the fine folks at peppercarrot.com with web video
> nonsense, and I notice palettegen outputs more colors than it should
> due to padding the generated palette with pure black.
> 
> Compare this (ffmpeg version 3.2.12-1~deb9u1):
> http://www.härdin.se/files/peppercarrot_gif/output-blackspecks-64.gif
> with this (282a471 with this patch applied):
> http://www.härdin.se/files/peppercarrot_gif/output-fixed-64.gif
> 
> The attached patch fixes this by padding with the last color instead of
> black.
> 
> /Tomas

> From 3a01f62fdcc95cc7afaf5aa6e439b8742cce43bc Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= 
> Date: Wed, 16 Jan 2019 13:07:48 +0100
> Subject: [PATCH] palettegen: Fill with last color, not black
> 
> If we fill with black then the generated palette will have one color more
> than what the user requested. This also resulted in unwanted black specks in
> the output of paletteuse, especially when generating small palettes.
> ---
>  libavfilter/vf_palettegen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
> index 5ff73e6b2b..44323782d2 100644
> --- a/libavfilter/vf_palettegen.c
> +++ b/libavfilter/vf_palettegen.c
> @@ -245,7 +245,7 @@ static void write_palette(AVFilterContext *ctx, AVFrame 
> *out)
>  av_log(ctx, AV_LOG_WARNING, "Dupped color: 
> %08"PRIX32"\n", pal[x]);
>  last_color = pal[x];
>  } else {
> -pal[x] = 0xff00; // pad with black
> +pal[x] = last_color; // pad with last color
>  }
>  }
>  pal += pal_linesize;

Code LGTM, thanks

No FATE change?

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavfi/nlmeans: fixup aarch64 assembly with clang

2018-07-27 Thread Clément Bœsch
On Fri, Jul 27, 2018 at 12:03:46AM +0300, Jan Ekström wrote:
> Clang is more strict about some things.
> ---
>  libavfilter/aarch64/vf_nlmeans_neon.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/aarch64/vf_nlmeans_neon.S 
> b/libavfilter/aarch64/vf_nlmeans_neon.S
> index 6308a428db..ac16157bbd 100644
> --- a/libavfilter/aarch64/vf_nlmeans_neon.S
> +++ b/libavfilter/aarch64/vf_nlmeans_neon.S
> @@ -22,7 +22,7 @@
>  
>  // acc_sum_store(ABCD) = {X+A, X+A+B, X+A+B+C, X+A+B+C+D}
>  .macro acc_sum_store x, xb
> -dup v24.4S, v24.4S[3]   // 
> ...X -> 
> +dup v24.4s, v24.s[3]// 
> ...X -> 

can you keep the capitalized form?

>  ext v25.16B, v26.16B, \xb, #12  // 
> ext(,ABCD,12)=0ABC
>  add v24.4S, v24.4S, \x  // 
> +ABCD={X+A,X+B,X+C,X+D}
>  add v24.4S, v24.4S, v25.4S  // 
> {X+A,X+B+A,X+C+B,X+D+C}   (+0ABC)
> @@ -37,7 +37,7 @@ function ff_compute_safe_ssd_integral_image_neon, export=1
>  moviv26.4S, #0  // 
> used as zero for the "rotations" in acc_sum_store
>  sub x3, x3, w6, UXTW// 
> s1 padding (s1_linesize - w)
>  sub x5, x5, w6, UXTW// 
> s2 padding (s2_linesize - w)
> -sub x9, x0, x1, UXTW #2 // 
> dst_top
> +sub x9, x0, w1, UXTW #2 // 
> dst_top
>  sub x1, x1, w6, UXTW// 
> dst padding (dst_linesize_32 - w)
>  lsl x1, x1, #2  // 
> dst padding expressed in bytes
>  1:  mov w10, w6 // 
> width copy for each line

LGTM otherwise, thx

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] checkasm: add vf_nlmeans test for ssd_integral_image

2018-05-18 Thread Clément Bœsch
On Thu, May 10, 2018 at 02:10:00AM +0200, Michael Niedermayer wrote:
> On Tue, May 08, 2018 at 08:29:00AM +0000, Clément Bœsch wrote:
> > ffmpeg | branch: master | Clément Bœsch <u...@pkh.me> | Sun May  6 10:57:23 
> > 2018 +0200| [f679711c1b516786a39f9e582622a200502fff74] | committer: Clément 
> > Bœsch
> > 
> > checkasm: add vf_nlmeans test for ssd_integral_image
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=f679711c1b516786a39f9e582622a200502fff74
> > ---
> > 
> >  tests/checkasm/Makefile |   1 +
> >  tests/checkasm/checkasm.c   |   3 ++
> >  tests/checkasm/checkasm.h   |   1 +
> >  tests/checkasm/vf_nlmeans.c | 113 
> > 
> >  4 files changed, 118 insertions(+)
> 
> This causes
> tests/checkasm/checkasm
> to crash
> (make fate does not crash)
> 
> tell me if you cant reproduce it then ill rebuild and get a proper backtrace 
> & valgrind
> 
> only got this ATM:
> checkasm: using random seed 1025639728
> *** buffer overflow detected ***: tests/checkasm/checkasm terminated
> === Backtrace: =
> /lib/x86_64-linux-gnu/libc.so.6(+0x7329f)[0x7fe82d5de29f]
> /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7fe82d67987c]
> /lib/x86_64-linux-gnu/libc.so.6(+0x10d750)[0x7fe82d678750]
> tests/checkasm/checkasm[0x4c36c7]
> tests/checkasm/checkasm[0x4a1774]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7fe82d58cf45]
> tests/checkasm/checkasm[0x4a1923]
> 

Fixed in 2940af9389e5cb2a7509655195e5ccb928577ed2

Thanks & sorry about the delay

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] libavfilter/vf_nlmeans: add amount parameter

2018-05-18 Thread Clément Bœsch
On Sat, May 12, 2018 at 10:24:35PM +0200, Paul B Mahol wrote:
> For better control of denoising.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi | 4 
>  libavfilter/vf_nlmeans.c | 5 -
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d77c67eb10..60ce18298b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -11420,6 +11420,10 @@ Set research size.
>  Same as @option{r} but for chroma planes.
>  
>  The default value is @var{0} and means automatic.
> +
> +@item a
> +Set denoising amount. Lower values reduces blurring.
> +Default value is @var{1.0} and means full denoising.

I'm not so fond of adding another semantically identical option to
"strength". Do you have some literature on such an option?

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_nlmeans: better weighting of centered pixel

2018-05-18 Thread Clément Bœsch
On Sat, May 12, 2018 at 10:24:34PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/vf_nlmeans.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
> index 82e779ce85..6c9c9d312d 100644
> --- a/libavfilter/vf_nlmeans.c
> +++ b/libavfilter/vf_nlmeans.c
> @@ -39,6 +39,7 @@
>  #include "video.h"
>  
>  struct weighted_avg {
> +float max_weight;
>  float total_weight;
>  float sum;
>  };
> @@ -403,6 +404,7 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
> int jobnr, int nb_jobs
>  if (patch_diff_sq < s->max_meaningful_diff) {
>  const unsigned weight_lut_idx = patch_diff_sq * 
> s->pdiff_lut_scale;
>  const float weight = s->weight_lut[weight_lut_idx]; // 
> exp(-patch_diff_sq * s->pdiff_scale)
> +wa[x].max_weight = FFMAX(weight, wa[x].max_weight);
>  wa[x].total_weight += weight;
>  wa[x].sum += weight * src[x];
>  }
> @@ -422,8 +424,10 @@ static void weight_averages(uint8_t *dst, ptrdiff_t 
> dst_linesize,
>  for (y = 0; y < h; y++) {
>  for (x = 0; x < w; x++) {
>  // Also weight the centered pixel
> -wa[x].total_weight += 1.f;
> -wa[x].sum += 1.f * src[x];
> +if (!isnormal(wa[x].max_weight))
> +wa[x].max_weight = 1.f;
> +wa[x].total_weight += wa[x].max_weight;
> +wa[x].sum += src[x] * wa[x].max_weight;
>  dst[x] = av_clip_uint8(wa[x].sum / wa[x].total_weight);
>  }
>  dst += dst_linesize;

Do you mind adding a cpw/center-pixel-weight option with multiple modes?
"one" and "max" for a start would be nice, then eventually advanced modes
can be added. Please also mention https://arxiv.org/pdf/1211.1656 at least
in the commit description.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [RFC][ALT PATCHES] Code of Conduct Enforcement

2018-05-17 Thread Clément Bœsch
On Mon, May 14, 2018 at 05:50:25PM +0100, Derek Buitenhuis wrote:
[...]
> 1. Implement a formal CoC enforcement system. This has been mostly 
> copypasted
>from VideoLAN's, and is meant as more of a blueprint. This will no 
> doubt
>be controversial.

So as mentioned already in the thread, the main issue is having a
police/justice entity. I would say it needs to be separate from the
development team (to maintain a power separation). Since such profile
doesn't seem to be exactly common in the open source world, maybe we could
externalize it. Does such a service exist with reasonable prices? Could we
use our funds for this? I understand this may sound far-fetched, but who
knows.

If such solution is not viable, we could fallback on the voting committee
to elect/design a subgroup of itself (an odd number like 3 persons maybe?)
to hold this moderation task for a period of 3 or 6 months, maybe 1 year.
Then these members are automatically maintainer of the CoC for this period
of time, and decide what to do with it.

Just random thoughts, no hard opinion on it to be honest.

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] Revert "configure: check that the required header for Linux Perf is available"

2018-05-08 Thread Clément Bœsch
This reverts commit 234a5e08313c6b8b0774796dfa1f285a3f877d14.

Linux perf is not enabled if you set --target=android, which you are
supposed to when building for Android.
---
 configure | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/configure b/configure
index 7c143238a8..bdca93a9ee 100755
--- a/configure
+++ b/configure
@@ -2018,7 +2018,6 @@ HEADERS_LIST="
 ES2_gl_h
 gsm_h
 io_h
-linux_perf_event_h
 machine_ioctl_bt848_h
 machine_ioctl_meteor_h
 malloc_h
@@ -2483,7 +2482,6 @@ simd_align_32_if_any="avx"
 simd_align_64_if_any="avx512"
 
 # system capabilities
-linux_perf_deps="linux_perf_event_h"
 symver_if_any="symver_asm_label symver_gnu_asm"
 valgrind_backtrace_conflict="optimizations"
 valgrind_backtrace_deps="valgrind_valgrind_h"
@@ -5811,7 +5809,6 @@ check_header dxgidebug.h
 check_header dxva.h
 check_header dxva2api.h -D_WIN32_WINNT=0x0600
 check_header io.h
-check_header linux/perf_event.h
 check_header libcrystalhd/libcrystalhd_if.h
 check_header malloc.h
 check_header net/udplite.h
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Misc improvements in nlmeans filter [v2]

2018-05-08 Thread Clément Bœsch
On Mon, May 07, 2018 at 07:32:55PM +0200, Paul B Mahol wrote:
> On 5/7/18, Clement Boesch  wrote:
> > Changes since v1:
> >
> > - fixed float operation in double as pointed out by Moritz
> > - fix broken commit split as pointed out by Michael
> > - added patch 10: "use unsigned for the integral patch"
> > - misc instruction shuffling in AArch64 SIMD for better performances
> >
> > I plan to push this soon unless someone wants more time to review.
> >
> > BTW, x86 SIMD patch welcome, the filter badly needs some performance
> > improvements. Also, any suggestion on how not to make it spend 80% of
> > the time in nlmeans_slice() welcome.
> >
> > Regards,
> 
> LGTM

patchset applied

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v2 04/10] lavfi/nlmeans: add AArch64 SIMD for compute_safe_ssd_integral_image

2018-05-08 Thread Clément Bœsch
On Tue, May 08, 2018 at 04:06:26AM +0200, Michael Niedermayer wrote:
> On Mon, May 07, 2018 at 07:24:16PM +0200, Clément Bœsch wrote:
> > ssd_integral_image_c: 49204.6
> > ssd_integral_image_neon: 28346.8
> > ---
> >  libavfilter/aarch64/Makefile  |  3 +
> >  libavfilter/aarch64/vf_nlmeans_init.c | 33 +++
> >  libavfilter/aarch64/vf_nlmeans_neon.S | 80 +++
> >  libavfilter/vf_nlmeans.c  | 26 ++---
> >  libavfilter/vf_nlmeans.h  | 35 
> >  5 files changed, 170 insertions(+), 7 deletions(-)
> >  create mode 100644 libavfilter/aarch64/Makefile
> >  create mode 100644 libavfilter/aarch64/vf_nlmeans_init.c
> >  create mode 100644 libavfilter/aarch64/vf_nlmeans_neon.S
> >  create mode 100644 libavfilter/vf_nlmeans.h
> 
> seems to break make testprogs unless iam missing something
> 

oups, I forgot I added such a test. It's a bit redundant with the checkasm
test and now mostly useless due to the recently introduced 16B padding
constraint, but I fixed it anyway locally.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 07/10] lavfi/nlmeans: switch from double to float

2018-05-07 Thread Clément Bœsch
Overall speed appears to be 1.1x faster with no noticeable quality
impact.
---
 libavfilter/vf_nlmeans.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index f37f1183f7..aba587f46b 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -40,8 +40,8 @@
 #include "video.h"
 
 struct weighted_avg {
-double total_weight;
-double sum;
+float total_weight;
+float sum;
 };
 
 #define WEIGHT_LUT_NBITS 9
@@ -63,8 +63,8 @@ typedef struct NLMeansContext {
 ptrdiff_t ii_lz_32; // linesize in 32-bit units of 
the integral image
 struct weighted_avg *wa;// weighted average of every 
pixel
 ptrdiff_t wa_linesize;  // linesize for wa in struct 
size unit
-double weight_lut[WEIGHT_LUT_SIZE]; // lookup table mapping 
(scaled) patch differences to their associated weights
-double pdiff_lut_scale; // scale factor for patch 
differences before looking into the LUT
+float weight_lut[WEIGHT_LUT_SIZE];  // lookup table mapping 
(scaled) patch differences to their associated weights
+float pdiff_lut_scale;  // scale factor for patch 
differences before looking into the LUT
 int max_meaningful_diff;// maximum difference 
considered (if the patch difference is too high we ignore the pixel)
 NLMeansDSPContext dsp;
 } NLMeansContext;
@@ -402,7 +402,7 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 const int patch_diff_sq = get_integral_patch_value(td->ii_start, 
s->ii_lz_32, x, y, td->p);
 if (patch_diff_sq < s->max_meaningful_diff) {
 const int weight_lut_idx = patch_diff_sq * s->pdiff_lut_scale;
-const double weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
+const float weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
 wa[x].total_weight += weight;
 wa[x].sum += weight * src[x];
 }
@@ -453,8 +453,8 @@ static int nlmeans_plane(AVFilterContext *ctx, int w, int 
h, int p, int r,
 struct weighted_avg *wa = >wa[y*s->wa_linesize + x];
 
 // Also weight the centered pixel
-wa->total_weight += 1.0;
-wa->sum += 1.0 * src[y*src_linesize + x];
+wa->total_weight += 1.f;
+wa->sum += 1.f * src[y*src_linesize + x];
 
 dst[y*dst_linesize + x] = av_clip_uint8(wa->sum / 
wa->total_weight);
 }
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 10/10] lavfi/nlmeans: use unsigned for the integral patch value

2018-05-07 Thread Clément Bœsch
This value can not be negative.
---
 libavfilter/vf_nlmeans.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index 22d26a12e3..547cb80acd 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -64,7 +64,7 @@ typedef struct NLMeansContext {
 ptrdiff_t wa_linesize;  // linesize for wa in struct 
size unit
 float weight_lut[WEIGHT_LUT_SIZE];  // lookup table mapping 
(scaled) patch differences to their associated weights
 float pdiff_lut_scale;  // scale factor for patch 
differences before looking into the LUT
-int max_meaningful_diff;// maximum difference 
considered (if the patch difference is too high we ignore the pixel)
+uint32_t max_meaningful_diff;   // maximum difference 
considered (if the patch difference is too high we ignore the pixel)
 NLMeansDSPContext dsp;
 } NLMeansContext;
 
@@ -129,12 +129,12 @@ static int query_formats(AVFilterContext *ctx)
  * contains the sum of the squared difference of every corresponding pixels of
  * two input planes of the same size as M.
  */
-static inline int get_integral_patch_value(const uint32_t *ii, int ii_lz_32, 
int x, int y, int p)
+static inline uint32_t get_integral_patch_value(const uint32_t *ii, int 
ii_lz_32, int x, int y, int p)
 {
-const int a = ii[(y - p - 1) * ii_lz_32 + (x - p - 1)];
-const int b = ii[(y - p - 1) * ii_lz_32 + (x + p)];
-const int d = ii[(y + p) * ii_lz_32 + (x - p - 1)];
-const int e = ii[(y + p) * ii_lz_32 + (x + p)];
+const uint32_t a = ii[(y - p - 1) * ii_lz_32 + (x - p - 1)];
+const uint32_t b = ii[(y - p - 1) * ii_lz_32 + (x + p)];
+const uint32_t d = ii[(y + p) * ii_lz_32 + (x - p - 1)];
+const uint32_t e = ii[(y + p) * ii_lz_32 + (x + p)];
 return e - d - b + a;
 }
 
@@ -398,9 +398,9 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 const uint8_t *src = td->src + y*src_linesize;
 struct weighted_avg *wa = s->wa + y*s->wa_linesize;
 for (x = td->startx; x < td->endx; x++) {
-const int patch_diff_sq = get_integral_patch_value(td->ii_start, 
s->ii_lz_32, x, y, td->p);
+const uint32_t patch_diff_sq = 
get_integral_patch_value(td->ii_start, s->ii_lz_32, x, y, td->p);
 if (patch_diff_sq < s->max_meaningful_diff) {
-const int weight_lut_idx = patch_diff_sq * s->pdiff_lut_scale;
+const unsigned weight_lut_idx = patch_diff_sq * 
s->pdiff_lut_scale;
 const float weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
 wa[x].total_weight += weight;
 wa[x].sum += weight * src[x];
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 08/10] lavfi/nlmeans: move final weighted averaging out of nlmeans_plane

2018-05-07 Thread Clément Bœsch
This helps figuring out where the filter is slow:

  70.53%  ffmpeg_g  ffmpeg_g  [.] nlmeans_slice
  25.73%  ffmpeg_g  ffmpeg_g  [.] compute_safe_ssd_integral_image_c
   1.74%  ffmpeg_g  ffmpeg_g  [.] compute_unsafe_ssd_integral_image
   0.82%  ffmpeg_g  ffmpeg_g  [.] ff_mjpeg_decode_sos
   0.51%  ffmpeg_g  [unknown] [k] 0x91800a80
   0.24%  ffmpeg_g  ffmpeg_g  [.] weight_averages

(Tested with a large image that takes several seconds to process)

Since this function is irrelevant speed wise, the file's TODO is
updated.
---
 libavfilter/vf_nlmeans.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index aba587f46b..72a75a6e7a 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -20,7 +20,6 @@
 
 /**
  * @todo
- * - SIMD for final weighted averaging
  * - better automatic defaults? see "Parameters" @ 
http://www.ipol.im/pub/art/2011/bcm_nlm/
  * - temporal support (probably doesn't need any displacement according to
  *   "Denoising image sequences does not require motion estimation")
@@ -411,11 +410,30 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 return 0;
 }
 
+static void weight_averages(uint8_t *dst, ptrdiff_t dst_linesize,
+const uint8_t *src, ptrdiff_t src_linesize,
+struct weighted_avg *wa, ptrdiff_t wa_linesize,
+int w, int h)
+{
+int x, y;
+
+for (y = 0; y < h; y++) {
+for (x = 0; x < w; x++) {
+// Also weight the centered pixel
+wa[x].total_weight += 1.f;
+wa[x].sum += 1.f * src[x];
+dst[x] = av_clip_uint8(wa[x].sum / wa[x].total_weight);
+}
+dst += dst_linesize;
+src += src_linesize;
+wa += wa_linesize;
+}
+}
+
 static int nlmeans_plane(AVFilterContext *ctx, int w, int h, int p, int r,
  uint8_t *dst, ptrdiff_t dst_linesize,
  const uint8_t *src, ptrdiff_t src_linesize)
 {
-int x, y;
 int offx, offy;
 NLMeansContext *s = ctx->priv;
 /* patches center points cover the whole research window so the patches
@@ -448,17 +466,10 @@ static int nlmeans_plane(AVFilterContext *ctx, int w, int 
h, int p, int r,
 }
 }
 }
-for (y = 0; y < h; y++) {
-for (x = 0; x < w; x++) {
-struct weighted_avg *wa = >wa[y*s->wa_linesize + x];
 
-// Also weight the centered pixel
-wa->total_weight += 1.f;
-wa->sum += 1.f * src[y*src_linesize + x];
+weight_averages(dst, dst_linesize, src, src_linesize,
+s->wa, s->wa_linesize, w, h);
 
-dst[y*dst_linesize + x] = av_clip_uint8(wa->sum / 
wa->total_weight);
-}
-}
 return 0;
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 09/10] lavfi/nlmeans: reorder memory accesses in get_integral_patch_value

2018-05-07 Thread Clément Bœsch
This doesn't seem to make much of a difference but it can't hurt.
---
 libavfilter/vf_nlmeans.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index 72a75a6e7a..22d26a12e3 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -131,10 +131,10 @@ static int query_formats(AVFilterContext *ctx)
  */
 static inline int get_integral_patch_value(const uint32_t *ii, int ii_lz_32, 
int x, int y, int p)
 {
-const int e = ii[(y + p) * ii_lz_32 + (x + p)];
-const int d = ii[(y + p) * ii_lz_32 + (x - p - 1)];
-const int b = ii[(y - p - 1) * ii_lz_32 + (x + p)];
 const int a = ii[(y - p - 1) * ii_lz_32 + (x - p - 1)];
+const int b = ii[(y - p - 1) * ii_lz_32 + (x + p)];
+const int d = ii[(y + p) * ii_lz_32 + (x - p - 1)];
+const int e = ii[(y + p) * ii_lz_32 + (x + p)];
 return e - d - b + a;
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 06/10] lavfi/nlmeans: make compute_safe_ssd_integral_image_c faster

2018-05-07 Thread Clément Bœsch
before:  ssd_integral_image_c: 49204.6
after:   ssd_integral_image_c: 44272.8

Unrolling by 4 for made the biggest different on odroid-c2 (aarch64);
unrolling by 2 or 8 both raised 46k cycles vs 44k for 4.

Additionally, this is a much better reference when writing SIMD (SIMD
vectorization will just target 16 instead of 4).
---
 libavfilter/vf_nlmeans.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index c30e44498f..f37f1183f7 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -146,10 +146,6 @@ static inline int get_integral_patch_value(const uint32_t 
*ii, int ii_lz_32, int
  * function, we do not need any clipping here.
  *
  * The line above dst and the column to its left are always readable.
- *
- * This C version computes the SSD integral image using a scalar accumulator,
- * while for SIMD implementation it is likely more interesting to use the
- * two-loops algorithm variant.
  */
 static void compute_safe_ssd_integral_image_c(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
   const uint8_t *s1, ptrdiff_t 
linesize1,
@@ -157,21 +153,32 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, ptrdiff_t dst_lines
   int w, int h)
 {
 int x, y;
+const uint32_t *dst_top = dst - dst_linesize_32;
 
 /* SIMD-friendly assumptions allowed here */
 av_assert2(!(w & 0xf) && w >= 16 && h >= 1);
 
 for (y = 0; y < h; y++) {
-uint32_t acc = dst[-1] - dst[-dst_linesize_32 - 1];
-
-for (x = 0; x < w; x++) {
-const int d  = s1[x] - s2[x];
-acc += d * d;
-dst[x] = dst[-dst_linesize_32 + x] + acc;
+for (x = 0; x < w; x += 4) {
+const int d0 = s1[x] - s2[x];
+const int d1 = s1[x + 1] - s2[x + 1];
+const int d2 = s1[x + 2] - s2[x + 2];
+const int d3 = s1[x + 3] - s2[x + 3];
+
+dst[x] = dst_top[x] - dst_top[x - 1] + d0*d0;
+dst[x + 1] = dst_top[x + 1] - dst_top[x] + d1*d1;
+dst[x + 2] = dst_top[x + 2] - dst_top[x + 1] + d2*d2;
+dst[x + 3] = dst_top[x + 3] - dst_top[x + 2] + d3*d3;
+
+dst[x] += dst[x - 1];
+dst[x + 1] += dst[x];
+dst[x + 2] += dst[x + 1];
+dst[x + 3] += dst[x + 2];
 }
 s1  += linesize1;
 s2  += linesize2;
 dst += dst_linesize_32;
+dst_top += dst_linesize_32;
 }
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 05/10] checkasm: add vf_nlmeans test for ssd_integral_image

2018-05-07 Thread Clément Bœsch
---
 tests/checkasm/Makefile |   1 +
 tests/checkasm/checkasm.c   |   3 +
 tests/checkasm/checkasm.h   |   1 +
 tests/checkasm/vf_nlmeans.c | 113 
 4 files changed, 118 insertions(+)
 create mode 100644 tests/checkasm/vf_nlmeans.c

diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
index 0233d2f989..9484acbbd7 100644
--- a/tests/checkasm/Makefile
+++ b/tests/checkasm/Makefile
@@ -35,6 +35,7 @@ AVFILTEROBJS-$(CONFIG_BLEND_FILTER) += vf_blend.o
 AVFILTEROBJS-$(CONFIG_COLORSPACE_FILTER) += vf_colorspace.o
 AVFILTEROBJS-$(CONFIG_HFLIP_FILTER)  += vf_hflip.o
 AVFILTEROBJS-$(CONFIG_THRESHOLD_FILTER)  += vf_threshold.o
+AVFILTEROBJS-$(CONFIG_NLMEANS_FILTER)+= vf_nlmeans.o
 
 CHECKASMOBJS-$(CONFIG_AVFILTER) += $(AVFILTEROBJS-yes)
 
diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index ba1d1d0253..721a0912fb 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -159,6 +159,9 @@ static const struct {
 #if CONFIG_HFLIP_FILTER
 { "vf_hflip", checkasm_check_vf_hflip },
 #endif
+#if CONFIG_NLMEANS_FILTER
+{ "vf_nlmeans", checkasm_check_nlmeans },
+#endif
 #if CONFIG_THRESHOLD_FILTER
 { "vf_threshold", checkasm_check_vf_threshold },
 #endif
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index dcab74de06..c45cfb46f8 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -62,6 +62,7 @@ void checkasm_check_huffyuvdsp(void);
 void checkasm_check_jpeg2000dsp(void);
 void checkasm_check_llviddsp(void);
 void checkasm_check_llviddspenc(void);
+void checkasm_check_nlmeans(void);
 void checkasm_check_pixblockdsp(void);
 void checkasm_check_sbrdsp(void);
 void checkasm_check_synth_filter(void);
diff --git a/tests/checkasm/vf_nlmeans.c b/tests/checkasm/vf_nlmeans.c
new file mode 100644
index 00..5e2c934226
--- /dev/null
+++ b/tests/checkasm/vf_nlmeans.c
@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2018 Clément Bœsch 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "checkasm.h"
+#include "libavfilter/vf_nlmeans.h"
+#include "libavutil/avassert.h"
+
+#define randomize_buffer(buf, size) do {\
+int i;  \
+for (i = 0; i < size / 4; i++)  \
+((uint32_t *)buf)[i] = rnd();   \
+} while (0)
+
+void checkasm_check_nlmeans(void)
+{
+NLMeansDSPContext dsp = {0};
+
+const int w = 123;  // source width
+const int h = 45;   // source height
+const int p = 3;// patch half size
+const int r = 2;// research window half size
+
+ff_nlmeans_init();
+
+/* See the filter's code for the explanations on the variables */
+if (check_func(dsp.compute_safe_ssd_integral_image, "ssd_integral_image")) 
{
+int offx, offy;
+const int e = p + r;
+const int ii_w = w + e*2;
+const int ii_h = h + e*2;
+const int ii_lz_32 = FFALIGN(ii_w + 1, 4);
+uint32_t *ii_orig_ref = av_mallocz_array(ii_h + 1, ii_lz_32 * 
sizeof(*ii_orig_ref));
+uint32_t *ii_ref = ii_orig_ref + ii_lz_32 + 1;
+uint32_t *ii_orig_new = av_mallocz_array(ii_h + 1, ii_lz_32 * 
sizeof(*ii_orig_new));
+uint32_t *ii_new = ii_orig_new + ii_lz_32 + 1;
+const int src_lz = FFALIGN(w, 16);
+uint8_t *src = av_mallocz_array(h, src_lz);
+
+declare_func(void, uint32_t *dst, ptrdiff_t dst_linesize_32,
+ const uint8_t *s1, ptrdiff_t linesize1,
+ const uint8_t *s2, ptrdiff_t linesize2,
+ int w, int h);
+
+randomize_buffer(src, h * src_lz);
+
+for (offy = -r; offy <= r; offy++) {
+for (offx = -r; offx <= r; offx++) {
+if (offx || offy) {
+const int s1x = e;
+const int s1y = e;
+const int s2x = e + offx;
+const int s2y = e + offy;
+const int startx_safe = FFMAX(s1x, s2x);
+const int starty_safe = FFMAX(s1y, s2y);
+const int u_endx_safe = FFMIN(s1x + w, s2x + w);
+const in

[FFmpeg-devel] [PATCH v2 02/10] lavfi/nlmeans: add SIMD-friendly assumptions for compute_safe_ssd_integral_image

2018-05-07 Thread Clément Bœsch
SIMD code will not have to deal with padding itself. Overwriting in that
function may have been possible but involve large overreading of the
sources. Instead, we simply make sure the width to process is always a
multiple of 16. Additionally, there must be some actual area to process
so the SIMD code can have its boundary checks after processing the first
pixels.
---
 libavfilter/vf_nlmeans.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index d222d3913e..3f0a43ee72 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -157,6 +157,9 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, int dst_linesize_32
 {
 int x, y;
 
+/* SIMD-friendly assumptions allowed here */
+av_assert2(!(w & 0xf) && w >= 16 && h >= 1);
+
 for (y = 0; y < h; y++) {
 uint32_t acc = dst[-1] - dst[-dst_linesize_32 - 1];
 
@@ -257,9 +260,16 @@ static void compute_ssd_integral_image(uint32_t *ii, int 
ii_linesize_32,
 // to compare the 2 sources pixels
 const int startx_safe = FFMAX(s1x, s2x);
 const int starty_safe = FFMAX(s1y, s2y);
-const int endx_safe   = FFMIN(s1x + w, s2x + w);
+const int u_endx_safe = FFMIN(s1x + w, s2x + w); // unaligned
 const int endy_safe   = FFMIN(s1y + h, s2y + h);
 
+// deduce the safe area width and height
+const int safe_pw = (u_endx_safe - startx_safe) & ~0xf;
+const int safe_ph = endy_safe - starty_safe;
+
+// adjusted end x position of the safe area after width of the safe area 
gets aligned
+const int endx_safe = startx_safe + safe_pw;
+
 // top part where only one of s1 and s2 is still readable, or none at all
 compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
   0, 0,
@@ -273,24 +283,25 @@ static void compute_ssd_integral_image(uint32_t *ii, int 
ii_linesize_32,
   0, starty_safe,
   src, linesize,
   offx, offy, e, w, h,
-  startx_safe, endy_safe - starty_safe);
+  startx_safe, safe_ph);
 
 // main and safe part of the integral
 av_assert1(startx_safe - s1x >= 0); av_assert1(startx_safe - s1x < w);
 av_assert1(starty_safe - s1y >= 0); av_assert1(starty_safe - s1y < h);
 av_assert1(startx_safe - s2x >= 0); av_assert1(startx_safe - s2x < w);
 av_assert1(starty_safe - s2y >= 0); av_assert1(starty_safe - s2y < h);
-compute_safe_ssd_integral_image_c(ii + starty_safe*ii_linesize_32 + 
startx_safe, ii_linesize_32,
-  src + (starty_safe - s1y) * linesize + 
(startx_safe - s1x), linesize,
-  src + (starty_safe - s2y) * linesize + 
(startx_safe - s2x), linesize,
-  endx_safe - startx_safe, endy_safe - 
starty_safe);
+if (safe_pw && safe_ph)
+compute_safe_ssd_integral_image_c(ii + starty_safe*ii_linesize_32 + 
startx_safe, ii_linesize_32,
+  src + (starty_safe - s1y) * linesize 
+ (startx_safe - s1x), linesize,
+  src + (starty_safe - s2y) * linesize 
+ (startx_safe - s2x), linesize,
+  safe_pw, safe_ph);
 
 // right part of the integral
 compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
   endx_safe, starty_safe,
   src, linesize,
   offx, offy, e, w, h,
-  ii_w - endx_safe, endy_safe - 
starty_safe);
+  ii_w - endx_safe, safe_ph);
 
 // bottom part where only one of s1 and s2 is still readable, or none at 
all
 compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH v2 04/10] lavfi/nlmeans: add AArch64 SIMD for compute_safe_ssd_integral_image

2018-05-07 Thread Clément Bœsch
ssd_integral_image_c: 49204.6
ssd_integral_image_neon: 28346.8
---
 libavfilter/aarch64/Makefile  |  3 +
 libavfilter/aarch64/vf_nlmeans_init.c | 33 +++
 libavfilter/aarch64/vf_nlmeans_neon.S | 80 +++
 libavfilter/vf_nlmeans.c  | 26 ++---
 libavfilter/vf_nlmeans.h  | 35 
 5 files changed, 170 insertions(+), 7 deletions(-)
 create mode 100644 libavfilter/aarch64/Makefile
 create mode 100644 libavfilter/aarch64/vf_nlmeans_init.c
 create mode 100644 libavfilter/aarch64/vf_nlmeans_neon.S
 create mode 100644 libavfilter/vf_nlmeans.h

diff --git a/libavfilter/aarch64/Makefile b/libavfilter/aarch64/Makefile
new file mode 100644
index 00..b58daa3a3f
--- /dev/null
+++ b/libavfilter/aarch64/Makefile
@@ -0,0 +1,3 @@
+OBJS-$(CONFIG_NLMEANS_FILTER)+= aarch64/vf_nlmeans_init.o
+
+NEON-OBJS-$(CONFIG_NLMEANS_FILTER)   += aarch64/vf_nlmeans_neon.o
diff --git a/libavfilter/aarch64/vf_nlmeans_init.c 
b/libavfilter/aarch64/vf_nlmeans_init.c
new file mode 100644
index 00..a1edefb144
--- /dev/null
+++ b/libavfilter/aarch64/vf_nlmeans_init.c
@@ -0,0 +1,33 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/aarch64/cpu.h"
+#include "libavfilter/vf_nlmeans.h"
+
+void ff_compute_safe_ssd_integral_image_neon(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
+ const uint8_t *s1, ptrdiff_t 
linesize1,
+ const uint8_t *s2, ptrdiff_t 
linesize2,
+ int w, int h);
+
+av_cold void ff_nlmeans_init_aarch64(NLMeansDSPContext *dsp)
+{
+int cpu_flags = av_get_cpu_flags();
+
+if (have_neon(cpu_flags))
+dsp->compute_safe_ssd_integral_image = 
ff_compute_safe_ssd_integral_image_neon;
+}
diff --git a/libavfilter/aarch64/vf_nlmeans_neon.S 
b/libavfilter/aarch64/vf_nlmeans_neon.S
new file mode 100644
index 00..6308a428db
--- /dev/null
+++ b/libavfilter/aarch64/vf_nlmeans_neon.S
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2018 Clément Bœsch 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/aarch64/asm.S"
+
+// acc_sum_store(ABCD) = {X+A, X+A+B, X+A+B+C, X+A+B+C+D}
+.macro acc_sum_store x, xb
+dup v24.4S, v24.4S[3]   // 
...X -> 
+ext v25.16B, v26.16B, \xb, #12  // 
ext(,ABCD,12)=0ABC
+add v24.4S, v24.4S, \x  // 
+ABCD={X+A,X+B,X+C,X+D}
+add v24.4S, v24.4S, v25.4S  // 
{X+A,X+B+A,X+C+B,X+D+C}   (+0ABC)
+ext v25.16B, v26.16B, v25.16B, #12  // 
ext(,0ABC,12)=00AB
+add v24.4S, v24.4S, v25.4S  // 
{X+A,X+B+A,X+C+B+A,X+D+C+B}   (+00AB)
+ext v25.16B, v26.16B, v25.16B, #12  // 
ext(,00AB,12)=000A
+add v24.4S, v24.4S, v25.4S  // 
{X+A,X+B+A,X+C+B+A,X+D+C+B+A} (+000A)
+st1 {v24.4S}, [x0], #16 // 
write 4x32-bit final values
+.endm
+
+function ff_compute_safe_ssd_integral_image_neon, export=1
+moviv26.4S, #0  // 
used as zero for the "rotations" in acc_sum_store
+sub x3, x3, w6, UXTW 

[FFmpeg-devel] [PATCH v2 03/10] lavfi/nlmeans: use ptrdiff_t for linesizes

2018-05-07 Thread Clément Bœsch
Similarly to previous commit, this will help writing SIMD code by not
having manual zero-extension in SIMD code
---
 libavfilter/vf_nlmeans.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index 3f0a43ee72..b081a4e5af 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -60,9 +60,9 @@ typedef struct NLMeansContext {
 uint32_t *ii_orig;  // integral image
 uint32_t *ii;   // integral image starting 
after the 0-line and 0-column
 int ii_w, ii_h; // width and height of the 
integral image
-int ii_lz_32;   // linesize in 32-bit units of 
the integral image
+ptrdiff_t ii_lz_32; // linesize in 32-bit units of 
the integral image
 struct weighted_avg *wa;// weighted average of every 
pixel
-int wa_linesize;// linesize for wa in struct 
size unit
+ptrdiff_t wa_linesize;  // linesize for wa in struct 
size unit
 double weight_lut[WEIGHT_LUT_SIZE]; // lookup table mapping 
(scaled) patch differences to their associated weights
 double pdiff_lut_scale; // scale factor for patch 
differences before looking into the LUT
 int max_meaningful_diff;// maximum difference 
considered (if the patch difference is too high we ignore the pixel)
@@ -150,9 +150,9 @@ static inline int get_integral_patch_value(const uint32_t 
*ii, int ii_lz_32, int
  * while for SIMD implementation it is likely more interesting to use the
  * two-loops algorithm variant.
  */
-static void compute_safe_ssd_integral_image_c(uint32_t *dst, int 
dst_linesize_32,
-  const uint8_t *s1, int linesize1,
-  const uint8_t *s2, int linesize2,
+static void compute_safe_ssd_integral_image_c(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
+  const uint8_t *s1, ptrdiff_t 
linesize1,
+  const uint8_t *s2, ptrdiff_t 
linesize2,
   int w, int h)
 {
 int x, y;
@@ -198,9 +198,9 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, int dst_linesize_32
  * @param w width to compute
  * @param h height to compute
  */
-static inline void compute_unsafe_ssd_integral_image(uint32_t *dst, int 
dst_linesize_32,
+static inline void compute_unsafe_ssd_integral_image(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
  int startx, int starty,
- const uint8_t *src, int 
linesize,
+ const uint8_t *src, 
ptrdiff_t linesize,
  int offx, int offy, int 
r, int sw, int sh,
  int w, int h)
 {
@@ -240,8 +240,8 @@ static inline void 
compute_unsafe_ssd_integral_image(uint32_t *dst, int dst_line
  * @param h source height
  * @param e research padding edge
  */
-static void compute_ssd_integral_image(uint32_t *ii, int ii_linesize_32,
-   const uint8_t *src, int linesize, int 
offx, int offy,
+static void compute_ssd_integral_image(uint32_t *ii, ptrdiff_t ii_linesize_32,
+   const uint8_t *src, ptrdiff_t linesize, 
int offx, int offy,
int e, int w, int h)
 {
 // ii has a surrounding padding of thickness "e"
@@ -367,7 +367,7 @@ static int config_input(AVFilterLink *inlink)
 
 struct thread_data {
 const uint8_t *src;
-int src_linesize;
+ptrdiff_t src_linesize;
 int startx, starty;
 int endx, endy;
 const uint32_t *ii_start;
@@ -379,7 +379,7 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 int x, y;
 NLMeansContext *s = ctx->priv;
 const struct thread_data *td = arg;
-const int src_linesize = td->src_linesize;
+const ptrdiff_t src_linesize = td->src_linesize;
 const int process_h = td->endy - td->starty;
 const int slice_start = (process_h *  jobnr   ) / nb_jobs;
 const int slice_end   = (process_h * (jobnr+1)) / nb_jobs;
@@ -403,8 +403,8 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 }
 
 static int nlmeans_plane(AVFilterContext *ctx, int w, int h, int p, int r,
- uint8_t *dst, int dst_linesize,
- const uint8_t *src, int src_linesize)
+ uint8_t *dst, ptrdiff_t dst_linesize,
+ const uint8_t *src, ptrdiff_t 

[FFmpeg-devel] [PATCH v2 01/10] lavfi/nlmeans: random code shuffling to help compiler

2018-05-07 Thread Clément Bœsch
This makes nlmeans_slice() slightly faster at least on GCC 7.3.
---
 libavfilter/vf_nlmeans.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index e4952e187e..d222d3913e 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -368,7 +368,6 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 int x, y;
 NLMeansContext *s = ctx->priv;
 const struct thread_data *td = arg;
-const uint8_t *src = td->src;
 const int src_linesize = td->src_linesize;
 const int process_h = td->endy - td->starty;
 const int slice_start = (process_h *  jobnr   ) / nb_jobs;
@@ -377,14 +376,15 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 const int endy   = td->starty + slice_end;
 
 for (y = starty; y < endy; y++) {
+const uint8_t *src = td->src + y*src_linesize;
+struct weighted_avg *wa = s->wa + y*s->wa_linesize;
 for (x = td->startx; x < td->endx; x++) {
 const int patch_diff_sq = get_integral_patch_value(td->ii_start, 
s->ii_lz_32, x, y, td->p);
 if (patch_diff_sq < s->max_meaningful_diff) {
-struct weighted_avg *wa = >wa[y*s->wa_linesize + x];
 const int weight_lut_idx = patch_diff_sq * s->pdiff_lut_scale;
 const double weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
-wa->total_weight += weight;
-wa->sum += weight * src[y*src_linesize + x];
+wa[x].total_weight += weight;
+wa[x].sum += weight * src[x];
 }
 }
 }
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] Misc improvements in nlmeans filter [v2]

2018-05-07 Thread Clément Bœsch
Changes since v1:

- fixed float operation in double as pointed out by Moritz
- fix broken commit split as pointed out by Michael
- added patch 10: "use unsigned for the integral patch"
- misc instruction shuffling in AArch64 SIMD for better performances

I plan to push this soon unless someone wants more time to review.

BTW, x86 SIMD patch welcome, the filter badly needs some performance
improvements. Also, any suggestion on how not to make it spend 80% of
the time in nlmeans_slice() welcome.

Regards,


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 7/9] lavfi/nlmeans: switch from double to float

2018-05-07 Thread Clément Bœsch
On Sun, May 06, 2018 at 04:53:54PM +0200, Moritz Barsnick wrote:
> On Sun, May 06, 2018 at 13:40:58 +0200, Clément Bœsch wrote:
> > Overall speed appears to be 1.1x faster with no noticeable quality impact.
> 
> Probably platform dependant?
> 
> >  struct weighted_avg {
> > -double total_weight;
> > -double sum;
> > +float total_weight;
> > +float sum;
> >  };
> 
> I believe these calculaions in nlmeans_plane() will promote to double
> before being cast back to float:
> 
>// Also weight the centered pixel
> wa->total_weight += 1.0;
> wa->sum += 1.0 * src[y*src_linesize + x];
> 
> (At least the second one. The first one - just an assignment of a
> constant - is covered by the preprocessor, IIUC.) They need to use
> "1.0f".
> 

It doesn't really matter here actually, in "lavfi/nlmeans: move final
weighted averaging out of nlmeans_plane" you can see that this code
represents 0.24% of the CPU time. I fixed it locally anyway, thanks.

> (There are others, but only in init(), which don't matter for
> performance.)

Yeah, I left these to double on purpose.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/9] lavfi/nlmeans: add SIMD-friendly assumptions for compute_safe_ssd_integral_image

2018-05-07 Thread Clément Bœsch
On Mon, May 07, 2018 at 12:14:37AM +0200, Michael Niedermayer wrote:
> On Sun, May 06, 2018 at 01:40:53PM +0200, Clément Bœsch wrote:
> > SIMD code will not have to deal with padding itself. Overwriting in that
> > function may have been possible but involve large overreading of the
> > sources. Instead, we simply make sure the width to process is always a
> > multiple of 16. Additionally, there must be some actual area to process
> > so the SIMD code can have its boundary checks after processing the first
> > pixels.
> > ---
> >  libavfilter/vf_nlmeans.c | 25 ++---
> >  1 file changed, 18 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
> > index d222d3913e..21f981a605 100644
> > --- a/libavfilter/vf_nlmeans.c
> > +++ b/libavfilter/vf_nlmeans.c
> > @@ -157,6 +157,9 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
> > *dst, int dst_linesize_32
> >  {
> >  int x, y;
> >  
> > +/* SIMD-friendly assumptions allowed here */
> > +av_assert2(!(w & 0xf) && w >= 16 && h >= 1);
> > +
> >  for (y = 0; y < h; y++) {
> >  uint32_t acc = dst[-1] - dst[-dst_linesize_32 - 1];
> >  
> > @@ -257,9 +260,16 @@ static void compute_ssd_integral_image(uint32_t *ii, 
> > int ii_linesize_32,
> >  // to compare the 2 sources pixels
> >  const int startx_safe = FFMAX(s1x, s2x);
> >  const int starty_safe = FFMAX(s1y, s2y);
> > -const int endx_safe   = FFMIN(s1x + w, s2x + w);
> > +const int u_endx_safe = FFMIN(s1x + w, s2x + w); // unaligned
> >  const int endy_safe   = FFMIN(s1y + h, s2y + h);
> >  
> > +// deduce the safe area width and height
> > +const int safe_pw = (u_endx_safe - startx_safe) & ~0xf;
> > +const int safe_ph = endy_safe - starty_safe;
> > +
> > +// adjusted end x position of the safe area after width of the safe 
> > area gets aligned
> > +const int endx_safe = startx_safe + safe_pw;
> > +
> >  // top part where only one of s1 and s2 is still readable, or none at 
> > all
> >  compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
> >0, 0,
> > @@ -273,24 +283,25 @@ static void compute_ssd_integral_image(uint32_t *ii, 
> > int ii_linesize_32,
> >0, starty_safe,
> >src, linesize,
> >offx, offy, e, w, h,
> > -  startx_safe, endy_safe - 
> > starty_safe);
> > +  startx_safe, safe_ph);
> >  
> >  // main and safe part of the integral
> >  av_assert1(startx_safe - s1x >= 0); av_assert1(startx_safe - s1x < w);
> >  av_assert1(starty_safe - s1y >= 0); av_assert1(starty_safe - s1y < h);
> >  av_assert1(startx_safe - s2x >= 0); av_assert1(startx_safe - s2x < w);
> >  av_assert1(starty_safe - s2y >= 0); av_assert1(starty_safe - s2y < h);
> > -compute_safe_ssd_integral_image_c(ii + starty_safe*ii_linesize_32 + 
> > startx_safe, ii_linesize_32,
> > -  src + (starty_safe - s1y) * linesize 
> > + (startx_safe - s1x), linesize,
> > -  src + (starty_safe - s2y) * linesize 
> > + (startx_safe - s2x), linesize,
> > -  endx_safe - startx_safe, endy_safe - 
> > starty_safe);
> > +if (safe_pw && safe_ph)
> > +dsp->compute_safe_ssd_integral_image(ii + 
> > starty_safe*ii_linesize_32 + startx_safe, ii_linesize_32,
> > + src + (starty_safe - s1y) * 
> > linesize + (startx_safe - s1x), linesize,
> > + src + (starty_safe - s2y) * 
> > linesize + (startx_safe - s2x), linesize,
> > + safe_pw, safe_ph);
> 
> 
> i think this is or i am missing some change
> 
> libavfilter/vf_nlmeans.c: In function ‘compute_ssd_integral_image’:
> libavfilter/vf_nlmeans.c:294:9: error: ‘dsp’ undeclared (first use in this 
> function)
>  dsp->compute_safe_ssd_integral_image(ii + starty_safe*ii_linesize_32 
> + startx_safe, ii_linesize_32,
>  ^
> libavfilter/vf_nlmeans.c:294:9: note: each undeclared identifier is reported 
> only once for each function it appears in
> libavfilter/vf_nlmeans.c: At top level:
> lib

Re: [FFmpeg-devel] [PATCH] opt: print a deprecation indicator when listing options

2018-05-06 Thread Clément Bœsch
On Sat, May 05, 2018 at 09:45:26PM +0100, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  libavutil/opt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 99282605f5..73295356a1 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1276,6 +1276,8 @@ static void opt_list(void *obj, void *av_log_obj, const 
> char *unit,
>  }
>  av_log(av_log_obj, AV_LOG_INFO, ")");
>  }
> +if (opt->flags & AV_OPT_FLAG_DEPRECATED)
> +av_log(av_log_obj, AV_LOG_INFO, " (deprecated)");
>  

does this fit well with the help message that is supposed to be updated to
say what to use instead of that deprecated field?

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 7/9] lavfi/nlmeans: switch from double to float

2018-05-06 Thread Clément Bœsch
Overall speed appears to be 1.1x faster with no noticeable quality impact.
---
 libavfilter/vf_nlmeans.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index f37f1183f7..201e4feb41 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -40,8 +40,8 @@
 #include "video.h"
 
 struct weighted_avg {
-double total_weight;
-double sum;
+float total_weight;
+float sum;
 };
 
 #define WEIGHT_LUT_NBITS 9
@@ -63,8 +63,8 @@ typedef struct NLMeansContext {
 ptrdiff_t ii_lz_32; // linesize in 32-bit units of 
the integral image
 struct weighted_avg *wa;// weighted average of every 
pixel
 ptrdiff_t wa_linesize;  // linesize for wa in struct 
size unit
-double weight_lut[WEIGHT_LUT_SIZE]; // lookup table mapping 
(scaled) patch differences to their associated weights
-double pdiff_lut_scale; // scale factor for patch 
differences before looking into the LUT
+float weight_lut[WEIGHT_LUT_SIZE];  // lookup table mapping 
(scaled) patch differences to their associated weights
+float pdiff_lut_scale;  // scale factor for patch 
differences before looking into the LUT
 int max_meaningful_diff;// maximum difference 
considered (if the patch difference is too high we ignore the pixel)
 NLMeansDSPContext dsp;
 } NLMeansContext;
@@ -206,7 +206,7 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, ptrdiff_t dst_lines
  * @param w width to compute
  * @param h height to compute
  */
-static inline void compute_unsafe_ssd_integral_image(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
+static void compute_unsafe_ssd_integral_image(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
  int startx, int starty,
  const uint8_t *src, 
ptrdiff_t linesize,
  int offx, int offy, int 
r, int sw, int sh,
@@ -402,7 +402,7 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 const int patch_diff_sq = get_integral_patch_value(td->ii_start, 
s->ii_lz_32, x, y, td->p);
 if (patch_diff_sq < s->max_meaningful_diff) {
 const int weight_lut_idx = patch_diff_sq * s->pdiff_lut_scale;
-const double weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
+const float weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
 wa[x].total_weight += weight;
 wa[x].sum += weight * src[x];
 }
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 8/9] lavfi/nlmeans: move final weighted averaging out of nlmeans_plane

2018-05-06 Thread Clément Bœsch
This helps figuring out where the filter is slow:

  70.53%  ffmpeg_g  ffmpeg_g  [.] nlmeans_slice
  25.73%  ffmpeg_g  ffmpeg_g  [.] compute_safe_ssd_integral_image_c
   1.74%  ffmpeg_g  ffmpeg_g  [.] compute_unsafe_ssd_integral_image
   0.82%  ffmpeg_g  ffmpeg_g  [.] ff_mjpeg_decode_sos
   0.51%  ffmpeg_g  [unknown] [k] 0x91800a80
   0.24%  ffmpeg_g  ffmpeg_g  [.] weight_averages

(Tested with a large image that takes several seconds to process)

Since this function is irrelevant speed wise, the file's TODO is
updated.
---
 libavfilter/vf_nlmeans.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index 201e4feb41..abe708a2fc 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -20,7 +20,6 @@
 
 /**
  * @todo
- * - SIMD for final weighted averaging
  * - better automatic defaults? see "Parameters" @ 
http://www.ipol.im/pub/art/2011/bcm_nlm/
  * - temporal support (probably doesn't need any displacement according to
  *   "Denoising image sequences does not require motion estimation")
@@ -411,11 +410,30 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 return 0;
 }
 
+static void weight_averages(uint8_t *dst, ptrdiff_t dst_linesize,
+const uint8_t *src, ptrdiff_t src_linesize,
+struct weighted_avg *wa, ptrdiff_t wa_linesize,
+int w, int h)
+{
+int x, y;
+
+for (y = 0; y < h; y++) {
+for (x = 0; x < w; x++) {
+// Also weight the centered pixel
+wa[x].total_weight += 1.0;
+wa[x].sum += 1.0 * src[x];
+dst[x] = av_clip_uint8(wa[x].sum / wa[x].total_weight);
+}
+dst += dst_linesize;
+src += src_linesize;
+wa += wa_linesize;
+}
+}
+
 static int nlmeans_plane(AVFilterContext *ctx, int w, int h, int p, int r,
  uint8_t *dst, ptrdiff_t dst_linesize,
  const uint8_t *src, ptrdiff_t src_linesize)
 {
-int x, y;
 int offx, offy;
 NLMeansContext *s = ctx->priv;
 /* patches center points cover the whole research window so the patches
@@ -448,17 +466,10 @@ static int nlmeans_plane(AVFilterContext *ctx, int w, int 
h, int p, int r,
 }
 }
 }
-for (y = 0; y < h; y++) {
-for (x = 0; x < w; x++) {
-struct weighted_avg *wa = >wa[y*s->wa_linesize + x];
 
-// Also weight the centered pixel
-wa->total_weight += 1.0;
-wa->sum += 1.0 * src[y*src_linesize + x];
+weight_averages(dst, dst_linesize, src, src_linesize,
+s->wa, s->wa_linesize, w, h);
 
-dst[y*dst_linesize + x] = av_clip_uint8(wa->sum / 
wa->total_weight);
-}
-}
 return 0;
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 6/9] lavfi/nlmeans: make compute_safe_ssd_integral_image_c faster

2018-05-06 Thread Clément Bœsch
before:  ssd_integral_image_c: 49204.6
after:   ssd_integral_image_c: 44272.8

Unrolling by 4 for made the biggest different on odroid-c2 (aarch64);
unrolling by 2 or 8 both raised 46k cycles vs 44k for 4.

Additionally, this is a much better reference when writing SIMD (SIMD
vectorization will just target 16 instead of 4).
---
 libavfilter/vf_nlmeans.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index c30e44498f..f37f1183f7 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -146,10 +146,6 @@ static inline int get_integral_patch_value(const uint32_t 
*ii, int ii_lz_32, int
  * function, we do not need any clipping here.
  *
  * The line above dst and the column to its left are always readable.
- *
- * This C version computes the SSD integral image using a scalar accumulator,
- * while for SIMD implementation it is likely more interesting to use the
- * two-loops algorithm variant.
  */
 static void compute_safe_ssd_integral_image_c(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
   const uint8_t *s1, ptrdiff_t 
linesize1,
@@ -157,21 +153,32 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, ptrdiff_t dst_lines
   int w, int h)
 {
 int x, y;
+const uint32_t *dst_top = dst - dst_linesize_32;
 
 /* SIMD-friendly assumptions allowed here */
 av_assert2(!(w & 0xf) && w >= 16 && h >= 1);
 
 for (y = 0; y < h; y++) {
-uint32_t acc = dst[-1] - dst[-dst_linesize_32 - 1];
-
-for (x = 0; x < w; x++) {
-const int d  = s1[x] - s2[x];
-acc += d * d;
-dst[x] = dst[-dst_linesize_32 + x] + acc;
+for (x = 0; x < w; x += 4) {
+const int d0 = s1[x] - s2[x];
+const int d1 = s1[x + 1] - s2[x + 1];
+const int d2 = s1[x + 2] - s2[x + 2];
+const int d3 = s1[x + 3] - s2[x + 3];
+
+dst[x] = dst_top[x] - dst_top[x - 1] + d0*d0;
+dst[x + 1] = dst_top[x + 1] - dst_top[x] + d1*d1;
+dst[x + 2] = dst_top[x + 2] - dst_top[x + 1] + d2*d2;
+dst[x + 3] = dst_top[x + 3] - dst_top[x + 2] + d3*d3;
+
+dst[x] += dst[x - 1];
+dst[x + 1] += dst[x];
+dst[x + 2] += dst[x + 1];
+dst[x + 3] += dst[x + 2];
 }
 s1  += linesize1;
 s2  += linesize2;
 dst += dst_linesize_32;
+dst_top += dst_linesize_32;
 }
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 9/9] lavfi/nlmeans: reorder memory accesses in get_integral_patch_value

2018-05-06 Thread Clément Bœsch
This doesn't seem to make much of a difference but it can't hurt.
---
 libavfilter/vf_nlmeans.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index abe708a2fc..38c50bc94a 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -131,10 +131,10 @@ static int query_formats(AVFilterContext *ctx)
  */
 static inline int get_integral_patch_value(const uint32_t *ii, int ii_lz_32, 
int x, int y, int p)
 {
-const int e = ii[(y + p) * ii_lz_32 + (x + p)];
-const int d = ii[(y + p) * ii_lz_32 + (x - p - 1)];
-const int b = ii[(y - p - 1) * ii_lz_32 + (x + p)];
 const int a = ii[(y - p - 1) * ii_lz_32 + (x - p - 1)];
+const int b = ii[(y - p - 1) * ii_lz_32 + (x + p)];
+const int d = ii[(y + p) * ii_lz_32 + (x - p - 1)];
+const int e = ii[(y + p) * ii_lz_32 + (x + p)];
 return e - d - b + a;
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 4/9] lavfi/nlmeans: add AArch64 SIMD for compute_safe_ssd_integral_image

2018-05-06 Thread Clément Bœsch
ssd_integral_image_c: 49204.6
ssd_integral_image_neon: 28346.8
---
 libavfilter/aarch64/Makefile  |  3 ++
 libavfilter/aarch64/vf_nlmeans_init.c | 33 
 libavfilter/aarch64/vf_nlmeans_neon.S | 78 +++
 libavfilter/vf_nlmeans.c  | 18 +--
 libavfilter/vf_nlmeans.h  | 35 
 5 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 libavfilter/aarch64/Makefile
 create mode 100644 libavfilter/aarch64/vf_nlmeans_init.c
 create mode 100644 libavfilter/aarch64/vf_nlmeans_neon.S
 create mode 100644 libavfilter/vf_nlmeans.h

diff --git a/libavfilter/aarch64/Makefile b/libavfilter/aarch64/Makefile
new file mode 100644
index 00..b58daa3a3f
--- /dev/null
+++ b/libavfilter/aarch64/Makefile
@@ -0,0 +1,3 @@
+OBJS-$(CONFIG_NLMEANS_FILTER)+= aarch64/vf_nlmeans_init.o
+
+NEON-OBJS-$(CONFIG_NLMEANS_FILTER)   += aarch64/vf_nlmeans_neon.o
diff --git a/libavfilter/aarch64/vf_nlmeans_init.c 
b/libavfilter/aarch64/vf_nlmeans_init.c
new file mode 100644
index 00..a1edefb144
--- /dev/null
+++ b/libavfilter/aarch64/vf_nlmeans_init.c
@@ -0,0 +1,33 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/aarch64/cpu.h"
+#include "libavfilter/vf_nlmeans.h"
+
+void ff_compute_safe_ssd_integral_image_neon(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
+ const uint8_t *s1, ptrdiff_t 
linesize1,
+ const uint8_t *s2, ptrdiff_t 
linesize2,
+ int w, int h);
+
+av_cold void ff_nlmeans_init_aarch64(NLMeansDSPContext *dsp)
+{
+int cpu_flags = av_get_cpu_flags();
+
+if (have_neon(cpu_flags))
+dsp->compute_safe_ssd_integral_image = 
ff_compute_safe_ssd_integral_image_neon;
+}
diff --git a/libavfilter/aarch64/vf_nlmeans_neon.S 
b/libavfilter/aarch64/vf_nlmeans_neon.S
new file mode 100644
index 00..4de573cf7f
--- /dev/null
+++ b/libavfilter/aarch64/vf_nlmeans_neon.S
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2018 Clément Bœsch 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/aarch64/asm.S"
+
+// acc_sum_store(ABCD) = {X+A, X+A+B, X+A+B+C, X+A+B+C+D}
+.macro acc_sum_store x, xb
+dup v24.4S, v24.4S[3]   // 
...X -> 
+ext v25.16B, v26.16B, \xb, #12  // 
ext(,ABCD,12)=0ABC
+add v24.4S, v24.4S, \x  // 
+ABCD={X+A,X+B,X+C,X+D}
+add v24.4S, v24.4S, v25.4S  // 
{X+A,X+B+A,X+C+B,X+D+C}   (+0ABC)
+ext v25.16B, v26.16B, v25.16B, #12  // 
ext(,0ABC,12)=00AB
+add v24.4S, v24.4S, v25.4S  // 
{X+A,X+B+A,X+C+B+A,X+D+C+B}   (+00AB)
+ext v25.16B, v26.16B, v25.16B, #12  // 
ext(,00AB,12)=000A
+add v24.4S, v24.4S, v25.4S  // 
{X+A,X+B+A,X+C+B+A,X+D+C+B+A} (+000A)
+st1 {v24.4S}, [x0], #16 // 
write 4x32-bit final values
+.endm
+
+function ff_compute_safe_ssd_integral_image_neon, export=1
+moviv26.4S, #0  // 
used as zero for the "rotations" in acc_sum_store
+sub x3, x3, w6, UXTW 

[FFmpeg-devel] [PATCH 5/9] checkasm: add vf_nlmeans test for ssd_integral_image

2018-05-06 Thread Clément Bœsch
---
 tests/checkasm/Makefile |   1 +
 tests/checkasm/checkasm.c   |   3 +
 tests/checkasm/checkasm.h   |   1 +
 tests/checkasm/vf_nlmeans.c | 113 
 4 files changed, 118 insertions(+)
 create mode 100644 tests/checkasm/vf_nlmeans.c

diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
index 0233d2f989..9484acbbd7 100644
--- a/tests/checkasm/Makefile
+++ b/tests/checkasm/Makefile
@@ -35,6 +35,7 @@ AVFILTEROBJS-$(CONFIG_BLEND_FILTER) += vf_blend.o
 AVFILTEROBJS-$(CONFIG_COLORSPACE_FILTER) += vf_colorspace.o
 AVFILTEROBJS-$(CONFIG_HFLIP_FILTER)  += vf_hflip.o
 AVFILTEROBJS-$(CONFIG_THRESHOLD_FILTER)  += vf_threshold.o
+AVFILTEROBJS-$(CONFIG_NLMEANS_FILTER)+= vf_nlmeans.o
 
 CHECKASMOBJS-$(CONFIG_AVFILTER) += $(AVFILTEROBJS-yes)
 
diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
index ba1d1d0253..721a0912fb 100644
--- a/tests/checkasm/checkasm.c
+++ b/tests/checkasm/checkasm.c
@@ -159,6 +159,9 @@ static const struct {
 #if CONFIG_HFLIP_FILTER
 { "vf_hflip", checkasm_check_vf_hflip },
 #endif
+#if CONFIG_NLMEANS_FILTER
+{ "vf_nlmeans", checkasm_check_nlmeans },
+#endif
 #if CONFIG_THRESHOLD_FILTER
 { "vf_threshold", checkasm_check_vf_threshold },
 #endif
diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
index dcab74de06..c45cfb46f8 100644
--- a/tests/checkasm/checkasm.h
+++ b/tests/checkasm/checkasm.h
@@ -62,6 +62,7 @@ void checkasm_check_huffyuvdsp(void);
 void checkasm_check_jpeg2000dsp(void);
 void checkasm_check_llviddsp(void);
 void checkasm_check_llviddspenc(void);
+void checkasm_check_nlmeans(void);
 void checkasm_check_pixblockdsp(void);
 void checkasm_check_sbrdsp(void);
 void checkasm_check_synth_filter(void);
diff --git a/tests/checkasm/vf_nlmeans.c b/tests/checkasm/vf_nlmeans.c
new file mode 100644
index 00..5e2c934226
--- /dev/null
+++ b/tests/checkasm/vf_nlmeans.c
@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2018 Clément Bœsch 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "checkasm.h"
+#include "libavfilter/vf_nlmeans.h"
+#include "libavutil/avassert.h"
+
+#define randomize_buffer(buf, size) do {\
+int i;  \
+for (i = 0; i < size / 4; i++)  \
+((uint32_t *)buf)[i] = rnd();   \
+} while (0)
+
+void checkasm_check_nlmeans(void)
+{
+NLMeansDSPContext dsp = {0};
+
+const int w = 123;  // source width
+const int h = 45;   // source height
+const int p = 3;// patch half size
+const int r = 2;// research window half size
+
+ff_nlmeans_init();
+
+/* See the filter's code for the explanations on the variables */
+if (check_func(dsp.compute_safe_ssd_integral_image, "ssd_integral_image")) 
{
+int offx, offy;
+const int e = p + r;
+const int ii_w = w + e*2;
+const int ii_h = h + e*2;
+const int ii_lz_32 = FFALIGN(ii_w + 1, 4);
+uint32_t *ii_orig_ref = av_mallocz_array(ii_h + 1, ii_lz_32 * 
sizeof(*ii_orig_ref));
+uint32_t *ii_ref = ii_orig_ref + ii_lz_32 + 1;
+uint32_t *ii_orig_new = av_mallocz_array(ii_h + 1, ii_lz_32 * 
sizeof(*ii_orig_new));
+uint32_t *ii_new = ii_orig_new + ii_lz_32 + 1;
+const int src_lz = FFALIGN(w, 16);
+uint8_t *src = av_mallocz_array(h, src_lz);
+
+declare_func(void, uint32_t *dst, ptrdiff_t dst_linesize_32,
+ const uint8_t *s1, ptrdiff_t linesize1,
+ const uint8_t *s2, ptrdiff_t linesize2,
+ int w, int h);
+
+randomize_buffer(src, h * src_lz);
+
+for (offy = -r; offy <= r; offy++) {
+for (offx = -r; offx <= r; offx++) {
+if (offx || offy) {
+const int s1x = e;
+const int s1y = e;
+const int s2x = e + offx;
+const int s2y = e + offy;
+const int startx_safe = FFMAX(s1x, s2x);
+const int starty_safe = FFMAX(s1y, s2y);
+const int u_endx_safe = FFMIN(s1x + w, s2x + w);
+const in

[FFmpeg-devel] [PATCH 3/9] lavfi/nlmeans: use ptrdiff_t for linesizes

2018-05-06 Thread Clément Bœsch
Similarly to previous commit, this will help writing SIMD code by not
having manual zero-extension in SIMD code
---
 libavfilter/vf_nlmeans.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index 21f981a605..4119fa3e01 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -60,9 +60,9 @@ typedef struct NLMeansContext {
 uint32_t *ii_orig;  // integral image
 uint32_t *ii;   // integral image starting 
after the 0-line and 0-column
 int ii_w, ii_h; // width and height of the 
integral image
-int ii_lz_32;   // linesize in 32-bit units of 
the integral image
+ptrdiff_t ii_lz_32; // linesize in 32-bit units of 
the integral image
 struct weighted_avg *wa;// weighted average of every 
pixel
-int wa_linesize;// linesize for wa in struct 
size unit
+ptrdiff_t wa_linesize;  // linesize for wa in struct 
size unit
 double weight_lut[WEIGHT_LUT_SIZE]; // lookup table mapping 
(scaled) patch differences to their associated weights
 double pdiff_lut_scale; // scale factor for patch 
differences before looking into the LUT
 int max_meaningful_diff;// maximum difference 
considered (if the patch difference is too high we ignore the pixel)
@@ -150,9 +150,9 @@ static inline int get_integral_patch_value(const uint32_t 
*ii, int ii_lz_32, int
  * while for SIMD implementation it is likely more interesting to use the
  * two-loops algorithm variant.
  */
-static void compute_safe_ssd_integral_image_c(uint32_t *dst, int 
dst_linesize_32,
-  const uint8_t *s1, int linesize1,
-  const uint8_t *s2, int linesize2,
+static void compute_safe_ssd_integral_image_c(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
+  const uint8_t *s1, ptrdiff_t 
linesize1,
+  const uint8_t *s2, ptrdiff_t 
linesize2,
   int w, int h)
 {
 int x, y;
@@ -198,9 +198,9 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, int dst_linesize_32
  * @param w width to compute
  * @param h height to compute
  */
-static inline void compute_unsafe_ssd_integral_image(uint32_t *dst, int 
dst_linesize_32,
+static inline void compute_unsafe_ssd_integral_image(uint32_t *dst, ptrdiff_t 
dst_linesize_32,
  int startx, int starty,
- const uint8_t *src, int 
linesize,
+ const uint8_t *src, 
ptrdiff_t linesize,
  int offx, int offy, int 
r, int sw, int sh,
  int w, int h)
 {
@@ -240,8 +240,8 @@ static inline void 
compute_unsafe_ssd_integral_image(uint32_t *dst, int dst_line
  * @param h source height
  * @param e research padding edge
  */
-static void compute_ssd_integral_image(uint32_t *ii, int ii_linesize_32,
-   const uint8_t *src, int linesize, int 
offx, int offy,
+static void compute_ssd_integral_image(uint32_t *ii, ptrdiff_t ii_linesize_32,
+   const uint8_t *src, ptrdiff_t linesize, 
int offx, int offy,
int e, int w, int h)
 {
 // ii has a surrounding padding of thickness "e"
@@ -367,7 +367,7 @@ static int config_input(AVFilterLink *inlink)
 
 struct thread_data {
 const uint8_t *src;
-int src_linesize;
+ptrdiff_t src_linesize;
 int startx, starty;
 int endx, endy;
 const uint32_t *ii_start;
@@ -379,7 +379,7 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 int x, y;
 NLMeansContext *s = ctx->priv;
 const struct thread_data *td = arg;
-const int src_linesize = td->src_linesize;
+const ptrdiff_t src_linesize = td->src_linesize;
 const int process_h = td->endy - td->starty;
 const int slice_start = (process_h *  jobnr   ) / nb_jobs;
 const int slice_end   = (process_h * (jobnr+1)) / nb_jobs;
@@ -403,8 +403,8 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 }
 
 static int nlmeans_plane(AVFilterContext *ctx, int w, int h, int p, int r,
- uint8_t *dst, int dst_linesize,
- const uint8_t *src, int src_linesize)
+ uint8_t *dst, ptrdiff_t dst_linesize,
+ const uint8_t *src, ptrdiff_t 

[FFmpeg-devel] [PATCH 2/9] lavfi/nlmeans: add SIMD-friendly assumptions for compute_safe_ssd_integral_image

2018-05-06 Thread Clément Bœsch
SIMD code will not have to deal with padding itself. Overwriting in that
function may have been possible but involve large overreading of the
sources. Instead, we simply make sure the width to process is always a
multiple of 16. Additionally, there must be some actual area to process
so the SIMD code can have its boundary checks after processing the first
pixels.
---
 libavfilter/vf_nlmeans.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index d222d3913e..21f981a605 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -157,6 +157,9 @@ static void compute_safe_ssd_integral_image_c(uint32_t 
*dst, int dst_linesize_32
 {
 int x, y;
 
+/* SIMD-friendly assumptions allowed here */
+av_assert2(!(w & 0xf) && w >= 16 && h >= 1);
+
 for (y = 0; y < h; y++) {
 uint32_t acc = dst[-1] - dst[-dst_linesize_32 - 1];
 
@@ -257,9 +260,16 @@ static void compute_ssd_integral_image(uint32_t *ii, int 
ii_linesize_32,
 // to compare the 2 sources pixels
 const int startx_safe = FFMAX(s1x, s2x);
 const int starty_safe = FFMAX(s1y, s2y);
-const int endx_safe   = FFMIN(s1x + w, s2x + w);
+const int u_endx_safe = FFMIN(s1x + w, s2x + w); // unaligned
 const int endy_safe   = FFMIN(s1y + h, s2y + h);
 
+// deduce the safe area width and height
+const int safe_pw = (u_endx_safe - startx_safe) & ~0xf;
+const int safe_ph = endy_safe - starty_safe;
+
+// adjusted end x position of the safe area after width of the safe area 
gets aligned
+const int endx_safe = startx_safe + safe_pw;
+
 // top part where only one of s1 and s2 is still readable, or none at all
 compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
   0, 0,
@@ -273,24 +283,25 @@ static void compute_ssd_integral_image(uint32_t *ii, int 
ii_linesize_32,
   0, starty_safe,
   src, linesize,
   offx, offy, e, w, h,
-  startx_safe, endy_safe - starty_safe);
+  startx_safe, safe_ph);
 
 // main and safe part of the integral
 av_assert1(startx_safe - s1x >= 0); av_assert1(startx_safe - s1x < w);
 av_assert1(starty_safe - s1y >= 0); av_assert1(starty_safe - s1y < h);
 av_assert1(startx_safe - s2x >= 0); av_assert1(startx_safe - s2x < w);
 av_assert1(starty_safe - s2y >= 0); av_assert1(starty_safe - s2y < h);
-compute_safe_ssd_integral_image_c(ii + starty_safe*ii_linesize_32 + 
startx_safe, ii_linesize_32,
-  src + (starty_safe - s1y) * linesize + 
(startx_safe - s1x), linesize,
-  src + (starty_safe - s2y) * linesize + 
(startx_safe - s2x), linesize,
-  endx_safe - startx_safe, endy_safe - 
starty_safe);
+if (safe_pw && safe_ph)
+dsp->compute_safe_ssd_integral_image(ii + starty_safe*ii_linesize_32 + 
startx_safe, ii_linesize_32,
+ src + (starty_safe - s1y) * 
linesize + (startx_safe - s1x), linesize,
+ src + (starty_safe - s2y) * 
linesize + (startx_safe - s2x), linesize,
+ safe_pw, safe_ph);
 
 // right part of the integral
 compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
   endx_safe, starty_safe,
   src, linesize,
   offx, offy, e, w, h,
-  ii_w - endx_safe, endy_safe - 
starty_safe);
+  ii_w - endx_safe, safe_ph);
 
 // bottom part where only one of s1 and s2 is still readable, or none at 
all
 compute_unsafe_ssd_integral_image(ii, ii_linesize_32,
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/9] lavfi/nlmeans: random code shuffling to help compiler

2018-05-06 Thread Clément Bœsch
This makes nlmeans_slice() slightly faster at least on GCC 7.3.
---
 libavfilter/vf_nlmeans.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index e4952e187e..d222d3913e 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -368,7 +368,6 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 int x, y;
 NLMeansContext *s = ctx->priv;
 const struct thread_data *td = arg;
-const uint8_t *src = td->src;
 const int src_linesize = td->src_linesize;
 const int process_h = td->endy - td->starty;
 const int slice_start = (process_h *  jobnr   ) / nb_jobs;
@@ -377,14 +376,15 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 const int endy   = td->starty + slice_end;
 
 for (y = starty; y < endy; y++) {
+const uint8_t *src = td->src + y*src_linesize;
+struct weighted_avg *wa = s->wa + y*s->wa_linesize;
 for (x = td->startx; x < td->endx; x++) {
 const int patch_diff_sq = get_integral_patch_value(td->ii_start, 
s->ii_lz_32, x, y, td->p);
 if (patch_diff_sq < s->max_meaningful_diff) {
-struct weighted_avg *wa = >wa[y*s->wa_linesize + x];
 const int weight_lut_idx = patch_diff_sq * s->pdiff_lut_scale;
 const double weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
-wa->total_weight += weight;
-wa->sum += weight * src[y*src_linesize + x];
+wa[x].total_weight += weight;
+wa[x].sum += weight * src[x];
 }
 }
 }
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] Misc improvements in nlmeans filter

2018-05-06 Thread Clément Bœsch
The biggest change is the introduction of the dsp infrastructure such
that more SIMD can be added, in particular x86 version(s) of the
integral computation function. Only aarch64 was added so far (because
the ASM is easy), and I don't plan to work on other arch for now.

The filter is still pretty slow, so I'm open to suggestions.

Regards,


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] avfilter/vf_edgedetect: add more formats support to canny mode

2018-05-04 Thread Clément Bœsch
On Thu, May 03, 2018 at 03:44:44PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/vf_edgedetect.c | 48 
> ++---
>  1 file changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/libavfilter/vf_edgedetect.c b/libavfilter/vf_edgedetect.c
> index 6f86115d23..55c4cc3b5a 100644
> --- a/libavfilter/vf_edgedetect.c
> +++ b/libavfilter/vf_edgedetect.c
> @@ -52,6 +52,7 @@ struct plane_info {
>  uint8_t  *tmpbuf;
>  uint16_t *gradients;
>  char *directions;
> +int  width, height;
>  };
>  
>  typedef struct EdgeDetectContext {
> @@ -98,7 +99,7 @@ static int query_formats(AVFilterContext *ctx)
>  {
>  const EdgeDetectContext *edgedetect = ctx->priv;
>  static const enum AVPixelFormat wires_pix_fmts[] = {AV_PIX_FMT_GRAY8, 
> AV_PIX_FMT_NONE};
> -static const enum AVPixelFormat canny_pix_fmts[] = {AV_PIX_FMT_YUV444P, 
> AV_PIX_FMT_GBRP, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE};
> +static const enum AVPixelFormat canny_pix_fmts[] = {AV_PIX_FMT_YUV420P, 
> AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P, AV_PIX_FMT_GBRP, AV_PIX_FMT_GRAY8, 
> AV_PIX_FMT_NONE};
>  static const enum AVPixelFormat colormix_pix_fmts[] = {AV_PIX_FMT_GBRP, 
> AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE};
>  AVFilterFormats *fmts_list;
>  const enum AVPixelFormat *pix_fmts = NULL;
> @@ -123,14 +124,19 @@ static int config_props(AVFilterLink *inlink)
>  int p;
>  AVFilterContext *ctx = inlink->dst;
>  EdgeDetectContext *edgedetect = ctx->priv;
> +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
>  
>  edgedetect->nb_planes = inlink->format == AV_PIX_FMT_GRAY8 ? 1 : 3;
>  for (p = 0; p < edgedetect->nb_planes; p++) {
>  struct plane_info *plane = >planes[p];
> -
> -plane->tmpbuf = av_malloc(inlink->w * inlink->h);
> -plane->gradients  = av_calloc(inlink->w * inlink->h, 
> sizeof(*plane->gradients));
> -plane->directions = av_malloc(inlink->w * inlink->h);
> +int vsub = p ? desc->log2_chroma_h : 0;
> +int hsub = p ? desc->log2_chroma_w : 0;
> +
> +plane->width  = AV_CEIL_RSHIFT(inlink->w, hsub);
> +plane->height = AV_CEIL_RSHIFT(inlink->h, vsub);
> +plane->tmpbuf = av_malloc(plane->width * plane->height);
> +plane->gradients  = av_calloc(plane->width * plane->height, 
> sizeof(*plane->gradients));
> +plane->directions = av_malloc(plane->width * plane->height);
>  if (!plane->tmpbuf || !plane->gradients || !plane->directions)
>  return AVERROR(ENOMEM);
>  }
> @@ -338,42 +344,44 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *in)
>  uint8_t  *tmpbuf = plane->tmpbuf;
>  uint16_t *gradients  = plane->gradients;
>  int8_t   *directions = plane->directions;
> +const int width  = plane->width;
> +const int height = plane->height;
>  
>  if (!((1 << p) & edgedetect->filter_planes)) {
>  if (!direct)
>  av_image_copy_plane(out->data[p], out->linesize[p],
>  in->data[p], in->linesize[p],
> -inlink->w, inlink->h);
> +width, height);
>  continue;
>  }
>  
>  /* gaussian filter to reduce noise  */
> -gaussian_blur(ctx, inlink->w, inlink->h,
> -  tmpbuf,  inlink->w,
> +gaussian_blur(ctx, width, height,
> +  tmpbuf,  width,
>in->data[p], in->linesize[p]);
>  
>  /* compute the 16-bits gradients and directions for the next step */
> -sobel(inlink->w, inlink->h,
> -  gradients, inlink->w,
> -  directions,inlink->w,
> -  tmpbuf,inlink->w);
> +sobel(width, height,
> +  gradients, width,
> +  directions,width,
> +  tmpbuf,width);
>  
>  /* non_maximum_suppression() will actually keep & clip what's 
> necessary and
>   * ignore the rest, so we need a clean output buffer */
> -memset(tmpbuf, 0, inlink->w * inlink->h);
> -non_maximum_suppression(inlink->w, inlink->h,
> -tmpbuf,inlink->w,
> -directions,inlink->w,
> -gradients, inlink->w);
> +memset(tmpbuf, 0, width * height);
> +non_maximum_suppression(width, height,
> +tmpbuf,width,
> +directions,width,
> +gradients, width);
>  
>  /* keep high values, or low values surrounded by high values */
>  double_threshold(edgedetect->low_u8, edgedetect->high_u8,
> - inlink->w, inlink->h,
> + width, height,
> 

Re: [FFmpeg-devel] [PATCH 2/3] avfilter/vf_edgedetect: add planes option

2018-05-04 Thread Clément Bœsch
On Thu, May 03, 2018 at 03:44:43PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi|  4 +++-
>  libavfilter/vf_edgedetect.c | 25 +
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 29b5a5b15f..245326154c 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -8272,8 +8272,10 @@ Mix the colors to create a paint/cartoon effect.
>  @item canny
>  Apply Canny edge detector on all selected planes.
>  @end table
> -
>  Default value is @var{wires}.
> +
> +@item planes
> +Select planes for filtering. By default all available planes are filtered.
>  @end table
>  
>  @subsection Examples
> diff --git a/libavfilter/vf_edgedetect.c b/libavfilter/vf_edgedetect.c
> index 534a302d90..6f86115d23 100644
> --- a/libavfilter/vf_edgedetect.c
> +++ b/libavfilter/vf_edgedetect.c
> @@ -26,12 +26,21 @@
>   */
>  
>  #include "libavutil/avassert.h"
> +#include "libavutil/imgutils.h"
>  #include "libavutil/opt.h"
>  #include "avfilter.h"
>  #include "formats.h"
>  #include "internal.h"
>  #include "video.h"
>  
> +#define PLANE_R 0x4
> +#define PLANE_G 0x1
> +#define PLANE_B 0x2
> +#define PLANE_Y 0x1
> +#define PLANE_U 0x2
> +#define PLANE_V 0x4
> +#define PLANE_A 0x8
> +
>  enum FilterMode {
>  MODE_WIRES,
>  MODE_COLORMIX,
> @@ -48,6 +57,7 @@ struct plane_info {
>  typedef struct EdgeDetectContext {
>  const AVClass *class;
>  struct plane_info planes[3];
> +int filter_planes;
>  int nb_planes;
>  double   low, high;
>  uint8_t  low_u8, high_u8;
> @@ -63,6 +73,13 @@ static const AVOption edgedetect_options[] = {
>  { "wires","white/gray wires on black",  0, AV_OPT_TYPE_CONST, 
> {.i64=MODE_WIRES},INT_MIN, INT_MAX, FLAGS, "mode" },
>  { "colormix", "mix colors", 0, AV_OPT_TYPE_CONST, 
> {.i64=MODE_COLORMIX}, INT_MIN, INT_MAX, FLAGS, "mode" },
>  { "canny","detect edges on planes", 0, AV_OPT_TYPE_CONST, 
> {.i64=MODE_CANNY},INT_MIN, INT_MAX, FLAGS, "mode" },
> +{ "planes", "set planes to filter",  OFFSET(filter_planes), 
> AV_OPT_TYPE_FLAGS, {.i64=7}, 1, 0x7, FLAGS, "flags" },

> +{  "y", "filter luma plane",  0, AV_OPT_TYPE_CONST, {.i64=PLANE_Y}, 
> 0, 0, FLAGS, "flags"},
> +{  "u", "filter u plane", 0, AV_OPT_TYPE_CONST, {.i64=PLANE_U}, 
> 0, 0, FLAGS, "flags"},
> +{  "v", "filter v plane", 0, AV_OPT_TYPE_CONST, {.i64=PLANE_V}, 
> 0, 0, FLAGS, "flags"},
> +{  "r", "filter red plane",   0, AV_OPT_TYPE_CONST, {.i64=PLANE_R}, 
> 0, 0, FLAGS, "flags"},
> +{  "g", "filter green plane", 0, AV_OPT_TYPE_CONST, {.i64=PLANE_G}, 
> 0, 0, FLAGS, "flags"},
> +{  "b", "filter blue plane",  0, AV_OPT_TYPE_CONST, {.i64=PLANE_B}, 
> 0, 0, FLAGS, "flags"},

please keep the style consistent with above (also beware of the trailing
space before "}")

>  { NULL }
>  };
>  
> @@ -322,6 +339,14 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *in)
>  uint16_t *gradients  = plane->gradients;
>  int8_t   *directions = plane->directions;
>  
> +if (!((1 << p) & edgedetect->filter_planes)) {
> +if (!direct)
> +av_image_copy_plane(out->data[p], out->linesize[p],
> +in->data[p], in->linesize[p],
> +inlink->w, inlink->h);
> +continue;
> +}
> +

Should be fine. Though, I'd say the green (0) is unexpected for chroma
planes; I'd expect gray (128) instead. Not blocking but could be changed
in the future.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/3] avfilter/vf_edgedetect: add canny mode

2018-05-04 Thread Clément Bœsch
On Thu, May 03, 2018 at 03:44:42PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  doc/filters.texi| 3 +++
>  libavfilter/vf_edgedetect.c | 5 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 218f30ef5f..29b5a5b15f 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -8268,6 +8268,9 @@ Draw white/gray wires on black background.
>  
>  @item colormix
>  Mix the colors to create a paint/cartoon effect.
> +
> +@item canny
> +Apply Canny edge detector on all selected planes.
>  @end table
>  
>  Default value is @var{wires}.
> diff --git a/libavfilter/vf_edgedetect.c b/libavfilter/vf_edgedetect.c
> index 173f9fe161..534a302d90 100644
> --- a/libavfilter/vf_edgedetect.c
> +++ b/libavfilter/vf_edgedetect.c
> @@ -35,6 +35,7 @@
>  enum FilterMode {
>  MODE_WIRES,
>  MODE_COLORMIX,
> +MODE_CANNY,
>  NB_MODE
>  };
>  
> @@ -61,6 +62,7 @@ static const AVOption edgedetect_options[] = {
>  { "mode", "set mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=MODE_WIRES}, 
> 0, NB_MODE-1, FLAGS, "mode" },
>  { "wires","white/gray wires on black",  0, AV_OPT_TYPE_CONST, 
> {.i64=MODE_WIRES},INT_MIN, INT_MAX, FLAGS, "mode" },
>  { "colormix", "mix colors", 0, AV_OPT_TYPE_CONST, 
> {.i64=MODE_COLORMIX}, INT_MIN, INT_MAX, FLAGS, "mode" },
> +{ "canny","detect edges on planes", 0, AV_OPT_TYPE_CONST, 
> {.i64=MODE_CANNY},INT_MIN, INT_MAX, FLAGS, "mode" },
>  { NULL }
>  };
>  
> @@ -79,6 +81,7 @@ static int query_formats(AVFilterContext *ctx)
>  {
>  const EdgeDetectContext *edgedetect = ctx->priv;
>  static const enum AVPixelFormat wires_pix_fmts[] = {AV_PIX_FMT_GRAY8, 
> AV_PIX_FMT_NONE};
> +static const enum AVPixelFormat canny_pix_fmts[] = {AV_PIX_FMT_YUV444P, 
> AV_PIX_FMT_GBRP, AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE};
>  static const enum AVPixelFormat colormix_pix_fmts[] = {AV_PIX_FMT_GBRP, 
> AV_PIX_FMT_GRAY8, AV_PIX_FMT_NONE};
>  AVFilterFormats *fmts_list;
>  const enum AVPixelFormat *pix_fmts = NULL;
> @@ -87,6 +90,8 @@ static int query_formats(AVFilterContext *ctx)
>  pix_fmts = wires_pix_fmts;
>  } else if (edgedetect->mode == MODE_COLORMIX) {
>  pix_fmts = colormix_pix_fmts;
> +} else if (edgedetect->mode == MODE_CANNY) {
> +pix_fmts = canny_pix_fmts;
>  } else {
>  av_assert0(0);
>  }

Sure, why not

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter/vf_lut3d: add planar rgb support

2018-05-04 Thread Clément Bœsch
On Thu, May 03, 2018 at 08:08:03PM +0200, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/vf_lut3d.c | 114 
> +++--
>  1 file changed, 111 insertions(+), 3 deletions(-)
> 
> diff --git a/libavfilter/vf_lut3d.c b/libavfilter/vf_lut3d.c
> index c9b72249aa..2f8fc723b7 100644
> --- a/libavfilter/vf_lut3d.c
> +++ b/libavfilter/vf_lut3d.c
> @@ -198,6 +198,83 @@ static inline struct rgbvec interp_tetrahedral(const 
> LUT3DContext *lut3d,
>  return c;
>  }
>  

> +#define DEFINE_INTERP_FUNC_PLANAR(name, nbits, depth)
>\
> +static int interp_##nbits##_##name##_p##depth(AVFilterContext *ctx, void 
> *arg, int jobnr, int nb_jobs) \

align style

[...]
>  #define SET_FUNC(name) do { \
> -if (is16bit) lut3d->interp = interp_16_##name;  \
> -else lut3d->interp = interp_8_##name;   \
> +if (planar) {   \
> +switch (depth) {\
> +case  8: lut3d->interp = interp_8_##name##_p8;   break; \
> +case  9: lut3d->interp = interp_16_##name##_p9;  break; \
> +case 10: lut3d->interp = interp_16_##name##_p10; break; \
> +case 12: lut3d->interp = interp_16_##name##_p12; break; \
> +case 14: lut3d->interp = interp_16_##name##_p14; break; \
> +case 16: lut3d->interp = interp_16_##name##_p16; break; \
> +}   \

> +} else if (is16bit) { lut3d->interp = interp_16_##name; \
> +} else {   lut3d->interp = interp_8_##name; }   \

align style

[...]

aside from these details, LGTM, thanks

feel free to adjust the av_clip in DEFINE_INTERP_FUNC in another commit if
it works

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/hls: don't propagate deprecated "user-agent" AVOption

2018-04-29 Thread Clément Bœsch
On Sat, Apr 28, 2018 at 08:37:06PM +0200, wm4 wrote:
> This code will print a warning if any user agent is set - even if the
> API user used the proper non-deprecated "user_agent" option.
> 
> This change should not even break anything, because even if the user
> sets the deprecated "user-agent" option, http.c copies it to the
> "user_agent" option anyway.
> ---
>  libavformat/hls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index ffec124818..4ee4be769d 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1593,7 +1593,7 @@ static int save_avio_options(AVFormatContext *s)
>  {
>  HLSContext *c = s->priv_data;
>  static const char * const opts[] = {
> -"headers", "http_proxy", "user_agent", "user-agent", "cookies", 
> "referer", "rw_timeout", NULL };
> +"headers", "http_proxy", "user_agent", "cookies", "referer", 
> "rw_timeout", NULL };
>  const char * const * opt = opts;
>  uint8_t *buf;
>  int ret = 0;

Should be fine, thanks.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] lavu/opt: add AV_OPT_FLAG_DEPRECATED

2018-04-26 Thread Clément Bœsch
On Sun, Apr 22, 2018 at 05:00:45PM +0200, wm4 wrote:
> On Sun, 22 Apr 2018 16:38:08 +0200
> Clément Bœsch <u...@pkh.me> wrote:
> 
> > ---
> > TODO: APIChanges + lavu minor bump (not done yet because it will
> > conflict with the threadmessage patch)
> > ---
> >  libavutil/opt.c | 6 ++
> >  libavutil/opt.h | 1 +
> >  2 files changed, 7 insertions(+)
> > 
> > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > index 3b0aab4ee8..99282605f5 100644
> > --- a/libavutil/opt.c
> > +++ b/libavutil/opt.c
> > @@ -463,6 +463,9 @@ int av_opt_set(void *obj, const char *name, const char 
> > *val, int search_flags)
> >  if (o->flags & AV_OPT_FLAG_READONLY)
> >  return AVERROR(EINVAL);
> >  
> > +if (o->flags & AV_OPT_FLAG_DEPRECATED)
> > +av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: 
> > %s\n", name, o->help);
> > +
> >  dst = ((uint8_t *)target_obj) + o->offset;
> >  switch (o->type) {
> >  case AV_OPT_TYPE_BOOL:
> > @@ -759,6 +762,9 @@ int av_opt_get(void *obj, const char *name, int 
> > search_flags, uint8_t **out_val)
> >  if (!o || !target_obj || (o->offset<=0 && o->type != 
> > AV_OPT_TYPE_CONST))
> >  return AVERROR_OPTION_NOT_FOUND;
> >  
> > +if (o->flags & AV_OPT_FLAG_DEPRECATED)
> > +av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: 
> > %s\n", name, o->help);
> > +
> >  dst = (uint8_t *)target_obj + o->offset;
> >  
> >  buf[0] = 0;
> > diff --git a/libavutil/opt.h b/libavutil/opt.h
> > index 07da68ea23..1352741fb6 100644
> > --- a/libavutil/opt.h
> > +++ b/libavutil/opt.h
> > @@ -289,6 +289,7 @@ typedef struct AVOption {
> >  #define AV_OPT_FLAG_READONLY128
> >  #define AV_OPT_FLAG_BSF_PARAM   (1<<8) ///< a generic parameter which 
> > can be set by the user for bit stream filtering
> >  #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which 
> > can be set by the user for filtering
> > +#define AV_OPT_FLAG_DEPRECATED  (1<<17)
> >  //FIXME think about enc-audio, ... style flags
> >  
> >  /**
> 
> Seems like a good idea.

documented & applied.

thx

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavu/threadmessage: add av_thread_message_queue_nelem()

2018-04-26 Thread Clément Bœsch
On Mon, Apr 23, 2018 at 02:13:41AM +0200, Michael Niedermayer wrote:
[...]
> doesnt document the AVERROR case
> 

added

> should be ok otherwise, no more comments from me
> 

thx, applied

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavu/threadmessage: add av_thread_message_queue_nelem()

2018-04-22 Thread Clément Bœsch
On Sun, Apr 22, 2018 at 12:33:14PM +0200, Marton Balint wrote:
> 
> 
> On Sun, 22 Apr 2018, Clément Bœsch wrote:
> 
> > On Sun, Apr 22, 2018 at 02:51:16AM +0100, Rostislav Pehlivanov wrote:
> > [...]
> > > I think av_thread_message_queue_elems would be a better name, had to think
> > > for a good period of time what "nelem" meant.
> > 
> > I'm afraid of "queue_elems" implying "queuing elements" so I went for
> > the more explicit av_thread_message_queue_nb_elems() instead.
> 
> I generally prefer nb_items instead of nb_elems. Use whichever you like.
> 

No personal opinion on this but the "element" semantic is already in use
in the API so I'd rather follow that.

-- 
Clément B.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/2] lavf/http: use AV_OPT_FLAG_DEPRECATED for user-agent option

2018-04-22 Thread Clément Bœsch
---
There are probably a bunch of other options and I don't plan to look for
them, I just needed at least one example of use.
---
 libavformat/http.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index aa6348f28b..3a35bc7eac 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -141,7 +141,7 @@ static const AVOption options[] = {
 { "user_agent", "override User-Agent header", OFFSET(user_agent), 
AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
 { "referer", "override referer header", OFFSET(referer), 
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
 #if FF_API_HTTP_USER_AGENT
-{ "user-agent", "override User-Agent header", 
OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT 
}, 0, 0, D },
+{ "user-agent", "use the \"user_agent\" option instead", 
OFFSET(user_agent_deprecated), AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT 
}, 0, 0, D|AV_OPT_FLAG_DEPRECATED },
 #endif
 { "multiple_requests", "use persistent connections", 
OFFSET(multiple_requests), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D | E },
 { "post_data", "set custom HTTP post data", OFFSET(post_data), 
AV_OPT_TYPE_BINARY, .flags = D | E },
@@ -1193,7 +1193,6 @@ static int http_connect(URLContext *h, const char *path, 
const char *local_path,
 
 #if FF_API_HTTP_USER_AGENT
 if (strcmp(s->user_agent_deprecated, DEFAULT_USER_AGENT)) {
-av_log(s, AV_LOG_WARNING, "the user-agent option is deprecated, please 
use user_agent option\n");
 s->user_agent = av_strdup(s->user_agent_deprecated);
 }
 #endif
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/2] lavu/opt: add AV_OPT_FLAG_DEPRECATED

2018-04-22 Thread Clément Bœsch
---
TODO: APIChanges + lavu minor bump (not done yet because it will
conflict with the threadmessage patch)
---
 libavutil/opt.c | 6 ++
 libavutil/opt.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/libavutil/opt.c b/libavutil/opt.c
index 3b0aab4ee8..99282605f5 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -463,6 +463,9 @@ int av_opt_set(void *obj, const char *name, const char 
*val, int search_flags)
 if (o->flags & AV_OPT_FLAG_READONLY)
 return AVERROR(EINVAL);
 
+if (o->flags & AV_OPT_FLAG_DEPRECATED)
+av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n", 
name, o->help);
+
 dst = ((uint8_t *)target_obj) + o->offset;
 switch (o->type) {
 case AV_OPT_TYPE_BOOL:
@@ -759,6 +762,9 @@ int av_opt_get(void *obj, const char *name, int 
search_flags, uint8_t **out_val)
 if (!o || !target_obj || (o->offset<=0 && o->type != AV_OPT_TYPE_CONST))
 return AVERROR_OPTION_NOT_FOUND;
 
+if (o->flags & AV_OPT_FLAG_DEPRECATED)
+av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n", 
name, o->help);
+
 dst = (uint8_t *)target_obj + o->offset;
 
 buf[0] = 0;
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07da68ea23..1352741fb6 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -289,6 +289,7 @@ typedef struct AVOption {
 #define AV_OPT_FLAG_READONLY128
 #define AV_OPT_FLAG_BSF_PARAM   (1<<8) ///< a generic parameter which can 
be set by the user for bit stream filtering
 #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which can 
be set by the user for filtering
+#define AV_OPT_FLAG_DEPRECATED  (1<<17)
 //FIXME think about enc-audio, ... style flags
 
 /**
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavu/threadmessage: add av_thread_message_queue_nelem()

2018-04-22 Thread Clément Bœsch
On Sun, Apr 22, 2018 at 02:51:16AM +0100, Rostislav Pehlivanov wrote:
[...]
> I think av_thread_message_queue_elems would be a better name, had to think
> for a good period of time what "nelem" meant.

I'm afraid of "queue_elems" implying "queuing elements" so I went for
the more explicit av_thread_message_queue_nb_elems() instead.

Also fixed the problem raised by Michael.

-- 
Clément B.
From 6647c716a31481b2d1348d03616557a63a6a0cef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= 
Date: Sat, 21 Apr 2018 21:42:19 +0200
Subject: [PATCH] lavu/threadmessage: add av_thread_message_queue_nb_elems()

---
 doc/APIchanges |  3 +++
 libavutil/threadmessage.c  | 13 +
 libavutil/threadmessage.h  |  5 +
 libavutil/version.h|  2 +-
 tests/api/api-threadmessage-test.c |  4 +++-
 5 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 4f6ac2a031..8d305d5867 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2018-04-xx - xx - lavu 56.16.100 - threadmessage.h
+  Add av_thread_message_queue_nb_elems().
+
  8< - FFmpeg 4.0 was cut here  8< -
 
 2018-04-03 - d6fc031caf - lavu 56.13.100 - pixdesc.h
diff --git a/libavutil/threadmessage.c b/libavutil/threadmessage.c
index 872e9392b1..764b7fb813 100644
--- a/libavutil/threadmessage.c
+++ b/libavutil/threadmessage.c
@@ -102,6 +102,19 @@ void av_thread_message_queue_free(AVThreadMessageQueue 
**mq)
 #endif
 }
 
+int av_thread_message_queue_nb_elems(AVThreadMessageQueue *mq)
+{
+#if HAVE_THREADS
+int ret;
+pthread_mutex_lock(>lock);
+ret = av_fifo_size(mq->fifo);
+pthread_mutex_unlock(>lock);
+return ret / mq->elsize;
+#else
+return AVERROR(ENOSYS);
+#endif
+}
+
 #if HAVE_THREADS
 
 static int av_thread_message_queue_send_locked(AVThreadMessageQueue *mq,
diff --git a/libavutil/threadmessage.h b/libavutil/threadmessage.h
index 8480a0a3db..7ebf03a8ac 100644
--- a/libavutil/threadmessage.h
+++ b/libavutil/threadmessage.h
@@ -95,6 +95,11 @@ void 
av_thread_message_queue_set_err_recv(AVThreadMessageQueue *mq,
 void av_thread_message_queue_set_free_func(AVThreadMessageQueue *mq,
void (*free_func)(void *msg));
 
+/**
+ * Return the current number of messages in the queue.
+ */
+int av_thread_message_queue_nb_elems(AVThreadMessageQueue *mq);
+
 /**
  * Flush the message queue
  *
diff --git a/libavutil/version.h b/libavutil/version.h
index 387421775f..23567000a3 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  15
+#define LIBAVUTIL_VERSION_MINOR  16
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
diff --git a/tests/api/api-threadmessage-test.c 
b/tests/api/api-threadmessage-test.c
index 05a8062b8c..3c693a70d1 100644
--- a/tests/api/api-threadmessage-test.c
+++ b/tests/api/api-threadmessage-test.c
@@ -130,7 +130,9 @@ static void *receiver_thread(void *arg)
 
 for (i = 0; i < rd->workload; i++) {
 if (rand() % rd->workload < rd->workload / 10) {
-av_log(NULL, AV_LOG_INFO, "receiver #%d: flushing the queue\n", 
rd->id);
+av_log(NULL, AV_LOG_INFO, "receiver #%d: flushing the queue, "
+   "discarding %d message(s)\n", rd->id,
+   av_thread_message_queue_nb_elems(rd->queue));
 av_thread_message_flush(rd->queue);
 } else {
 struct message msg;
-- 
2.17.0



signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] lavu/threadmessage: add av_thread_message_queue_nelem()

2018-04-21 Thread Clément Bœsch
---
Been away from FFmpeg for way too long. Hope this patch get me back on
track. Feel free to nitpick on the name.
---
 doc/APIchanges | 3 +++
 libavutil/threadmessage.c  | 9 +
 libavutil/threadmessage.h  | 5 +
 libavutil/version.h| 2 +-
 tests/api/api-threadmessage-test.c | 4 +++-
 5 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 4f6ac2a031..393491c8e9 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
 
 API changes, most recent first:
 
+2018-04-xx - xx - lavu 56.16.100 - threadmessage.h
+  Add av_thread_message_queue_nelem().
+
  8< - FFmpeg 4.0 was cut here  8< -
 
 2018-04-03 - d6fc031caf - lavu 56.13.100 - pixdesc.h
diff --git a/libavutil/threadmessage.c b/libavutil/threadmessage.c
index 872e9392b1..fed398926a 100644
--- a/libavutil/threadmessage.c
+++ b/libavutil/threadmessage.c
@@ -102,6 +102,15 @@ void av_thread_message_queue_free(AVThreadMessageQueue 
**mq)
 #endif
 }
 
+int av_thread_message_queue_nelem(AVThreadMessageQueue *mq)
+{
+int ret;
+pthread_mutex_lock(>lock);
+ret = av_fifo_size(mq->fifo);
+pthread_mutex_unlock(>lock);
+return ret / mq->elsize;
+}
+
 #if HAVE_THREADS
 
 static int av_thread_message_queue_send_locked(AVThreadMessageQueue *mq,
diff --git a/libavutil/threadmessage.h b/libavutil/threadmessage.h
index 8480a0a3db..e41d95ccf7 100644
--- a/libavutil/threadmessage.h
+++ b/libavutil/threadmessage.h
@@ -95,6 +95,11 @@ void 
av_thread_message_queue_set_err_recv(AVThreadMessageQueue *mq,
 void av_thread_message_queue_set_free_func(AVThreadMessageQueue *mq,
void (*free_func)(void *msg));
 
+/**
+ * Return the current number of messages in the queue.
+ */
+int av_thread_message_queue_nelem(AVThreadMessageQueue *mq);
+
 /**
  * Flush the message queue
  *
diff --git a/libavutil/version.h b/libavutil/version.h
index 387421775f..23567000a3 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  15
+#define LIBAVUTIL_VERSION_MINOR  16
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
diff --git a/tests/api/api-threadmessage-test.c 
b/tests/api/api-threadmessage-test.c
index 05a8062b8c..494808f176 100644
--- a/tests/api/api-threadmessage-test.c
+++ b/tests/api/api-threadmessage-test.c
@@ -130,7 +130,9 @@ static void *receiver_thread(void *arg)
 
 for (i = 0; i < rd->workload; i++) {
 if (rand() % rd->workload < rd->workload / 10) {
-av_log(NULL, AV_LOG_INFO, "receiver #%d: flushing the queue\n", 
rd->id);
+av_log(NULL, AV_LOG_INFO, "receiver #%d: flushing the queue, "
+   "discarding %d message(s)\n", rd->id,
+   av_thread_message_queue_nelem(rd->queue));
 av_thread_message_flush(rd->queue);
 } else {
 struct message msg;
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] vf_paletteuse: Add error checking to apply_palette

2018-01-02 Thread Clément Bœsch
On Tue, Jan 02, 2018 at 10:02:25PM +, Derek Buitenhuis wrote:
> On 1/2/2018 9:52 PM, Clément Bœsch wrote:
> > not exactly sure why you haven't just checked if `out` wasn't NULL, but it
> > should be fine that way too if you prefer it.
> > 
> > I guess that's only to provide a finer grain error handling? It would make
> > sense if ff_get_video_buffer was returning a meaningful error, but so far
> > you're just assuming EINVAL when it could be ENOMEM, so I don't really get
> > it.
> 
> s->set_frame can return ENOMEM, which is why I made it finer grained.
> 
> I'm not really sure what to return for ff_get_video_buffer failure, since
> it wasn't designed with any error mechanism for some reason.
> 

I don't think you'll be much off by always assuming ENOMEM  here when
getting a NULL out frame, I think that's the common idiom when a function
supposed to return allocated stuff returns NULL.

But that's not very important, feel free to push as is if you prefer that
way.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] vf_paletteuse: Add error checking to apply_palette

2018-01-02 Thread Clément Bœsch
On Mon, Jan 01, 2018 at 11:28:36AM -0500, Derek Buitenhuis wrote:
> This fixes a segfault caused by passing NULL to ff_filter_frame
> when an error occurs.
> 
> Signed-off-by: Derek Buitenhuis 
> ---
>  libavfilter/vf_paletteuse.c | 25 -
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
> index 1980907..ede2e2e 100644
> --- a/libavfilter/vf_paletteuse.c
> +++ b/libavfilter/vf_paletteuse.c
> @@ -894,9 +894,9 @@ static void set_processing_window(enum diff_mode 
> diff_mode,
>  *hp = height;
>  }
>  
> -static AVFrame *apply_palette(AVFilterLink *inlink, AVFrame *in)
> +static int apply_palette(AVFilterLink *inlink, AVFrame *in, AVFrame **outf)
>  {
> -int x, y, w, h;
> +int x, y, w, h, ret;
>  AVFilterContext *ctx = inlink->dst;
>  PaletteUseContext *s = ctx->priv;
>  AVFilterLink *outlink = inlink->dst->outputs[0];
> @@ -904,7 +904,8 @@ static AVFrame *apply_palette(AVFilterLink *inlink, 
> AVFrame *in)
>  AVFrame *out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>  if (!out) {
>  av_frame_free();
> -return NULL;
> +*outf = NULL;
> +return AVERROR(EINVAL);
>  }
>  av_frame_copy_props(out, in);
>  
> @@ -918,21 +919,25 @@ static AVFrame *apply_palette(AVFilterLink *inlink, 
> AVFrame *in)
>  av_frame_make_writable(s->last_in) < 0) {
>  av_frame_free();
>  av_frame_free();
> -return NULL;
> +*outf = NULL;
> +return AVERROR(EINVAL);
>  }
>  
>  ff_dlog(ctx, "%dx%d rect: (%d;%d) -> (%d,%d) [area:%dx%d]\n",
>  w, h, x, y, x+w, y+h, in->width, in->height);
>  
> -if (s->set_frame(s, out, in, x, y, w, h) < 0) {
> +ret = s->set_frame(s, out, in, x, y, w, h);
> +if (ret < 0) {
>  av_frame_free();
> -return NULL;
> +*outf = NULL;
> +return ret;
>  }
>  memcpy(out->data[1], s->palette, AVPALETTE_SIZE);
>  if (s->calc_mean_err)
>  debug_mean_error(s, in, out, inlink->frame_count_out);
>  av_frame_free();
> -return out;
> +*outf = out;
> +return 0;
>  }
>  
>  static int config_output(AVFilterLink *outlink)
> @@ -1011,7 +1016,7 @@ static int load_apply_palette(FFFrameSync *fs)
>  AVFilterContext *ctx = fs->parent;
>  AVFilterLink *inlink = ctx->inputs[0];
>  PaletteUseContext *s = ctx->priv;
> -AVFrame *master, *second, *out;
> +AVFrame *master, *second, *out = NULL;
>  int ret;
>  
>  // writable for error diffusal dithering
> @@ -1025,7 +1030,9 @@ static int load_apply_palette(FFFrameSync *fs)
>  if (!s->palette_loaded) {
>  load_palette(s, second);
>  }
> -out = apply_palette(inlink, master);
> +ret = apply_palette(inlink, master, );
> +if (ret < 0)
> +goto error;
>  return ff_filter_frame(ctx->outputs[0], out);
>  

not exactly sure why you haven't just checked if `out` wasn't NULL, but it
should be fine that way too if you prefer it.

I guess that's only to provide a finer grain error handling? It would make
sense if ff_get_video_buffer was returning a meaningful error, but so far
you're just assuming EINVAL when it could be ENOMEM, so I don't really get
it.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] vf_paletteuse: Don't free the second frame from ff_framesync_dualinput_get_writable on error

2018-01-02 Thread Clément Bœsch
On Mon, Jan 01, 2018 at 11:28:37AM -0500, Derek Buitenhuis wrote:
> This fixes a double free in he error case.
> 
> Signed-off-by: Derek Buitenhuis 
> ---
> This does fix the double free, but I am unsure if it is the correct free
> to removed to fix it. Comments welcome.
> ---
>  libavfilter/vf_paletteuse.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
> index ede2e2e..c2d0c6b 100644
> --- a/libavfilter/vf_paletteuse.c
> +++ b/libavfilter/vf_paletteuse.c
> @@ -1037,7 +1037,6 @@ static int load_apply_palette(FFFrameSync *fs)
>  
>  error:
>  av_frame_free();
> -av_frame_free();
>  return ret;
>  }

That's some weird ownership semantic for the error-path, but Nicolas knows
better this API so I'll trust him on this one.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavr: deprecate the entire library

2017-12-25 Thread Clément Bœsch
On Mon, Dec 25, 2017 at 11:57:31AM +, Rostislav Pehlivanov wrote:
[...]
> > What you call "dead project" did basically all of the hardware
> > transcoding improvements that were merged into ffmpeg. How very
> > insincere of you.
> >
> 
> Didn't mean dead as an offense, just as an honest fact. Have you seen their
> ML these past months? 23 times less mails than here just this month. 13
> times for the previous month. Similar commits wise.
> 

Not actively developed doesn't mean unmaintained.

[...]

I suggested an alternative wording in another thread, something along the
lines "libavresample is not maintained *in FFmpeg* and duplicates
functionalities with libswresample". You could follow with "lavr has been
copied in our source tree years ago and left untouched for compatibility
with the Libav project but maintaining both libswresample and
libavresample is not good for the durability and consistency of the FFmpeg
project. We decided to keep our implementation since it's overall
considered better from our point of view."

No need to keep up the bad vibes. It served its purpose and dropping
libavresample will cause enough problems to our users, we don't need to be
dicks in our political statements.

So again, I support the patch, but not its announce form. I'd advise to be
wise with how we take political stands, being aggressive will not deserve
us.

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] DVD subtitles hacking

2017-12-19 Thread Clément Bœsch
On Fri, Dec 15, 2017 at 09:16:02AM -0600, Matt Zagrabelny wrote:
> On Thu, Dec 14, 2017 at 8:06 PM, Carl Eugen Hoyos 
> wrote:
> 
> >
> > You could implement a hack (similar to what the CC code does)
> > that allows processing the subs but it is likely that the patch
> > would be rejected because actual subtitle support for libavfilter
> > is needed.
> >
> 
> What would it take to add subtitle support to libavfilter?
> 

http://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/202152.html

Something like that

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] configure: Allow users to disable all hwaccel libraries

2017-11-30 Thread Clément Bœsch
On Thu, Nov 30, 2017 at 03:58:21PM +0530, Gyan Doshi wrote:
> I added --disable-hwaccels during configure and noticed that all
> autodetected hwaccel libs were still being linked - 6 in my case, which
> makes for a more bloated binary than warranted.
> 
> Turns out the option only disables native components which make use of the
> hwaccel libs. The reporter of ticket #3906 had a similar concern.
> 
> Added option to disable all autodetected hwaccel libs.
> 

Is --disable-autodetect broken/not complete for you?

The autodetect handling in the configure is rather sensible as it needs to
work properly in combination with other options (see for instance around
lines 3720-3740), which makes me believe that the patch you're proposing
may not be enough.

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-devel 1/1] avcodec/samidec: check av_strdup() return value

2017-11-29 Thread Clément Bœsch
On Mon, Nov 27, 2017 at 02:56:32PM +0800, Pan Bian wrote:
> In function sami_paragraph_to_ass(), the return value of av_strdup() is
> not checked. To avoid potential NULL dereference, the return value
> should be checked against NULL.
> 
> Signed-off-by: Pan Bian 
> ---
> V2: fix patcheck warnings
> ---
>  libavcodec/samidec.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/samidec.c b/libavcodec/samidec.c
> index 2620424..6a59806 100644
> --- a/libavcodec/samidec.c
> +++ b/libavcodec/samidec.c
> @@ -48,6 +48,9 @@ static int sami_paragraph_to_ass(AVCodecContext *avctx, 
> const char *src)
>  AVBPrint *dst_content = >encoded_content;
>  AVBPrint *dst_source = >encoded_source;
>  
> +if (!dupsrc)
> +return AVERROR(ENOMEM);
> +
>  av_bprint_clear(>encoded_content);
>  av_bprint_clear(>content);
>  av_bprint_clear(>encoded_source);

Patch applied, thanks

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Accurately describing ffmpeg-cvslog list [was: Re: [PATCH] Refactor Developer Docs, update dev list section (v2)]

2017-11-28 Thread Clément Bœsch
On Mon, Nov 27, 2017 at 11:19:01PM +, Rostislav Pehlivanov wrote:
> On 27 November 2017 at 23:05, Ronald S. Bultje  wrote:
> 
> > Hi,
> >
> > On Mon, Nov 27, 2017 at 6:03 PM, Carl Eugen Hoyos 
> > wrote:
> >
> > > 2017-11-26 22:57 GMT+01:00 Jim DeLaHunt :
> > > > On 2017-11-26 03:42, Carl Eugen Hoyos wrote:
> > > >
> > > >> 2017-11-26 9:31 GMT+01:00 Jim DeLaHunt :
> > > >> [...]
> > > >>>
> > > >>> +
> > > >>>   @subheading Subscribe to the ffmpeg-cvslog mailing list.
> > > >>> -It is important to do this as the diffs of all commits are sent
> > there
> > > >>> and
> > > >>> -reviewed by all the other developers. Bugs and possible improvements
> > > or
> > > >>> -general questions regarding commits are discussed there. We expect
> > you
> > > >>> to
> > > >>> -react if problems with your code are uncovered.
> > > >>> +Diffs of all commits are sent to the
> > > >>> +@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-cvslog,
> > > >>> ffmpeg-cvslog}
> > > >>> +mailing list. Some developers read this list to review all code base
> > > >>> changes
> > > >>> +from all sources. Subscribing to this list is not mandatory, if
> > > >>> +all you want to do is submit a patch here and there.
> > > >>
> > > >> I am (still) against this change.
> > > >
> > > >
> > > > OK, what specifically are you against?  More important, what are you in
> > > > favour of?
> > >
> > > I have no issue with the current text.
> > >
> > > If you believe that it is unclear that there is a difference between an
> > > occasional contributor (who most likely would not read -devel nor
> > > -cvslog) and a committer (who is supposed to read -cvslog), then
> > > maybe a patch is useful.
> >
> >
> > FYI I'm a committer and I do not read -cvslog.
> >
> > Ronald
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> 
> Yeah, never touched cvslog either.
> If someone has comments to a commit which was pushed and doesn't use IRC
> then they should send an email to this ML. Or better yet use IRC.

I'm subscribed to cvslog, and sometimes use it to comment on a commit.
But when I do, I CC ffmpeg-devel anyway. I don't think cvslog subscription
should be mandatory at all, even for core developers. It's fine to
acknowledge its existence in the developers documentation though.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] avformat: deprecate AVFMT_FLAG_AUTO_BSFr

2017-11-26 Thread Clément Bœsch
On Sun, Nov 26, 2017 at 05:51:04PM -0300, James Almer wrote:
> The bitstream filters inserted by this option should not be optional.

Will ffmpeg error out if it's built without the required bsf? (or is there
a hard dep in the configure?)

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/h264_parse: Treat escaped and unescaped decoding error equal in decode_extradata_ps_mp4()

2017-11-25 Thread Clément Bœsch
On Sat, Nov 25, 2017 at 10:49:09PM +0100, Michael Niedermayer wrote:
> Fixes: lorex.mp4
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/h264_parse.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
> index a7c71d9bbb..9216d0bdbd 100644
> --- a/libavcodec/h264_parse.c
> +++ b/libavcodec/h264_parse.c
> @@ -427,8 +427,6 @@ static int decode_extradata_ps_mp4(const uint8_t *buf, 
> int buf_size, H264ParamSe
>  
>  ret = decode_extradata_ps(escaped_buf, escaped_buf_size, ps, 1, 
> logctx);
>  av_freep(_buf);
> -if (ret < 0)
> -return ret;

If you don't want the return code to be reintroduced differently 10x in
the future (like, someone deciding to return ret in the function instead
of 0), I'd suggest 2 things:

- use "(void)decode_extradata_ps(...)" to explicitly ignore the code
  return; it's a hint for the compiler and the developer, typically used
  in OpenBSD (I believe that's because they warn about unchecked return
  code by default)
- add a comment above about the why

No comment on the change itself.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v3 2/3] libavcodec/utils.c: simplify avcodec locking with atomics

2017-11-25 Thread Clément Bœsch
On Sat, Nov 25, 2017 at 05:01:56PM +, Rostislav Pehlivanov wrote:
[...]
> -volatile int ff_avcodec_locked;
> +static atomic_bool ff_avcodec_locked;
>  static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0);
>  static void *codec_mutex;
>  static void *avformat_mutex;
> @@ -1937,6 +1937,7 @@ int av_lockmgr_register(int (*cb)(void **mutex, enum 
> AVLockOp op))
>  
>  int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>  {

> +_Bool exp = 1;

why _Bool instead of atomic_bool?

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavr: deprecate the entire library

2017-11-25 Thread Clément Bœsch
On Fri, Nov 17, 2017 at 03:58:16PM +, Rostislav Pehlivanov wrote:
[...]
> diff --git a/libavresample/utils.c b/libavresample/utils.c
> index b4fb906556..3e629fe901 100644
> --- a/libavresample/utils.c
> +++ b/libavresample/utils.c
> @@ -37,6 +37,9 @@ int avresample_open(AVAudioResampleContext *avr)
>  {
>  int ret;
>  
> +av_log(avr, AV_LOG_WARNING, "This library is being deprecated in favor 
> of libswresample, "
> +   "please migrate your program.");
> +

I'm fine with the patch but not so with this message.

Maybe "libavresample is not maintained by the FFmpeg project and will be
dropped at the next major bump. Please use libswresample instead."

And it probably needs a longer explanation somewhere (website/news/...)

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] 8-bit hevc decoding optimization on aarch64 with neon

2017-11-25 Thread Clément Bœsch
On Sat, Nov 18, 2017 at 06:35:48PM +0100, Rafal Dabrowa wrote:
> 
> This is a proposal of performance optimizations for 8-bit
> hevc video decoding on aarch64 platform with neon (simd) extension.
> 
> I'm testing my optimizations on NanoPi M3 device. I'm using
> mainly "Big Buck Bunny" video file in format 1280x720 for testing.
> The video file was pulled from libde265.org page, see
> http://www.libde265.org/hevc-bitstreams/bbb-1280x720-cfg06.mkv
> The movie duration is 00:10:34.53.
> 
> Overall performance gain is about 2x. Without optimizations the movie
> playback stops in practice after a few seconds. With
> optimizations the file is played smoothly 99% of the time.
> 
> For performance testing the following command was used:
> 
> time ./ffmpeg -hide_banner -i ~/bbb-1280x720-cfg06.mkv -f yuv4mpegpipe - 
> >/dev/null
> 
> The video file was pre-read before test to minimize disk reads during testing.
> Program execution time without optimization was as follows:
> 
> real  11m48.576s
> user  43m8.111s
> sys   0m12.469s
> 
> Execution time with optimizations:
> 
> real  6m17.046s
> user  21m19.792s
> sys   0m14.724s
> 

Can you post the results of checkasm --bench for hevc?

Did you run it to check for any calling convention violation?

> 
> The patch contains optimizations for most heavily used qpel, epel, sao and 
> idct
> functions.  Among the functions provided for optimization there are two
> intensively used, but not optimized in this patch: hevc_v_loop_filter_luma_8
> and hevc_h_loop_filter_luma_8. I have no idea how they could be optimized
> hence I leaved them without optimizations.
> 

You may want to check x86/hevc_deblock.asm then (no idea if these are
implemented).

[...]
> +function ff_hevc_put_hevc_pel_pixels4_8_neon, export=1
> +mov x7, 128
> +1:  ld1 { v0.s }[0], [x1], x2
> +ushll   v4.8h, v0.8b, 6

> +st1 { v4.d }[0], [x0], x7

using #128 not possible?

> +subsx3, x3, 1
> +b.ne1b
> +ret

here and below: no use of the x6 register?

A few comments on the style:

- please use a consistent spacing (current function mismatches with later
  code), preferably using a relatively large number of spaces as common
  ground (check the other sources)
- we use capitalized size suffixes (B, H, ...); and IIRC the lower case
  form are problematic with some assembler but don't quote me on that.
- we don't use spaces between {}

> +endfunc
> +
> +function ff_hevc_put_hevc_pel_pixels6_8_neon, export=1
> +mov x7, 120
> +1:  ld1 { v0.8b }, [x1], x2
> +ushll   v4.8h, v0.8b, 6

> +st1 { v4.d }[0], [x0], 8

I think you need to use # as prefix for the immediates

> +st1 { v4.s }[2], [x0], x7

I assume you can't use #120?

Have you checked if using #128 here and decrementing x0 afterward isn't
faster?

[...]
> +function ff_hevc_put_hevc_pel_bi_pixels32_8_neon, export=1
> +mov x10, 128
> +1:  ld1 { v0.16b, v1.16b }, [x2], x3// src
> +ushll   v16.8h, v0.8b, 6
> +ushll2  v17.8h, v0.16b, 6
> +ushll   v18.8h, v1.8b, 6
> +ushll2  v19.8h, v1.16b, 6
> +ld1 { v20.8h, v21.8h, v22.8h, v23.8h }, [x4], x10   // src2
> +sqadd   v16.8h, v16.8h, v20.8h
> +sqadd   v17.8h, v17.8h, v21.8h
> +sqadd   v18.8h, v18.8h, v22.8h
> +sqadd   v19.8h, v19.8h, v23.8h

> +sqrshrunv0.8b,  v16.8h, 7
> +sqrshrun2   v0.16b, v17.8h, 7
> +sqrshrunv1.8b,  v18.8h, 7
> +sqrshrun2   v1.16b, v19.8h, 7

does pairing helps here?

sqrshrunv0.8b,  v16.8h, 7
sqrshrunv1.8b,  v18.8h, 7
sqrshrun2   v0.16b, v17.8h, 7
sqrshrun2   v1.16b, v19.8h, 7

[...]
> +sqrshrunv0.8b,  v16.8h, 7
> +sqrshrun2   v0.16b, v17.8h, 7
> +sqrshrunv1.8b,  v18.8h, 7
> +sqrshrun2   v1.16b, v19.8h, 7
> +sqrshrunv2.8b,  v20.8h, 7
> +sqrshrun2   v2.16b, v21.8h, 7
> +sqrshrunv3.8b,  v22.8h, 7
> +sqrshrun2   v3.16b, v23.8h, 7

Again, this might be a good candidate for attempting to shuffle the
instructions and see if it helps (there are many other places, I picked
one randomly).

> +.Lepel_filters:

const/endconst + align might be better for all these labels

[...]
> +function ff_hevc_put_hevc_epel_hv12_8_neon, export=1
> +add x10, x3, 3
> +lsl x10, x10, 7
> +sub sp, sp, x10 // tmp_array
> +stp x0, x3, [sp, -16]!
> +stp x5, x30, [sp, -16]!
> +add x0, sp, 32
> +sub x1, x1, x2
> +add x3, x3, 3
> +bl  ff_hevc_put_hevc_epel_h12_8_neon
> +ldp x5, x30, [sp], 16
> +ldp x0, x3, [sp], 16
> +load_epel_filterh x5, x4
> +mov x5, 112
> +mov x10, 128
> +ld1 { v16.8h, v17.8h }, [sp], x10
> +ld1 { v18.8h, v19.8h }, [sp], x10
> +ld1 { v20.8h, v21.8h }, [sp], x10
> +1:  ld1 { v22.8h, v23.8h }, [sp], x10
> +calc_epelh  v4, v16, 

Re: [FFmpeg-devel] [PATCH] avfilter/vf_cropdetect: change license to LGPL

2017-11-16 Thread Clément Bœsch
On Tue, Nov 14, 2017 at 02:35:39PM -0800, Aman Gupta wrote:
[...]
> The import into ffmpeg was done by Stefano Sabatini. It was then touched by 
> the following
> contributors, who specifically made changes to the filter. This does not 
> include
> commits which changed this file but were part of larger refactorings that 
> changed
> lavfi APIs and other LGPL filters as well. See
> https://github.com/FFmpeg/FFmpeg/commits/master/libavfilter/vf_cropdetect.c
> 
>   Stefano Sabatini
>   Michael Niedermayer
>   Anton Khirnov
>   Clément Bœsch
>   Paul B Mahol
>   Carl Eugen Hoyos
>   Vittorio Giovara
>   Ganesh Ajjanagadde
> 
> The people above are CC'd on this patch for their permission to relicense 
> vf_cropdetect as LGPL.

Sure, OK for me.

You need to update LICENSE.md btw.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: add API for mb_types

2017-11-12 Thread Clément Bœsch
On Sun, Nov 12, 2017 at 01:55:14AM +, Rostislav Pehlivanov wrote:
[...]
> The range should be derived from the codec ID.

The frame doesn't contain the codec ID, so this information should be
added to that struct somehow (it needs to reach the filters who are
agnostics).

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

2017-11-09 Thread Clément Bœsch
On Fri, Nov 10, 2017 at 12:18:07AM +0100, Aurelien Jacobs wrote:
[...]
> > >   Also, allowing this but not the mixed statements and declarations means
> > >   this is a style decision and not a technical one anymore.
> 
> Allowing limiting the scope of a variable to a loop seems like a
> technical decision to me, but I understand that some consider it more
> of a style decision.
> 

What I meant by that is that currently the for (int ..) form is prevented
by style but also technical argument (compiler compatibility). If you
remove the technical argument by breaking the compiler compatibility of
such form, what remains is only a style choice to make between mixed
stat/decl and for (int ...); AFAIK there is no difference for a compiler
between the two.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

2017-11-09 Thread Clément Bœsch
On Thu, Nov 09, 2017 at 04:56:07PM -0300, James Almer wrote:
[...]
> > - this require a Changelog entry as it has a technical impact (which could
> >   be considered negligible).
> 
> No, Changelog is not for this kind of change.
> 

Sorry, I should have elaborated: I meant to document in the Changelog the
drop of support for the specific affected compilers.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/9] lavfi/paletteuse: check get_color return value

2017-11-09 Thread Clément Bœsch
On Wed, Nov 08, 2017 at 07:17:45PM +0100, Timo Rothenpieler wrote:
> Fixes CID #1420396
> ---
>  libavfilter/vf_paletteuse.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
> index ed80ab04d5..1980907e70 100644
> --- a/libavfilter/vf_paletteuse.c
> +++ b/libavfilter/vf_paletteuse.c
> @@ -380,8 +380,11 @@ static av_always_inline int 
> get_dst_color_err(PaletteUseContext *s,
>  const uint8_t r = c >> 16 & 0xff;
>  const uint8_t g = c >>  8 & 0xff;
>  const uint8_t b = c   & 0xff;
> +uint32_t dstc;
>  const int dstx = color_get(s, c, a, r, g, b, search_method);
> -const uint32_t dstc = s->palette[dstx];
> +if (dstx < 0)
> +return dstx;
> +dstc = s->palette[dstx];
>  *er = r - (dstc >> 16 & 0xff);
>  *eg = g - (dstc >>  8 & 0xff);
>  *eb = b - (dstc   & 0xff);

should be fine

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

2017-11-09 Thread Clément Bœsch
On Wed, Nov 08, 2017 at 09:26:13PM +, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  doc/developer.texi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index a7b4f1d737..de7d887451 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x = @{ .i 
> = 17 @};});
>  @item
>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>  
> +@item
> +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
> +

I'm fine with this and would be happy to update all the code I maintain in
FFmpeg to follow this pattern. But I have a few questions/reservations:

- what does it imply for mixed statements and declarations?

  If we still do not allow them, how are you going to make the compiler
  reject them but not the for (int ... ) form?

  Also, allowing this but not the mixed statements and declarations means
  this is a style decision and not a technical one anymore.

- are we going to accept all kind of patches to change the coding style
  all over the codebase?

- this require a Changelog entry as it has a technical impact (which could
  be considered negligible).

Overall, I'd enjoy this change being accepted (even along mixed statements
and declarations).

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] lavfi: check links properties after configuring them.

2017-11-01 Thread Clément Bœsch
On Wed, Nov 01, 2017 at 11:01:22PM +0100, Nicolas George wrote:
> Le primidi 11 brumaire, an CCXXVI, Clement Boesch a écrit :
> > nb_filters and nb_outputs are signed so the counters better be the same
> > type.
> 
> I re-checked, I think you are mistaken.
> 

My bad, you're right, I was looking at the wrong header with the same
variable names. Dismiss my comment.

> > I think we already had that discussion but I'd rather have the counters
> > signed so the compiler can exploit the undefined property of signed
> > overflow to assume it will never happen.
> 
> On the other hand, it forces it to make code that can handle negative as
> well, even though the value are positive. I really think it is better to
> inform the compiler of the intent, and the intent is that the numbers
> are positive, i.e. unsigned.

I'd say that a counter is unlikely to require a sign vs unsigned
optimization (and if it does and we know the counter is positive only, we
can explicit it with bit shifts etc). OTOH, the compiler will always have
to assume an overflow can happen if the initial counter value comes from
another variable, and you can't do much to hint it about it. Assuming a
loop overflow has IMO much more impact of what the compiler can do in the
general logic of the loop. But that's pure speculation from me.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] lavfi: check links properties after configuring them.

2017-11-01 Thread Clément Bœsch
On Wed, Nov 01, 2017 at 09:39:52PM +0100, Nicolas George wrote:
> For now, check the image size.
> Inspired by a patch from Paul B Mahol.
> 
> Invalid sizes would be detected later by allocation failures,
> detecting problems earlier is cleaner.
> 
> Signed-off-by: Nicolas George 
> ---
>  libavfilter/avfiltergraph.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 69cf26896d..a009e0a760 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/avstring.h"
>  #include "libavutil/bprint.h"
>  #include "libavutil/channel_layout.h"
> +#include "libavutil/imgutils.h"
>  #include "libavutil/internal.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> @@ -263,6 +264,27 @@ static int graph_config_links(AVFilterGraph *graph, 
> AVClass *log_ctx)
>  return 0;
>  }
>  
> +static int graph_check_links(AVFilterGraph *graph, AVClass *log_ctx)
> +{
> +AVFilterContext *f;
> +AVFilterLink *l;

> +unsigned i, j;

nb_filters and nb_outputs are signed so the counters better be the same
type.

I think we already had that discussion but I'd rather have the counters
signed so the compiler can exploit the undefined property of signed
overflow to assume it will never happen.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavfi/palettegen: Allow setting the background colour

2017-10-29 Thread Clément Bœsch
On Sun, Oct 29, 2017 at 01:47:03AM +0200, Carl Eugen Hoyos wrote:
> 2017-10-28 23:50 GMT+02:00 Clément Bœsch <u...@pkh.me>:
> > On Sat, Oct 28, 2017 at 10:57:32PM +0200, Carl Eugen Hoyos wrote:
> >> 2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos <ceffm...@gmail.com>:
> >>
> >> > Attached patch is useful in combination with the transparency patch
> >> > for paletteuse.
> >>
> >> > +{ "background", "set a background color for transparency", 
> >> > OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, 
> >> > CHAR_MAX, FLAGS },
> >>
> >> Locally changed to "lime" to make sure fate stays unchanged.
> >>
> >
> > Please use "transparency_color" as option name (and field struct).
> 
> I liked "background";-(
> 

But it's not a background color, it's a reference color code typically
used for the transparency optimization. It's a reserved entry such that
the encoder can perform a good compression.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] ffplay: create the window and the renderer before starting playback

2017-10-28 Thread Clément Bœsch
On Sat, Oct 28, 2017 at 11:05:15PM +0200, Marton Balint wrote:
> Signed-off-by: Marton Balint 
> ---
>  fftools/ffplay.c | 67 
> +---
>  1 file changed, 35 insertions(+), 32 deletions(-)
> 

Won't this prevent using ffplay without a display? Think of playing audio
with ffplay on a headless machine (no display server).

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavfi/palettegen: Allow setting the background colour

2017-10-28 Thread Clément Bœsch
On Sat, Oct 28, 2017 at 10:57:32PM +0200, Carl Eugen Hoyos wrote:
> 2017-10-17 23:42 GMT+02:00 Carl Eugen Hoyos :
> 
> > Attached patch is useful in combination with the transparency patch
> > for paletteuse.
> 
> > +{ "background", "set a background color for transparency", 
> > OFFSET(background), AV_OPT_TYPE_COLOR, {.str="white"}, CHAR_MIN, CHAR_MAX, 
> > FLAGS },
> 
> Locally changed to "lime" to make sure fate stays unchanged.
> 

Please use "transparency_color" as option name (and field struct).

Also, the correct type in the structure is uint8_t[4]

And ideally, you would also document the option in doc/filters.texi

You can apply any time with these changes.

Thanks,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavfi/paletteuse: fix to support transparency

2017-10-28 Thread Clément Bœsch
On Mon, Oct 23, 2017 at 07:12:57PM -0400, Bjorn Roche wrote:
> This patch enables paletteuse to identify the transparency in incoming
> video and tag transparent pixels on outgoing video with the correct
> index from the palette.
> 
> This requires tracking the transparency index in the palette,
> establishing an alpha threshold below which a pixel is considered
> transparent and above which the pixel is considered opaque, and
> additional changes to track the alpha value throughout the conversion
> process.
> 
> This change is a partial fix for https://trac.ffmpeg.org/ticket/4443
> However, animated GIFs are still output incorrectly due to a bug
> in gif optimization which does not correctly handle transparency.
> ---
>  doc/filters.texi|   7 ++
>  libavfilter/vf_paletteuse.c | 195 
> 

patch applied. I took the liberty to make the following changes before
pushing:

- fix spacing issues around () you forgot to fix
- rename "threshold" into "alpha_threshold"
- move the option documentation in the appropriate place (it was between
  diff_mode and one of its constant)

Thanks for the patch, and sorry about the delay.

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter/vf_tile: remove limit of max tile size

2017-10-28 Thread Clément Bœsch
On Sat, Oct 28, 2017 at 02:42:26PM +0200, Paul B Mahol wrote:
> On 10/28/17, Clement Boesch  wrote:
> > On Fri, Oct 27, 2017 at 10:38:11PM +0200, Nicolas George wrote:
> >> Le sextidi 6 brumaire, an CCXXVI, Paul B Mahol a ecrit :
> >> > Signed-off-by: Paul B Mahol 
> >> > ---
> >> >  libavfilter/vf_tile.c | 8 
> >> >  1 file changed, 8 deletions(-)
> >>
> >> Nack.
> >>
> >> ./ffmpeg_g -lavfi testsrc2=s=2x2,tile=65536x65536,scale=1024x1024 -f
> >> framecrc -
> >>
> >> hangs instead of returning a proper error.
> >>
> >
> > In case there is confusion with what Paul said: `ffmpeg -lavfi
> > testsrc2=s=2x2 -f framecrc -` hangs with current git/master, so the hang
> > you're reporting is likely unrelated to this patch. Instead, it looks like
> > a
> > problem related to the 2x2 size being too small for testsrc2 (testsrc
> > supports 2x2, testsrc2 doesn't).
> >
> > (Not saying the current patch is correct though)
> 
> I don't remmember I hired lawyer. Did I?

I'm not defending you, I'm explaining a potential misunderstanding between
Nicolas and you, giving information you should probably have provided
yourself to ease the communication to prevent pointless escalation.

Regards,

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter/vf_tile: remove limit of max tile size

2017-10-28 Thread Clément Bœsch
On Fri, Oct 27, 2017 at 10:38:11PM +0200, Nicolas George wrote:
> Le sextidi 6 brumaire, an CCXXVI, Paul B Mahol a écrit :
> > Signed-off-by: Paul B Mahol 
> > ---
> >  libavfilter/vf_tile.c | 8 
> >  1 file changed, 8 deletions(-)
> 
> Nack.
> 
> ./ffmpeg_g -lavfi testsrc2=s=2x2,tile=65536x65536,scale=1024x1024 -f framecrc 
> -
> 
> hangs instead of returning a proper error.
> 

In case there is confusion with what Paul said: `ffmpeg -lavfi
testsrc2=s=2x2 -f framecrc -` hangs with current git/master, so the hang
you're reporting is likely unrelated to this patch. Instead, it looks like a
problem related to the 2x2 size being too small for testsrc2 (testsrc
supports 2x2, testsrc2 doesn't).

(Not saying the current patch is correct though)

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavc: drop VDA

2017-10-23 Thread Clément Bœsch
On Fri, Sep 29, 2017 at 06:59:56PM -0300, James Almer wrote:
[...]
> Then IMO we should wait until the major bump (Which is not too far
> away). Leaving a stub object for the symbols and configure clutter
> should be avoided.

Patch applied.

Thanks

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] ffserver: Fix off by 1 error in path

2017-10-23 Thread Clément Bœsch
On Mon, Oct 23, 2017 at 10:42:53AM +0200, Clément Bœsch wrote:
[...]
> how about changing the whole chunk with (untested):
> 
> prog = av_strdup(my_program_name);
> dirname = av_dirname(prog);
> pathname = *dirname ? av_asprintf("%s/%s", dirname, "ffmpeg")
> : av_asprintf("ffmpeg")
> av_free(filepath);
  
  prog

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] ffserver: Fix off by 1 error in path

2017-10-23 Thread Clément Bœsch
On Sun, Oct 22, 2017 at 05:11:20PM +0200, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer 
> ---
>  fftools/ffserver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffserver.c b/fftools/ffserver.c
> index d4885dfa0e..51f31bc704 100644
> --- a/fftools/ffserver.c
> +++ b/fftools/ffserver.c
> @@ -499,9 +499,9 @@ static void start_children(FFServerStream *feed)
>  if (!slash) {
>  pathname = av_mallocz(sizeof("ffmpeg"));
>  } else {
> -pathname = av_mallocz(slash - my_program_name + sizeof("ffmpeg"));
> +pathname = av_mallocz(slash - my_program_name + 1 + 
> sizeof("ffmpeg"));
>  if (pathname != NULL) {
> -memcpy(pathname, my_program_name, slash - my_program_name);
> +memcpy(pathname, my_program_name, slash - my_program_name + 1);
>  }

maybe that's correct but the whole code around here needs rewrite.

how about changing the whole chunk with (untested):

prog = av_strdup(my_program_name);
dirname = av_dirname(prog);
pathname = *dirname ? av_asprintf("%s/%s", dirname, "ffmpeg")
: av_asprintf("ffmpeg")
av_free(filepath);

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/version: Postpone FF_API_DEBUG_MV

2017-10-23 Thread Clément Bœsch
On Sun, Oct 22, 2017 at 12:09:04PM -0400, Ronald S. Bultje wrote:
[...]
> > > The replacement will never be written if:
> > > a) nobody cares; AND
> > > b) we keep delaying the removal
> > >
> > > I'm considering veto'ing this patch.
> >
> > Personally I'm with Clement in this. The deprecation was poorly handled,
> > and the feature currently has no replacement. Michael uses it for
> > debugging, so removing it does not seem productive.
> >
> > But, also agreeing with Clement, this should absolutely be ported to the
> > codecview filter before the next bump, or it will be removed.
> >
> 
> But this is the whole problem. We're stuck in a stalemate between nothing
> and nobody. As with ffserver, we'll keep postponing this forever more until
> the stalemate is broken.

Well, AFAIK contrary to ffserver, this is not exactly much a hinder for
further development; MPEG related development is pretty much stalled these
days, and there is no real API abuse here, it's just in the wrong place.

Also, contrary to ffserver, maybe it lacked some visibility by the
interested parties to do the actual portage work.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/version: Postpone FF_API_DEBUG_MV

2017-10-22 Thread Clément Bœsch
On Sun, Oct 22, 2017 at 10:43:35AM -0300, James Almer wrote:
[...]
> Personally I'm with Clement in this. The deprecation was poorly handled,
> and the feature currently has no replacement. Michael uses it for
> debugging, so removing it does not seem productive.

Not just Michael, it's generally a good study case for understanding
codecs.

See https://trac.ffmpeg.org/wiki/Debug/MacroblocksAndMotionVectors

Apparently, some people were caring about it enough to document it.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 0/6] Patchset to remove ffserver

2017-10-22 Thread Clément Bœsch
On Sun, Oct 22, 2017 at 10:51:38AM +0200, Marton Balint wrote:
> 
> On Sun, 22 Oct 2017, Clément Bœsch wrote:
> 
> > On Sun, Oct 22, 2017 at 02:55:38AM +0200, Michael Niedermayer wrote:
> > > On Sat, Oct 21, 2017 at 04:15:37PM -0300, James Almer wrote:
> > > > On 10/21/2017 3:54 PM, Rostislav Pehlivanov wrote:
> > > > > This patchset removes the long-deprecated ffserver program and all
> > > > > its privately exposed things from libavformat.
> > > > > 
> > > > > Rostislav Pehlivanov (6):
> > > > >   Remove the ffserver program
> > > > >   libavformat: remove the ffmenc and ffmdec muxer and demuxers
> > > > >   libavformat: unexpose the ff_inet_aton function
> > > > >   libavformat: remove the ff_rtp_get_local_rtcp_port function
> > > > >   libavformat: unexpose private ff_ functions needed by ffserver
> > > > >   libavformat/mpjpeg: use "ffmpeg" instead of "ffserver" as boundary 
> > > > > tag
> > > > 
> > > > This set will be applied a month or so from now, when the unstable ABI
> > > > period is over.
> > > > 
> > > > If you can do in a month what was not done in a year plus, anyone is
> > > > welcome to fix all ffserver issues or preferably replace it altogether
> > > > with a new tool with a more user friendly syntax/interface.
> > > 
> > > Can you list the technical problems that require dropping ffserver,
> > > so that someone interrested in fixing them can do so ?
> > 
> > It's probably too late, one month is not enough. We already had that
> > discussion:
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/203482.html
> > 
> > The goal was ZERO internal API usage + at least partial FATE coverage. We
> > gave it a year and nothing changed because no one cared.
> 
> It is natural that people only care if it gets removed otherwise.
> 

July 10th, 2016, ffserver program being dropped

After thorough deliberation, we're announcing that we're about to drop
the ffserver program from the project starting with the next release.
ffserver has been a problematic program to maintain due to its use of
internal APIs, which complicated the recent cleanups to the
libavformat library, and block further cleanups and improvements which
are desired by API users and will be easier to maintain. Furthermore
the program has been hard for users to deploy and run due to
reliability issues, lack of knowledgable people to help and confusing
configuration file syntax. Current users and members of the community
are invited to write a replacement program to fill the same niche that
ffserver did using the new APIs and to contact us so we may point
users to test and contribute to its development. 

source: ffmpeg.org

Removal was announced, people were aware, we got discussions, but no one
cared enough to do something about it.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 0/6] Patchset to remove ffserver

2017-10-22 Thread Clément Bœsch
On Sun, Oct 22, 2017 at 02:55:38AM +0200, Michael Niedermayer wrote:
> On Sat, Oct 21, 2017 at 04:15:37PM -0300, James Almer wrote:
> > On 10/21/2017 3:54 PM, Rostislav Pehlivanov wrote:
> > > This patchset removes the long-deprecated ffserver program and all
> > > its privately exposed things from libavformat.
> > > 
> > > Rostislav Pehlivanov (6):
> > >   Remove the ffserver program
> > >   libavformat: remove the ffmenc and ffmdec muxer and demuxers
> > >   libavformat: unexpose the ff_inet_aton function
> > >   libavformat: remove the ff_rtp_get_local_rtcp_port function
> > >   libavformat: unexpose private ff_ functions needed by ffserver
> > >   libavformat/mpjpeg: use "ffmpeg" instead of "ffserver" as boundary tag
> > 
> > This set will be applied a month or so from now, when the unstable ABI
> > period is over.
> > 
> > If you can do in a month what was not done in a year plus, anyone is
> > welcome to fix all ffserver issues or preferably replace it altogether
> > with a new tool with a more user friendly syntax/interface.
> 
> Can you list the technical problems that require dropping ffserver,
> so that someone interrested in fixing them can do so ?

It's probably too late, one month is not enough. We already had that
discussion:
http://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/203482.html

The goal was ZERO internal API usage + at least partial FATE coverage. We
gave it a year and nothing changed because no one cared.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/version: Postpone FF_API_DEBUG_MV

2017-10-22 Thread Clément Bœsch
On Sat, Oct 21, 2017 at 11:23:29PM -0300, James Almer wrote:
[...]
> >> The commit even states it should have removed "all traces of its use",
> >> but it looks like with the years more and more code was added to it,
> >> seeing the removal commit 8933ac2079644fb09916f1875c569103aefe84b1 in
> >> libav doesn't even feature the massive amount of code we have under this
> >> deprecation wrapper in mpegvideo.
> >> So basically, the functionality was deprecated, but then further
> >> developed to the point removing it is almost impossible.
> >>
> >> How the hell did this happen?
> > 
> > I dont know, maybe,
> > Libav deprecating and removing as much FFmpeg code as they can
> > FFmpeg continuing to work on the FFmpeg code
> 
> No, i mean, why did a feature marked as deprecated keep getting
> development? Shouldn't the replacement have gotten it instead? And if a
> replacement was never planned, why wasn't the deprecation undone,
> instead of getting pushed further into the future indefinitely?

If I remember correctly (this needs double check), FFmpeg didn't continue
development of the frame debugging in MPEG after Libav deprecated it. On
the other hand, Libav did remove some code because it was broken by one of
their previous commit (see 37045e422; it was still working in FFmpeg at
that time unless I'm mistaken), so they had less code to deprecate in the
future.

Around that time, I was trying to export the motion vectors and render
them in a filter instead (known as codecview), which would soon become the
first step at dropping that code from the MPEG codec. I was less
interested in the macroblock type debugging which is also part of the
visual MPEG debugging "toolkit", so I left it as an exercise for people
interested in it. It would be nice to have it in codecview, but I wasn't
motivated to do all the work.

So anyway, implementing this requires to export the information as side
data, and port the code from the MPEG codec to the codecview filter. I
don't think it's wise to drop the MPEG code until this is done since it's
useful in practice. Delaying (again) the drop is fine with me.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-21 Thread Clément Bœsch
On Sat, Oct 21, 2017 at 09:47:47PM +0200, Carl Eugen Hoyos wrote:
> 2017-10-21 21:43 GMT+02:00 Clément Bœsch <u...@pkh.me>:
> > On Sat, Oct 21, 2017 at 09:37:06PM +0200, Carl Eugen Hoyos wrote:
> >> 2017-10-21 18:40 GMT+02:00 Clément Bœsch <u...@pkh.me>:
> >>
> >> > Aside from these nitpicks, I'm still concerned about how it's going
> >> > to conflict with GIF encoding where the transparent color is actually
> >> > used as a mean of not updating pixels from previous frames.
> >>
> >> But is this really related to this patch?
> >
> > Maybe not, but we need to keep that in mind and not make a
> > hasty decision wrt how we handle the transparency, because
> > it might makes future related development much harder.
> 
> Given that this is a libavfilter-only patch and we can reproduce
> the issue without using libavfilter, I am not completely
> convinced, but this is of course your decision.
> 

Yes it should be fine, I just want to be sure that using
palettegen/paletteuse will not create input streams that the limited GIF
encoder does not handle well because it doesn't make a difference between
"transparency flavours". If paletteuse starts inserting transparent colors
that are not meant to be used for the frame-diff system it could become a
problem.

In any case, I'll probably apply the patch at the next version, I'm just
not exactly comfortable with that part.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-21 Thread Clément Bœsch
On Sat, Oct 21, 2017 at 09:37:06PM +0200, Carl Eugen Hoyos wrote:
> 2017-10-21 18:40 GMT+02:00 Clément Bœsch <u...@pkh.me>:
> 
> > Aside from these nitpicks, I'm still concerned about how it's going
> > to conflict with GIF encoding where the transparent color is actually
> > used as a mean of not updating pixels from previous frames.
> 
> But is this really related to this patch?

Maybe not, but we need to keep that in mind and not make a hasty decision
wrt how we handle the transparency, because it might makes future related
development much harder.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-21 Thread Clément Bœsch
> Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

The commit message needs adjustment "lavfi/paletteuse: ..."

[...]
>  struct color_node {
> -uint8_t val[3];
> +uint8_t val[4];
>  uint8_t palette_id;
>  int split;
>  int left_id, right_id;
> @@ -86,6 +86,8 @@ typedef struct PaletteUseContext {
>  struct cache_node cache[CACHE_SIZE];/* lookup cache */
>  struct color_node map[AVPALETTE_COUNT]; /* 3D-Tree (KD-Tree with K=3) 
> for reverse colormap */
>  uint32_t palette[AVPALETTE_COUNT];

> +int transparency_index; /* index in the palette of transparency. -1 if 
> there isn't a transparency. */

"if there is no transparent color" might be more correct but I'm not a
native speaker.

> +int trans_thresh;
>  int palette_loaded;
>  int dither;
>  int new;
> @@ -116,6 +118,7 @@ static const AVOption paletteuse_options[] = {
>  { "bayer_scale", "set scale for bayer dithering", OFFSET(bayer_scale), 
> AV_OPT_TYPE_INT, {.i64=2}, 0, 5, FLAGS },
>  { "diff_mode",   "set frame difference mode", OFFSET(diff_mode),   
> AV_OPT_TYPE_INT, {.i64=DIFF_MODE_NONE}, 0, NB_DIFF_MODE-1, FLAGS, "diff_mode" 
> },
>  { "rectangle", "process smallest different rectangle", 0, 
> AV_OPT_TYPE_CONST, {.i64=DIFF_MODE_RECTANGLE}, INT_MIN, INT_MAX, FLAGS, 
> "diff_mode" },

> +{ "threshold", "set the alpha threshold for transparency. Above this 
> threshold, pixels are considered opaque, below they are considered 
> transparent.", OFFSET(trans_thresh), AV_OPT_TYPE_INT, {.i64=128}, 0, 255, },

The long explanation belongs in doc/filters.texi

[...]
> -static av_always_inline int diff(const uint8_t *c1, const uint8_t *c2)
> +static av_always_inline int diff(const uint8_t *c1, const uint8_t *c2, const 
> int trans_thresh)
>  {
>  // XXX: try L*a*b with CIE76 (dL*dL + da*da + db*db)
> -const int dr = c1[0] - c2[0];
> -const int dg = c1[1] - c2[1];
> -const int db = c1[2] - c2[2];
> -return dr*dr + dg*dg + db*db;

> +const static int max_diff = 195075; //equal to 255*255 + 255*255 + 
> 255*255

That's not what I meant; I wasn't concerned about the expression but the
static const int. I was thinking "return 255*255 + 255*255 + 255*255"

[...]
>  /**
>   * Check if the requested color is in the cache already. If not, find it in 
> the
>   * color tree and cache it.
> - * Note: r, g, and b are the component of c but are passed as well to avoid
> + * Note: a, r, g, and b are the components of argb, but are passed as well 
> to avoid
>   * recomputing them (they are generally computed by the caller for other 
> uses).
>   */
> -static av_always_inline int color_get(struct cache_node *cache, uint32_t 
> color,
> -  uint8_t r, uint8_t g, uint8_t b,

> +static av_always_inline int color_get(struct cache_node *cache, uint32_t 
> argb,

I'm sorry to insist, but renaming "color" belongs in another commit.

[...]
>  static av_always_inline int get_dst_color_err(struct cache_node *cache,

> -  uint32_t c, const struct 
> color_node *map,
> +  uint32_t argb, const struct 
> color_node *map,

ditto, "c" should be renamed in another commit

[...]
>  i = 0;
>  for (y = 0; y < palette_frame->height; y++) {
> -for (x = 0; x < palette_frame->width; x++)
> -s->palette[i++] = p[x];
> +for (x = 0; x < palette_frame->width; x++) {
> +s->palette[i] = p[x];
> +if (p[x]>>24 < s->trans_thresh) {
> +s->transparency_index = i; // we are assuming at most one 
> transparent color in palette
> +}

> +++i;

i++ is the appropriate idiom.

Aside from these nitpicks, I'm still concerned about how it's going to
conflict with GIF encoding where the transparent color is actually used as
a mean of not updating pixels from previous frames.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-20 Thread Clément Bœsch
On Fri, Oct 20, 2017 at 12:35:00AM +0200, Carl Eugen Hoyos wrote:
> 2017-10-20 0:00 GMT+02:00 Bjorn Roche :
> > The palettes generated by palettegen include one transparent color.
> > This patch enables paletteuse to identify the transparency in incoming
> > video and tag transparent pixels on outgoing video.
> >
> > This requires tracking the transparency index in the palette,
> > establishing an alpha threshold below which a pixel is considered
> > transparent and above which the pixel is considered opaque, and
> > additional changes to track the alpha value throught the conversion
> > process. If the format of a color variable has changed in this patch,
> > an effort was also made to change the variable name for clarity.
> >
> > This change is a partial fix for https://trac.ffmpeg.org/ticket/4443
> > However, animated GIFs are still output incorrectly due to a bug
> > in gif optimization which does not correctly handle transparency.
> 
> If Clément doesn't comment, I will probably push this in a few days.
> 

I need a little more time. I don't think I can do this today, but tomorrow
might be OK.

Sorry about the delay.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] Merge commit '7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63'

2017-10-12 Thread Clément Bœsch
On Thu, Oct 12, 2017 at 02:22:08PM -0400, Helmut K. C. Tessarek wrote:
> On 2017-10-12 13:25, James Almer wrote:
> > That'd be because showcqt is missing its fontconfig and freetype deps in
> > configure, a mistake that was hidden by extralibs being global before
> > this merge.
> 
> This does not seem to be the only mistake in configure.
> 
> It looks like there are dependencies missing for freetype as well:
> 

what's your configure line?

how is freetype built?

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add support for libopenjpeg 2.3

2017-10-08 Thread Clément Bœsch
On Sun, Oct 08, 2017 at 04:51:49PM +0530, Gyan Doshi wrote:
> OpenJPEG 2.3 was released a few days ago. Changelog reports "No API/ABI
> break compared to v2.2.0 but additional symbols for subset of components
> decoding"
> 
> This patch is adapted from an earlier patch which added support for 2.2.
> 
> Applied and tested locally.

> From c42f0c4290170cb49dc00f7898bee31d2e8ee814 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi 
> Date: Sun, 8 Oct 2017 14:59:23 +0530
> Subject: [PATCH] lavc: add support for openjpeg 2.3
> 
> Signed-off-by: Gyan Doshi 
> ---
>  configure   |  5 -
>  libavcodec/libopenjpegdec.c |  8 +---
>  libavcodec/libopenjpegenc.c | 10 ++
>  3 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/configure b/configure
> index 391c141e7a..3280e74f0f 100755
> --- a/configure
> +++ b/configure
> @@ -1930,6 +1930,7 @@ HEADERS_LIST="
>  machine_ioctl_meteor_h
>  malloc_h
>  opencv2_core_core_c_h
> +openjpeg_2_3_openjpeg_h
>  openjpeg_2_2_openjpeg_h
>  openjpeg_2_1_openjpeg_h
>  openjpeg_2_0_openjpeg_h
> @@ -5950,7 +5951,9 @@ enabled libopencv && { check_header 
> opencv2/core/core_c.h &&
>   require opencv opencv2/core/core_c.h 
> cvCreateImageHeader -lopencv_core -lopencv_imgproc; } ||
> require_pkg_config libopencv opencv 
> opencv/cxcore.h cvCreateImageHeader; }
>  enabled libopenh264   && require_pkg_config libopenh264 openh264 
> wels/codec_api.h WelsGetCodecVersion
> -enabled libopenjpeg   && { { check_lib libopenjpeg 
> openjpeg-2.2/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags 
> -DOPJ_STATIC; } ||
> +enabled libopenjpeg   && { { check_lib libopenjpeg 
> openjpeg-2.3/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags 
> -DOPJ_STATIC; } ||
> +   check_lib libopenjpeg openjpeg-2.3/openjpeg.h 
> opj_version -lopenjp2 ||
> +{ check_lib 
> libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && 
> add_cppflags -DOPJ_STATIC; } ||
> check_lib libopenjpeg openjpeg-2.2/openjpeg.h 
> opj_version -lopenjp2 ||
> { check_lib libopenjpeg 
> openjpeg-2.1/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags 
> -DOPJ_STATIC; } ||
> check_lib libopenjpeg openjpeg-2.1/openjpeg.h 
> opj_version -lopenjp2 ||

I'm sorry but this needs to stop. Why the hell do we have to add a ton of
garbage to the configure every time there is a new openjpeg release?

Why is it the only project that pollutes the configure like this at every
release?

[...]
> -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || 
> HAVE_OPENJPEG_2_0_OPENJPEG_H
> +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || 
> HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H

...and then this?

Yeah well, please put an end to this. This condition should be something
like #if OPENJPEG_MAJOR >= 2 or something like that.

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-07 Thread Clément Bœsch
On Sat, Oct 07, 2017 at 01:48:16PM +0200, Clément Bœsch wrote:
> > Subject: Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support 
> > transparency
> 
> "lavf/paletteuse: add support for transparency"
> 

Sorry, "lavfi", not "lavf".

Also, can you add the trac ticket reference for this?

[...]

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

2017-10-07 Thread Clément Bœsch
> Subject: Re: [FFmpeg-devel] [PATCH] Fix for paletteuse to support transparency

"lavf/paletteuse: add support for transparency"

On Fri, Oct 06, 2017 at 04:59:55PM -0400, Bjorn Roche wrote:
> ---
>  libavfilter/vf_paletteuse.c | 175 
> 
>  1 file changed, 112 insertions(+), 63 deletions(-)
> 
> diff --git a/libavfilter/vf_paletteuse.c b/libavfilter/vf_paletteuse.c
> index ffd37bf1da..4203543843 100644
> --- a/libavfilter/vf_paletteuse.c
> +++ b/libavfilter/vf_paletteuse.c
> @@ -56,7 +56,7 @@ enum diff_mode {
>  };
>  
>  struct color_node {
> -uint8_t val[3];
> +uint8_t val[4];
>  uint8_t palette_id;
>  int split;
>  int left_id, right_id;
> @@ -86,6 +86,7 @@ typedef struct PaletteUseContext {
>  struct cache_node cache[CACHE_SIZE];/* lookup cache */
>  struct color_node map[AVPALETTE_COUNT]; /* 3D-Tree (KD-Tree with K=3) 
> for reverse colormap */
>  uint32_t palette[AVPALETTE_COUNT];
> +int transparency_index; /* index in the palette of transparency. -1 if 
> there isn't a transparency. */
>  int palette_loaded;
>  int dither;
>  int new;
> @@ -108,6 +109,7 @@ typedef struct PaletteUseContext {
>  #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
>  static const AVOption paletteuse_options[] = {
>  { "dither", "select dithering mode", OFFSET(dither), AV_OPT_TYPE_INT, 
> {.i64=DITHERING_SIERRA2_4A}, 0, NB_DITHERING-1, FLAGS, "dithering_mode" },

> +{ "none","no dither",
>   0, AV_OPT_TYPE_CONST, {.i64=DITHERING_NONE},
> INT_MIN, INT_MAX, FLAGS, "dithering_mode" },

I think none is already supported as builtin in AVOption, isn't it? In any
case, this should probably be in a separated patch if deemed useful.

>  { "bayer",   "ordered 8x8 bayer dithering (deterministic)",  
>   0, AV_OPT_TYPE_CONST, {.i64=DITHERING_BAYER},   
> INT_MIN, INT_MAX, FLAGS, "dithering_mode" },
>  { "heckbert","dithering as defined by Paul Heckbert in 1982 
> (simple error diffusion)", 0, AV_OPT_TYPE_CONST, {.i64=DITHERING_HECKBERT},   
>  INT_MIN, INT_MAX, FLAGS, "dithering_mode" },
>  { "floyd_steinberg", "Floyd and Steingberg dithering (error 
> diffusion)",   0, AV_OPT_TYPE_CONST, 
> {.i64=DITHERING_FLOYD_STEINBERG}, INT_MIN, INT_MAX, FLAGS, "dithering_mode" },
> @@ -157,7 +159,8 @@ static int query_formats(AVFilterContext *ctx)
>  
>  static av_always_inline int dither_color(uint32_t px, int er, int eg, int 
> eb, int scale, int shift)
>  {
> -return av_clip_uint8((px >> 16 & 0xff) + ((er * scale) / (1< 16

> +return av_clip_uint8((px >> 24 & 0xff)  ) << 
> 24

Here and several times later, I believe you can drop the `& 0xff`.

> + | av_clip_uint8((px >> 16 & 0xff) + ((er * scale) / (1< 16
>   | av_clip_uint8((px >>  8 & 0xff) + ((eg * scale) / (1<  8
>   | av_clip_uint8((px   & 0xff) + ((eb * scale) / (1<  }
> @@ -165,10 +168,18 @@ static av_always_inline int dither_color(uint32_t px, 
> int er, int eg, int eb, in
>  static av_always_inline int diff(const uint8_t *c1, const uint8_t *c2)
>  {
>  // XXX: try L*a*b with CIE76 (dL*dL + da*da + db*db)
> -const int dr = c1[0] - c2[0];
> -const int dg = c1[1] - c2[1];
> -const int db = c1[2] - c2[2];
> -return dr*dr + dg*dg + db*db;
> +const static int max_diff = 255*255 + 255*255 + 255*255;
> +const int dr = c1[1] - c2[1];
> +const int dg = c1[2] - c2[2];
> +const int db = c1[3] - c2[3];
> +
> +if (c1[0] == 0 && c2[0] == 0) {
> +return 0;
> +} else if (c1[0] == c2[0]) {
> +return dr*dr + dg*dg + db*db;
> +} else {
> +return max_diff;
> +}

So if both alpha are 0, you consider the color identical, and otherwise if
both alpha are different, you consider the color completely different?

I guess that's OK. Please inline 255*255 + 255*255 + 255*255 in the return
though, I don't trust compilers into optimizing that static int.

>  }
>  
>  static av_always_inline uint8_t colormap_nearest_bruteforce(const uint32_t 
> *palette, const uint8_t *rgb)
> @@ -179,18 +190,20 @@ static av_always_inline uint8_t 
> colormap_nearest_bruteforce(const uint32_t *pale
>  const uint32_t c = palette[i];
>  
>  if ((c & 0xff00) == 0xff00) { // ignore transparent entry
> -const uint8_t palrgb[] = {
> +const uint8_t palargb[] = {
> +palette[i]>>24 & 0xff,

according to the condition just above, this is always 0xff. Is this what
you want?

>  palette[i]>>16 & 0xff,
>  palette[i]>> 8 & 0xff,
>  palette[i] & 0xff,
>  };
> -const int d = diff(palrgb, rgb);
> +const int 

  1   2   3   4   5   6   7   8   9   10   >