diff mbox series

[FFmpeg-devel,1/2] ffplay: add -scaling_quality option for SDL

Message ID 20240530213657.1033191-1-ramiro.polla@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] ffplay: add -scaling_quality option for SDL | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Ramiro Polla May 30, 2024, 9:36 p.m. UTC
---
 doc/ffplay.texi  | 2 ++
 fftools/ffplay.c | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Ramiro Polla June 4, 2024, 1 p.m. UTC | #1
On Thu, May 30, 2024 at 11:36 PM Ramiro Polla <ramiro.polla@gmail.com> wrote:
>
> ---
>  doc/ffplay.texi  | 2 ++
>  fftools/ffplay.c | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/doc/ffplay.texi b/doc/ffplay.texi
> index 93f77eeece..60f883e159 100644
> --- a/doc/ffplay.texi
> +++ b/doc/ffplay.texi
> @@ -72,6 +72,8 @@ as 100.
>  Force format.
>  @item -window_title @var{title}
>  Set window title (default is the input filename).
> +@item -scaling_quality @var{value}
> +Set SDL_HINT_RENDER_SCALE_QUALITY value (default is "linear").
>  @item -left @var{title}
>  Set the x position for the left of the window (default is a centered window).
>  @item -top @var{title}
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index b9d11eecee..75d2bec777 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -351,6 +351,7 @@ static int filter_nbthreads = 0;
>  static int enable_vulkan = 0;
>  static char *vulkan_params = NULL;
>  static const char *hwaccel = NULL;
> +static const char *scaling_quality = NULL;
>
>  /* current context */
>  static int is_full_screen;
> @@ -3683,6 +3684,7 @@ static const OptionDef options[] = {
>      { "framedrop",          OPT_TYPE_BOOL,   OPT_EXPERT, { &framedrop }, "drop frames when cpu is too slow", "" },
>      { "infbuf",             OPT_TYPE_BOOL,   OPT_EXPERT, { &infinite_buffer }, "don't limit the input buffer size (useful with realtime streams)", "" },
>      { "window_title",       OPT_TYPE_STRING,          0, { &window_title }, "set window title", "window title" },
> +    { "scaling_quality",    OPT_TYPE_STRING, OPT_EXPERT, { &scaling_quality }, "set SDL_HINT_RENDER_SCALE_QUALITY value (default=linear)", "value" },
>      { "left",               OPT_TYPE_INT,    OPT_EXPERT, { &screen_left }, "set the x position for the left of the window", "x pos" },
>      { "top",                OPT_TYPE_INT,    OPT_EXPERT, { &screen_top }, "set the y position for the top of the window", "y pos" },
>      { "vf",                 OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_EXPERT, { .func_arg = opt_add_vfilter }, "set video filters", "filter_graph" },
> @@ -3831,7 +3833,9 @@ int main(int argc, char **argv)
>              }
>          }
>          window = SDL_CreateWindow(program_name, SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, default_width, default_height, flags);
> -        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "linear");
> +        if (!scaling_quality)
> +            scaling_quality = "linear";
> +        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, scaling_quality);
>          if (!window) {
>              av_log(NULL, AV_LOG_FATAL, "Failed to create window: %s", SDL_GetError());
>              do_exit(NULL);
> --
> 2.39.2
>

Can anyone comment on this? I had a few doubts on this patch:
- does the option name properly convey its functionality?
- is the documentation too terse?
- should we include the accepted values in the documentation, even
though they are sdl-specific?
Marton Balint June 10, 2024, 7:04 p.m. UTC | #2
On Tue, 4 Jun 2024, Ramiro Polla wrote:

> On Thu, May 30, 2024 at 11:36 PM Ramiro Polla <ramiro.polla@gmail.com> wrote:
>>
>> ---
>>  doc/ffplay.texi  | 2 ++
>>  fftools/ffplay.c | 6 +++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/ffplay.texi b/doc/ffplay.texi
>> index 93f77eeece..60f883e159 100644
>> --- a/doc/ffplay.texi
>> +++ b/doc/ffplay.texi
>> @@ -72,6 +72,8 @@ as 100.
>>  Force format.
>>  @item -window_title @var{title}
>>  Set window title (default is the input filename).
>> +@item -scaling_quality @var{value}
>> +Set SDL_HINT_RENDER_SCALE_QUALITY value (default is "linear").
>>  @item -left @var{title}
>>  Set the x position for the left of the window (default is a centered window).
>>  @item -top @var{title}
>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
>> index b9d11eecee..75d2bec777 100644
>> --- a/fftools/ffplay.c
>> +++ b/fftools/ffplay.c
>> @@ -351,6 +351,7 @@ static int filter_nbthreads = 0;
>>  static int enable_vulkan = 0;
>>  static char *vulkan_params = NULL;
>>  static const char *hwaccel = NULL;
>> +static const char *scaling_quality = NULL;
>>
>>  /* current context */
>>  static int is_full_screen;
>> @@ -3683,6 +3684,7 @@ static const OptionDef options[] = {
>>      { "framedrop",          OPT_TYPE_BOOL,   OPT_EXPERT, { &framedrop }, "drop frames when cpu is too slow", "" },
>>      { "infbuf",             OPT_TYPE_BOOL,   OPT_EXPERT, { &infinite_buffer }, "don't limit the input buffer size (useful with realtime streams)", "" },
>>      { "window_title",       OPT_TYPE_STRING,          0, { &window_title }, "set window title", "window title" },
>> +    { "scaling_quality",    OPT_TYPE_STRING, OPT_EXPERT, { &scaling_quality }, "set SDL_HINT_RENDER_SCALE_QUALITY value (default=linear)", "value" },
>>      { "left",               OPT_TYPE_INT,    OPT_EXPERT, { &screen_left }, "set the x position for the left of the window", "x pos" },
>>      { "top",                OPT_TYPE_INT,    OPT_EXPERT, { &screen_top }, "set the y position for the top of the window", "y pos" },
>>      { "vf",                 OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_EXPERT, { .func_arg = opt_add_vfilter }, "set video filters", "filter_graph" },
>> @@ -3831,7 +3833,9 @@ int main(int argc, char **argv)
>>              }
>>          }
>>          window = SDL_CreateWindow(program_name, SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, default_width, default_height, flags);
>> -        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "linear");
>> +        if (!scaling_quality)
>> +            scaling_quality = "linear";
>> +        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, scaling_quality);
>>          if (!window) {
>>              av_log(NULL, AV_LOG_FATAL, "Failed to create window: %s", SDL_GetError());
>>              do_exit(NULL);
>> --
>> 2.39.2
>>
>
> Can anyone comment on this? I had a few doubts on this patch:
> - does the option name properly convey its functionality?
> - is the documentation too terse?
> - should we include the accepted values in the documentation, even
> though they are sdl-specific?

What is the benefit of having such option? I don't really see a strong use 
case for it. Also you want to propagate the scaling quality to placebo 
backend as well? Does it acutally make sense to do that?

Thanks,
Marton
Ramiro Polla June 11, 2024, 11:08 a.m. UTC | #3
Hi,

On Mon, Jun 10, 2024 at 9:04 PM Marton Balint <cus@passwd.hu> wrote:
> On Tue, 4 Jun 2024, Ramiro Polla wrote:
> > On Thu, May 30, 2024 at 11:36 PM Ramiro Polla <ramiro.polla@gmail.com> wrote:
> >>
> >> ---
> >>  doc/ffplay.texi  | 2 ++
> >>  fftools/ffplay.c | 6 +++++-
> >>  2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/ffplay.texi b/doc/ffplay.texi
> >> index 93f77eeece..60f883e159 100644
> >> --- a/doc/ffplay.texi
> >> +++ b/doc/ffplay.texi
> >> @@ -72,6 +72,8 @@ as 100.
> >>  Force format.
> >>  @item -window_title @var{title}
> >>  Set window title (default is the input filename).
> >> +@item -scaling_quality @var{value}
> >> +Set SDL_HINT_RENDER_SCALE_QUALITY value (default is "linear").
> >>  @item -left @var{title}
> >>  Set the x position for the left of the window (default is a centered window).
> >>  @item -top @var{title}
> >> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> >> index b9d11eecee..75d2bec777 100644
> >> --- a/fftools/ffplay.c
> >> +++ b/fftools/ffplay.c
> >> @@ -351,6 +351,7 @@ static int filter_nbthreads = 0;
> >>  static int enable_vulkan = 0;
> >>  static char *vulkan_params = NULL;
> >>  static const char *hwaccel = NULL;
> >> +static const char *scaling_quality = NULL;
> >>
> >>  /* current context */
> >>  static int is_full_screen;
> >> @@ -3683,6 +3684,7 @@ static const OptionDef options[] = {
> >>      { "framedrop",          OPT_TYPE_BOOL,   OPT_EXPERT, { &framedrop }, "drop frames when cpu is too slow", "" },
> >>      { "infbuf",             OPT_TYPE_BOOL,   OPT_EXPERT, { &infinite_buffer }, "don't limit the input buffer size (useful with realtime streams)", "" },
> >>      { "window_title",       OPT_TYPE_STRING,          0, { &window_title }, "set window title", "window title" },
> >> +    { "scaling_quality",    OPT_TYPE_STRING, OPT_EXPERT, { &scaling_quality }, "set SDL_HINT_RENDER_SCALE_QUALITY value (default=linear)", "value" },
> >>      { "left",               OPT_TYPE_INT,    OPT_EXPERT, { &screen_left }, "set the x position for the left of the window", "x pos" },
> >>      { "top",                OPT_TYPE_INT,    OPT_EXPERT, { &screen_top }, "set the y position for the top of the window", "y pos" },
> >>      { "vf",                 OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_EXPERT, { .func_arg = opt_add_vfilter }, "set video filters", "filter_graph" },
> >> @@ -3831,7 +3833,9 @@ int main(int argc, char **argv)
> >>              }
> >>          }
> >>          window = SDL_CreateWindow(program_name, SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, default_width, default_height, flags);
> >> -        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "linear");
> >> +        if (!scaling_quality)
> >> +            scaling_quality = "linear";
> >> +        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, scaling_quality);
> >>          if (!window) {
> >>              av_log(NULL, AV_LOG_FATAL, "Failed to create window: %s", SDL_GetError());
> >>              do_exit(NULL);
> >> --
> >> 2.39.2
> >>
> >
> > Can anyone comment on this? I had a few doubts on this patch:
> > - does the option name properly convey its functionality?
> > - is the documentation too terse?
> > - should we include the accepted values in the documentation, even
> > though they are sdl-specific?
>
> What is the benefit of having such option? I don't really see a strong use
> case for it. Also you want to propagate the scaling quality to placebo
> backend as well? Does it acutally make sense to do that?

I use this option to set scaling quality to "nearest" when I want the
display to be pixelated in fullscreen.

I haven't thought about the placebo backend. I'll have a look when I
get the time.
diff mbox series

Patch

diff --git a/doc/ffplay.texi b/doc/ffplay.texi
index 93f77eeece..60f883e159 100644
--- a/doc/ffplay.texi
+++ b/doc/ffplay.texi
@@ -72,6 +72,8 @@  as 100.
 Force format.
 @item -window_title @var{title}
 Set window title (default is the input filename).
+@item -scaling_quality @var{value}
+Set SDL_HINT_RENDER_SCALE_QUALITY value (default is "linear").
 @item -left @var{title}
 Set the x position for the left of the window (default is a centered window).
 @item -top @var{title}
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index b9d11eecee..75d2bec777 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -351,6 +351,7 @@  static int filter_nbthreads = 0;
 static int enable_vulkan = 0;
 static char *vulkan_params = NULL;
 static const char *hwaccel = NULL;
+static const char *scaling_quality = NULL;
 
 /* current context */
 static int is_full_screen;
@@ -3683,6 +3684,7 @@  static const OptionDef options[] = {
     { "framedrop",          OPT_TYPE_BOOL,   OPT_EXPERT, { &framedrop }, "drop frames when cpu is too slow", "" },
     { "infbuf",             OPT_TYPE_BOOL,   OPT_EXPERT, { &infinite_buffer }, "don't limit the input buffer size (useful with realtime streams)", "" },
     { "window_title",       OPT_TYPE_STRING,          0, { &window_title }, "set window title", "window title" },
+    { "scaling_quality",    OPT_TYPE_STRING, OPT_EXPERT, { &scaling_quality }, "set SDL_HINT_RENDER_SCALE_QUALITY value (default=linear)", "value" },
     { "left",               OPT_TYPE_INT,    OPT_EXPERT, { &screen_left }, "set the x position for the left of the window", "x pos" },
     { "top",                OPT_TYPE_INT,    OPT_EXPERT, { &screen_top }, "set the y position for the top of the window", "y pos" },
     { "vf",                 OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_EXPERT, { .func_arg = opt_add_vfilter }, "set video filters", "filter_graph" },
@@ -3831,7 +3833,9 @@  int main(int argc, char **argv)
             }
         }
         window = SDL_CreateWindow(program_name, SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, default_width, default_height, flags);
-        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "linear");
+        if (!scaling_quality)
+            scaling_quality = "linear";
+        SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, scaling_quality);
         if (!window) {
             av_log(NULL, AV_LOG_FATAL, "Failed to create window: %s", SDL_GetError());
             do_exit(NULL);