Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--vf=scale isn't supported after ffmpeg 95568c4 #14047

Closed
zhongfly opened this issue May 3, 2024 · 7 comments
Closed

--vf=scale isn't supported after ffmpeg 95568c4 #14047

zhongfly opened this issue May 3, 2024 · 7 comments
Labels

Comments

@zhongfly
Copy link

zhongfly commented May 3, 2024

Important Information

Provide following Information:

mpv and ffmpeg version:

mpv v0.38.0-99-gd61d2946 Copyright © 2000-2024 mpv/MPlayer/mplayer2 projects
 built on May  2 2024 12:10:35
libplacebo version: v7.349.0 (v6.338.0-129-g47ea18e-dirty)
FFmpeg version: N-115056-g95568c4e3
FFmpeg library versions:
   libavutil       59.16.101
   libavcodec      61.5.103
   libavformat     61.3.103
   libswscale      8.2.100
   libavfilter     10.2.101
   libswresample   5.2.100

Reproduction steps

run mpv --vf=scale=help

mpv with FFmpeg/FFmpeg@69b4d97 is good ,but FFmpeg/FFmpeg@95568c4 broken.So some commits between 69b4d97 and 95568c4 broken --vf=scale option

Expected behavior

Get help info about --vf=scale,like:

Options:

 w          <string>     Output video width
 h          <string>     Output video height
 flags      <string>     Flags to pass to libswscale
 interl     <bool>       set interlacing
 size       <string>     set video size
 in_color_matrix <int>        set input YCbCr type
 out_color_matrix <int>        set output YCbCr type
......

Actual behavior

Option vf: 'scale' isn't supported.
Error parsing option vf (option parameter could not be parsed)
Setting commandline option --vf=scale=help failed.
Exiting... (Fatal error)
@kasper93
Copy link
Contributor

kasper93 commented May 3, 2024

@haasn

@sfan5
Copy link
Member

sfan5 commented May 3, 2024

it's FFmpeg/FFmpeg@bb80445
mpv rejects filters with more than one in-/output for use with --vf:

mpv/filters/f_lavfi.c

Lines 980 to 997 in a26bbbd

// Does it have exactly one video input and one video output?
static bool is_usable(const AVFilter *filter, int media_type)
{
#if LIBAVFILTER_VERSION_INT >= AV_VERSION_INT(8, 3, 0)
int nb_inputs = avfilter_filter_pad_count(filter, 0),
nb_outputs = avfilter_filter_pad_count(filter, 1);
#else
int nb_inputs = avfilter_pad_count(filter->inputs),
nb_outputs = avfilter_pad_count(filter->outputs);
#endif
bool input_ok = filter->flags & AVFILTER_FLAG_DYNAMIC_INPUTS;
bool output_ok = filter->flags & AVFILTER_FLAG_DYNAMIC_OUTPUTS;
if (nb_inputs == 1)
input_ok = avfilter_pad_get_type(filter->inputs, 0) == media_type;
if (nb_outputs == 1)
output_ok = avfilter_pad_get_type(filter->outputs, 0) == media_type;
return input_ok && output_ok;
}

@haasn
Copy link
Member

haasn commented May 3, 2024

Does this fix it?

commit ce996d2d80a6d184856c9d3a869a10e8f1414e9b
Author: Niklas Haas <git@haasn.dev>
Date:   Fri May 3 22:07:30 2024 +0200

    avfilter/vf_scale: add missing filter flag
    
    Fixes: bb8044581366fe286e16b14515d873979133dbda

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index bc53571c1c..696ee272ec 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -1261,6 +1261,7 @@ const AVFilter ff_vf_scale = {
     FILTER_QUERY_FUNC(query_formats),
     .activate        = activate,
     .process_command = process_command,
+    .flags           = AVFILTER_FLAG_DYNAMIC_INPUTS,
 };
 
 static const AVFilterPad avfilter_vf_scale2ref_inputs[] = {

@sfan5
Copy link
Member

sfan5 commented May 3, 2024

I bet it does but isn't that misusing the flag?

/**
 * The number of the filter inputs is not determined just by AVFilter.inputs.
 * The filter might add additional inputs during initialization depending on the
 * options supplied to it.
 */

I suppose scale could be changed to dynamically add the ref input if any of the relevant vars are used.

Edit: Based on the ffmpeg documentation I noticed we're not actually checking the pad count correctly and proposed #14052. Incidentally this would make your patch no longer work.

@sfan5 sfan5 added the filter label May 3, 2024
@haasn
Copy link
Member

haasn commented May 3, 2024

The pad count serves as a minimum even if the filter has dynamic inputs/outputs, so we should reject them here.

Huh, then indeed vf_scale would need to be fixed to only add the second input dynamically if it's being used. I'm not sure if I read the same into that documentation as you, though. You think that implies it can only ever strictly add inputs, rather than also removing them?

@sfan5
Copy link
Member

sfan5 commented May 3, 2024

Yes, that is my interpretation.

@zhongfly
Copy link
Author

zhongfly commented May 4, 2024

Fixed by FFmpeg/FFmpeg@6a5b021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants