| Message ID | 1578716038-9645-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,V1,01/11] lavfi/spp: add "quality" option in runtime change path | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Sat, Jan 11, 2020 at 12:13:48PM +0800, Jun Zhao wrote: > From: Jun Zhao <barryjzhao@tencent.com> > > it's stranage to use option "level" in runtime change path but used > "quality" in option, add "quality" in runtime change path, it's more > intuitive and keep the "level" for compatibility. > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > --- > doc/filters.texi | 9 +++++++++ > libavfilter/vf_spp.c | 2 +- > 2 files changed, 10 insertions(+), 1 deletions(-) not sure both should be documented otherwise probably ok thx [...]
On Sun, Jan 12, 2020 at 2:50 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Sat, Jan 11, 2020 at 12:13:48PM +0800, Jun Zhao wrote: > > From: Jun Zhao <barryjzhao@tencent.com> > > > > it's stranage to use option "level" in runtime change path but used > > "quality" in option, add "quality" in runtime change path, it's more > > intuitive and keep the "level" for compatibility. > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > --- > > doc/filters.texi | 9 +++++++++ > > libavfilter/vf_spp.c | 2 +- > > 2 files changed, 10 insertions(+), 1 deletions(-) > > not sure both should be documented I can't find a special reason to document one argr, so will keep both. > > otherwise probably ok > > thx >
On Mon, Jan 13, 2020 at 9:19 AM mypopy@gmail.com <mypopy@gmail.com> wrote: > > On Sun, Jan 12, 2020 at 2:50 AM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > On Sat, Jan 11, 2020 at 12:13:48PM +0800, Jun Zhao wrote: > > > From: Jun Zhao <barryjzhao@tencent.com> > > > > > > it's stranage to use option "level" in runtime change path but used > > > "quality" in option, add "quality" in runtime change path, it's more > > > intuitive and keep the "level" for compatibility. > > > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > > --- > > > doc/filters.texi | 9 +++++++++ > > > libavfilter/vf_spp.c | 2 +- > > > 2 files changed, 10 insertions(+), 1 deletions(-) > > > > not sure both should be documented > I can't find a special reason to document one argr, so will keep both. > > > > otherwise probably ok patchset pushed, thanks
On Sat, Jan 11, 2020 at 12:13:48 +0800, Jun Zhao wrote: > +@item level > +@item quality > +Same as quality option. And the command accepts the @code{max} same as the @code{6}. > +@end table I'm sorry for coming in late, but this sentence doesn't make much sense to me: And the command accepts the "max" same as the 6. Are you trying to say that the allowed range for the command(s) is (are) the same as for the option? Moritz
On Mon, Jan 13, 2020 at 9:27 PM Moritz Barsnick <barsnick@gmx.net> wrote: > > On Sat, Jan 11, 2020 at 12:13:48 +0800, Jun Zhao wrote: > > +@item level > > +@item quality > > +Same as quality option. And the command accepts the @code{max} same as the @code{6}. > > +@end table > > I'm sorry for coming in late, but this sentence doesn't make much sense > to me: > And the command accepts the "max" same as the 6. > Are you trying to say that the allowed range for the command(s) is > (are) the same as for the option? > No, from the code, if (!strcmp(args, "max")) s->log2_count = MAX_LEVEL; this means, we can setting value "max" or "6" (MAX_LEVEL)
On Tue, Jan 14, 2020 at 09:31:56 +0800, mypopy@gmail.com wrote: > > On Sat, Jan 11, 2020 at 12:13:48 +0800, Jun Zhao wrote: > > > +Same as quality option. And the command accepts the @code{max} same as the @code{6}. > > Are you trying to say that the allowed range for the command(s) is > > (are) the same as for the option? > No, from the code, > > if (!strcmp(args, "max")) > s->log2_count = MAX_LEVEL; > > this means, we can setting value "max" or "6" (MAX_LEVEL) Sorry, I didn't check the code, but was reading the docs from a user point of view. I suggest fixing this to: Same as quality option. The command accepts the value @code{max} with the same meaning as @code{6}. Or something like this. Thanks, Moritz
On Tue, Jan 14, 2020 at 8:46 PM Moritz Barsnick <barsnick@gmx.net> wrote: > > On Tue, Jan 14, 2020 at 09:31:56 +0800, mypopy@gmail.com wrote: > > > On Sat, Jan 11, 2020 at 12:13:48 +0800, Jun Zhao wrote: > > > > +Same as quality option. And the command accepts the @code{max} same as the @code{6}. > > > > Are you trying to say that the allowed range for the command(s) is > > > (are) the same as for the option? > > > No, from the code, > > > > if (!strcmp(args, "max")) > > s->log2_count = MAX_LEVEL; > > > > this means, we can setting value "max" or "6" (MAX_LEVEL) > > Sorry, I didn't check the code, but was reading the docs from a user > point of view. > > I suggest fixing this to: > > Same as quality option. The command accepts the value @code{max} with > the same meaning as @code{6}. > Will update the doc part, thanks the suggestion.
diff --git a/doc/filters.texi b/doc/filters.texi index a2f862e..7459255 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -17271,6 +17271,15 @@ option may cause flicker since the B-Frames have often larger QP. Default is @code{0} (not enabled). @end table +@subsection Commands + +This filter supports the following commands: +@table @option +@item level +@item quality +Same as quality option. And the command accepts the @code{max} same as the @code{6}. +@end table + @section sr Scale the input by applying one of the super-resolution methods based on diff --git a/libavfilter/vf_spp.c b/libavfilter/vf_spp.c index fe579ce..db381cd 100644 --- a/libavfilter/vf_spp.c +++ b/libavfilter/vf_spp.c @@ -444,7 +444,7 @@ static int process_command(AVFilterContext *ctx, const char *cmd, const char *ar { SPPContext *s = ctx->priv; - if (!strcmp(cmd, "level")) { + if (!strcmp(cmd, "level") || !strcmp(cmd, "quality")) { if (!strcmp(args, "max")) s->log2_count = MAX_LEVEL; else