Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-19 Thread Derek Buitenhuis
On Sun, Jul 19, 2015 at 1:13 PM, Clément Bœsch  wrote:
>> Is there any reasonable way to determine when to print such a warning? Seems
>> silly to warn over e.g. 40 frames.
>
> I meant in the documentation. I don't know for the code.

OK.

>> How does this look:
>>
>> -vf trim=end=10,reverse
>>
>
> Sure. Look how other examples are formatted and fine with me.

OK.

>> So the other option is to remove the check for it?
>
> Yes, it's always 0 currently since you didn't add the timeline flag.

OK. Removed.

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


Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-19 Thread Clément Bœsch
On Thu, Jul 16, 2015 at 09:01:53PM +0100, Derek Buitenhuis wrote:
> On Thu, Jul 16, 2015 at 7:43 PM, Clément Bœsch  wrote:
> >> +Reverses a clip. Requires memory to buffer the entire clip, so trimming 
> >> is suggested.
> >
> > We use infinitive form, so "Reverse".
> 
> Done locally.
> 
> > You might want to print "Warning: this filter requires ..."
> 
> Is there any reasonable way to determine when to print such a warning? Seems
> silly to warn over e.g. 40 frames.
> 

I meant in the documentation. I don't know for the code.

> > Can you add an example to make sure users don't membomb too much their
> > OS?
> 
> How does this look:
> 
> -vf trim=end=10,reverse
> 

Sure. Look how other examples are formatted and fine with me.

> >> +if (ret == AVERROR_EOF && !ctx->is_disabled && s->nb_frames > 0) {
> >
> > is_disabled suggest a timeline support. You could add that if you feel
> > like it. That way, "reverse=enable='between(t,30,40)'" would reverse only
> > between t=30 and t=40 and pass through the rest of the time.
> 
> So the other option is to remove the check for it?
> 

Yes, it's always 0 currently since you didn't add the timeline flag.

> - Derek

-- 
Clément B.


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


Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-17 Thread Derek Buitenhuis
>> The design is how ubitux requested. Use trim with
>> it or risk using ALL THEM MEMORY.
>
> I always wondered if these kind of filters should
> require an option like "-i_dont_care_about_oom"
> but I am not saying it changes much...

Wasn't my idea. ;)

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


Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-17 Thread Carl Eugen Hoyos
Derek Buitenhuis  gmail.com> writes:

> The design is how ubitux requested. Use trim with 
> it or risk using ALL THEM MEMORY.

I always wondered if these kind of filters should 
require an option like "-i_dont_care_about_oom" 
but I am not saying it changes much...

Carl Eugen

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


Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-16 Thread Derek Buitenhuis
> Missing Changelog entry.

Added locally.

>> > +typedef struct ReverseContext {
>> > +const AVClass *class;
>
> Apparently not needed.

Woops. Yeah. Obviously. Removed.

>> > +AVFilterContext *ctx = inlink->dst;
>> > +ReverseContext *s= ctx->priv;
>> > +
>> > +
>
> Extra newline.

Fixed.

> Rest looks good.

[...]

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


Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-16 Thread Derek Buitenhuis
On Thu, Jul 16, 2015 at 7:43 PM, Clément Bœsch  wrote:
>> +Reverses a clip. Requires memory to buffer the entire clip, so trimming is 
>> suggested.
>
> We use infinitive form, so "Reverse".

Done locally.

> You might want to print "Warning: this filter requires ..."

Is there any reasonable way to determine when to print such a warning? Seems
silly to warn over e.g. 40 frames.

> Can you add an example to make sure users don't membomb too much their
> OS?

How does this look:

-vf trim=end=10,reverse

>> +if (ret == AVERROR_EOF && !ctx->is_disabled && s->nb_frames > 0) {
>
> is_disabled suggest a timeline support. You could add that if you feel
> like it. That way, "reverse=enable='between(t,30,40)'" would reverse only
> between t=30 and t=40 and pass through the rest of the time.

So the other option is to remove the check for it?

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


Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-16 Thread Paul B Mahol
Dana 16. 7. 2015. 20:43 osoba "Clément Bœsch"  napisala je:
>
> On Thu, Jul 16, 2015 at 05:43:59PM +, Derek Buitenhuis wrote:
> > Signed-off-by: Derek Buitenhuis 
> > ---
> > The design is how ubitux requested. Use trim with it or risk using ALL
THEM MEMORY.
>
> ;)
>
> > ---
> >  doc/filters.texi |   4 ++
> >  libavfilter/Makefile |   1 +
> >  libavfilter/allfilters.c |   1 +
> >  libavfilter/version.h|   2 +-
> >  libavfilter/vf_reverse.c | 145

Missing Changelog entry.
+++
> >  5 files changed, 152 insertions(+), 1 deletion(-)
> >  create mode 100644 libavfilter/vf_reverse.c
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 3b4ec2c..7b0410a 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -8247,6 +8247,10 @@ pixels will slow things down on a large logo.
> >  This filter uses the repeat_field flag from the Video ES headers and
hard repeats
> >  fields based on its value.
> >
> > +@section reverse
> > +
> > +Reverses a clip. Requires memory to buffer the entire clip, so
trimming is suggested.
>
> We use infinitive form, so "Reverse".
>
> You might want to print "Warning: this filter requires ..."
>
> Can you add an example to make sure users don't membomb too much their
> OS?
>
> > +
> >  @section rotate
> >
> >  Rotate video by an arbitrary angle expressed in radians.
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index 058b9e9..1638ae8 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -185,6 +185,7 @@ OBJS-$(CONFIG_QP_FILTER) +=
vf_qp.o
> >  OBJS-$(CONFIG_REMOVEGRAIN_FILTER)+= vf_removegrain.o
> >  OBJS-$(CONFIG_REMOVELOGO_FILTER) += bbox.o lswsutils.o
lavfutils.o vf_removelogo.o
> >  OBJS-$(CONFIG_REPEATFIELDS_FILTER)   += vf_repeatfields.o
> > +OBJS-$(CONFIG_REVERSE_FILTER)+= vf_reverse.o
> >  OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o
> >  OBJS-$(CONFIG_SEPARATEFIELDS_FILTER) += vf_separatefields.o
> >  OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o
> > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> > index b0d8410..5e2a322 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -200,6 +200,7 @@ void avfilter_register_all(void)
> >  REGISTER_FILTER(REMOVEGRAIN,removegrain,vf);
> >  REGISTER_FILTER(REMOVELOGO, removelogo, vf);
> >  REGISTER_FILTER(REPEATFIELDS,   repeatfields,   vf);
> > +REGISTER_FILTER(REVERSE,reverse,vf);
> >  REGISTER_FILTER(ROTATE, rotate, vf);
> >  REGISTER_FILTER(SAB,sab,vf);
> >  REGISTER_FILTER(SCALE,  scale,  vf);
> > diff --git a/libavfilter/version.h b/libavfilter/version.h
> > index 618c626..d22b2c5 100644
> > --- a/libavfilter/version.h
> > +++ b/libavfilter/version.h
> > @@ -30,7 +30,7 @@
> >  #include "libavutil/version.h"
> >
> >  #define LIBAVFILTER_VERSION_MAJOR  5
> > -#define LIBAVFILTER_VERSION_MINOR  22
> > +#define LIBAVFILTER_VERSION_MINOR  23
> >  #define LIBAVFILTER_VERSION_MICRO 100
> >
> >  #define LIBAVFILTER_VERSION_INT
AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> > diff --git a/libavfilter/vf_reverse.c b/libavfilter/vf_reverse.c
> > new file mode 100644
> > index 000..00512be
> > --- /dev/null
> > +++ b/libavfilter/vf_reverse.c
> > @@ -0,0 +1,145 @@
> > +/*
> > + * Copyright (c) 2015 Derek Buitenhuis
> > + *
> > + * 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/opt.h"
> > +#include "avfilter.h"
> > +#include "formats.h"
> > +#include "internal.h"
> > +#include "video.h"
> > +
> > +#define DEFAULT_LENGTH 300
> > +
> > +typedef struct ReverseContext {
> > +const AVClass *class;

Apparently not needed.

> > +
> > +int nb_frames;
> > +AVFrame **frames;
> > +unsigned int frames_size;
> > +unsigned int pts_size;
> > +int64_t *pts;
> > +int flush_idx;
> > +} ReverseContext;
> > +
> > +static av_cold int init(AVFilterContext *ctx)
> > +{
> > +ReverseContext *s = ctx->priv;
> > +
> > +s->pts = av_fast_realloc(NULL, &s-

Re: [FFmpeg-devel] [PATCH] avfilter: Add reverse filter

2015-07-16 Thread Clément Bœsch
On Thu, Jul 16, 2015 at 05:43:59PM +, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis 
> ---
> The design is how ubitux requested. Use trim with it or risk using ALL THEM 
> MEMORY.

;)

> ---
>  doc/filters.texi |   4 ++
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/version.h|   2 +-
>  libavfilter/vf_reverse.c | 145 
> +++
>  5 files changed, 152 insertions(+), 1 deletion(-)
>  create mode 100644 libavfilter/vf_reverse.c
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 3b4ec2c..7b0410a 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -8247,6 +8247,10 @@ pixels will slow things down on a large logo.
>  This filter uses the repeat_field flag from the Video ES headers and hard 
> repeats
>  fields based on its value.
>  
> +@section reverse
> +
> +Reverses a clip. Requires memory to buffer the entire clip, so trimming is 
> suggested.

We use infinitive form, so "Reverse".

You might want to print "Warning: this filter requires ..."

Can you add an example to make sure users don't membomb too much their
OS?

> +
>  @section rotate
>  
>  Rotate video by an arbitrary angle expressed in radians.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 058b9e9..1638ae8 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -185,6 +185,7 @@ OBJS-$(CONFIG_QP_FILTER) += vf_qp.o
>  OBJS-$(CONFIG_REMOVEGRAIN_FILTER)+= vf_removegrain.o
>  OBJS-$(CONFIG_REMOVELOGO_FILTER) += bbox.o lswsutils.o 
> lavfutils.o vf_removelogo.o
>  OBJS-$(CONFIG_REPEATFIELDS_FILTER)   += vf_repeatfields.o
> +OBJS-$(CONFIG_REVERSE_FILTER)+= vf_reverse.o
>  OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o
>  OBJS-$(CONFIG_SEPARATEFIELDS_FILTER) += vf_separatefields.o
>  OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index b0d8410..5e2a322 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -200,6 +200,7 @@ void avfilter_register_all(void)
>  REGISTER_FILTER(REMOVEGRAIN,removegrain,vf);
>  REGISTER_FILTER(REMOVELOGO, removelogo, vf);
>  REGISTER_FILTER(REPEATFIELDS,   repeatfields,   vf);
> +REGISTER_FILTER(REVERSE,reverse,vf);
>  REGISTER_FILTER(ROTATE, rotate, vf);
>  REGISTER_FILTER(SAB,sab,vf);
>  REGISTER_FILTER(SCALE,  scale,  vf);
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index 618c626..d22b2c5 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -30,7 +30,7 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVFILTER_VERSION_MAJOR  5
> -#define LIBAVFILTER_VERSION_MINOR  22
> +#define LIBAVFILTER_VERSION_MINOR  23
>  #define LIBAVFILTER_VERSION_MICRO 100
>  
>  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> diff --git a/libavfilter/vf_reverse.c b/libavfilter/vf_reverse.c
> new file mode 100644
> index 000..00512be
> --- /dev/null
> +++ b/libavfilter/vf_reverse.c
> @@ -0,0 +1,145 @@
> +/*
> + * Copyright (c) 2015 Derek Buitenhuis
> + *
> + * 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/opt.h"
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +#define DEFAULT_LENGTH 300
> +
> +typedef struct ReverseContext {
> +const AVClass *class;
> +
> +int nb_frames;
> +AVFrame **frames;
> +unsigned int frames_size;
> +unsigned int pts_size;
> +int64_t *pts;
> +int flush_idx;
> +} ReverseContext;
> +
> +static av_cold int init(AVFilterContext *ctx)
> +{
> +ReverseContext *s = ctx->priv;
> +
> +s->pts = av_fast_realloc(NULL, &s->pts_size,
> + DEFAULT_LENGTH * sizeof(*(s->pts)));
> +if (!s->pts)
> +return AVERROR(ENOMEM);
> +
> +s->frames = av_fast_realloc(NULL, &s->frames_size,
> +DEFAULT_LENGTH * sizeof(*(s->frames)));
> +if (!s->frames) {
> +av_freep(&s->pts);
> +