Re: [FFmpeg-devel] [PATCH] lavfi/pan: allow negative gain parameters also for other inputs than the first named
On Sat, Oct 29, 2016 at 03:03:58PM +0200, Nicolas George wrote: > Le septidi 7 brumaire, an CCXXV, Moritz Barsnick a écrit : > > Expands the parser to also accept the separator '-' in addition to > > '+', and take the negative sign into consideration. > > > > The optional sign for the first factor in the expression is already > > covered by parsing for an integer. > > > > Signed-off-by: Moritz Barsnick > > --- > > doc/filters.texi | 2 +- > > libavfilter/af_pan.c | 10 +++--- > > 2 files changed, 8 insertions(+), 4 deletions(-) > > Still LGTM, of course, thanks. applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/pan: allow negative gain parameters also for other inputs than the first named
Le septidi 7 brumaire, an CCXXV, Moritz Barsnick a écrit : > Expands the parser to also accept the separator '-' in addition to > '+', and take the negative sign into consideration. > > The optional sign for the first factor in the expression is already > covered by parsing for an integer. > > Signed-off-by: Moritz Barsnick > --- > doc/filters.texi | 2 +- > libavfilter/af_pan.c | 10 +++--- > 2 files changed, 8 insertions(+), 4 deletions(-) Still LGTM, of course, thanks. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/pan: allow negative gain parameters also for other inputs than the first named
On Thu, Oct 13, 2016 at 11:31:22 +0200, Nicolas George wrote: > Nit: inconsistent placement of the else clause. Not blocking. Fixed, and ping. Thanks, Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/pan: allow negative gain parameters also for other inputs than the first named
Le decadi 20 vendémiaire, an CCXXV, Moritz Barsnick a écrit : > Expands the parser to also accept the separator '-' in addition to > '+', and take the negative sign into consideration. > > Signed-off-by: Moritz Barsnick > --- > doc/filters.texi | 2 +- > libavfilter/af_pan.c | 11 --- > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index 4b2f7bf..fb4756e 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -3001,7 +3001,7 @@ output channel layout or number of channels > > @item outdef > output channel specification, of the form: > -"@var{out_name}=[@var{gain}*]@var{in_name}[+[@var{gain}*]@var{in_name}...]" > +"@var{out_name}=[@var{gain}*]@var{in_name}[(+-)[@var{gain}*]@var{in_name}...]" > > @item out_name > output channel to define, either a channel name (FL, FR, etc.) or a channel > diff --git a/libavfilter/af_pan.c b/libavfilter/af_pan.c > index fbd79a5..161097a 100644 > --- a/libavfilter/af_pan.c > +++ b/libavfilter/af_pan.c > @@ -102,7 +102,7 @@ static av_cold int init(AVFilterContext *ctx) > { > PanContext *const pan = ctx->priv; > char *arg, *arg0, *tokenizer, *args = av_strdup(pan->args); > -int out_ch_id, in_ch_id, len, named, ret; > +int out_ch_id, in_ch_id, len, named, ret, sign = 1; > int nb_in_channels[2] = { 0, 0 }; // number of unnamed and named input > channels > double gain; > > @@ -178,15 +178,20 @@ static av_cold int init(AVFilterContext *ctx) > ret = AVERROR(EINVAL); > goto fail; > } > -pan->gain[out_ch_id][in_ch_id] = gain; > +pan->gain[out_ch_id][in_ch_id] = sign * gain; > skip_spaces(&arg); > if (!*arg) > break; > -if (*arg != '+') { > +if (*arg == '-') { > +sign = -1; > +} else if (*arg != '+') { > av_log(ctx, AV_LOG_ERROR, "Syntax error near \"%.8s\"\n", > arg); > ret = AVERROR(EINVAL); > goto fail; > } > +else { Nit: inconsistent placement of the else clause. Not blocking. > +sign = 1; > +} > arg++; > } > } I would have put the "sign = 1" unconditionally before the test for the delimiter, but this version works too, so LGTM. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/pan: allow negative gain parameters also for other inputs than the first named
On Mon, Oct 10, 2016 at 19:09:34 +0200, Nicolas George wrote: > Maybe I am missing something, but I do not see where sign is reset to 1 > before the next run of the loop. You are correct. Too stupid me, I must pay more attention. And improve my testing. ;-) New patch coming. Thanks for checking, Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavfi/pan: allow negative gain parameters also for other inputs than the first named
L'octidi 18 vendémiaire, an CCXXV, Moritz Barsnick a écrit : > Expands the parser to also accept the separator '-' in addition to > '+', and take the negative sign into consideration. > > Signed-off-by: Moritz Barsnick > --- > doc/filters.texi | 2 +- > libavfilter/af_pan.c | 8 +--- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index 4b2f7bf..64934f7 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -3001,7 +3001,7 @@ output channel layout or number of channels > > @item outdef > output channel specification, of the form: > -"@var{out_name}=[@var{gain}*]@var{in_name}[+[@var{gain}*]@var{in_name}...]" > +"@var{out_name}=[@var{gain}*]@var{in_name}[(+|-)[@var{gain}*]@var{in_name}...]" > > @item out_name > output channel to define, either a channel name (FL, FR, etc.) or a channel > diff --git a/libavfilter/af_pan.c b/libavfilter/af_pan.c > index 3037864..f395c71 100644 > --- a/libavfilter/af_pan.c > +++ b/libavfilter/af_pan.c > @@ -102,7 +102,7 @@ static av_cold int init(AVFilterContext *ctx) > { > PanContext *const pan = ctx->priv; > char *arg, *arg0, *tokenizer, *args = av_strdup(pan->args); > -int out_ch_id, in_ch_id, len, named, ret; > +int out_ch_id, in_ch_id, len, named, ret, sign = 1; > int nb_in_channels[2] = { 0, 0 }; // number of unnamed and named input > channels > double gain; > > @@ -178,11 +178,13 @@ static av_cold int init(AVFilterContext *ctx) > ret = AVERROR(EINVAL); > goto fail; > } > -pan->gain[out_ch_id][in_ch_id] = gain; > +pan->gain[out_ch_id][in_ch_id] = sign * gain; > skip_spaces(&arg); > if (!*arg) > break; > -if (*arg != '+') { > +if (*arg == '-') { > +sign = -1; > +} else if (*arg != '+') { > av_log(ctx, AV_LOG_ERROR, "Syntax error near \"%.8s\"\n", > arg); > ret = AVERROR(EINVAL); > goto fail; Maybe I am missing something, but I do not see where sign is reset to 1 before the next run of the loop. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel