[FFmpeg-devel] [PATCH] fate: use the lavfi -graph_file option to workaround path issues.

[Nicolas George]

Le tridi 3 brumaire, an CCXXI, Clément Bœsch a écrit :
> ---
> FATE is still broken due to various path issues. This is an attempt to fix it
> using the new introduced -graph_file option. I have no mean to test it with
> mingw/msys/msvc stuff so I have no idea if it works as expected.
> 
> Here are the two issues supposed to be fixed:
> 
>   - the source file is passed as a standalone argument to printf, and I just
>     hope here msys will do its work and replace paths such as "/d/foo/bar" into
>     "d:/foo/bar". This is a problem currently happening with the scene
>     detection because the filename is bogged down into the middle of the
>     filtergraph and isn't substitute.
> 
>   - in the command line, when you do "movie='d:/foo/bar':foobar=...", the ''
>     are dropped in a first pass, so the filter receives
>     "movie=d:/foo/bar:foobar=..." thus breaks. This is a problem currently
>     happening with the silence detection. It shouldn't happen with a
>     filtergraph in a file since it is opened within lavfi.

It looks a bit fragile. I do not know what Stefano intended when
implementing this option, but I suspect it was not the typical use case.

As far as I understand the situation, -graph_file is meant to deal with
problems around the shell escaping rules, while the paths that break FATE
are problems with regard to ffmpeg's escaping rules.

In other words, if you can prepare a file with paths that will work, you can
prepare a string in a shell variable that will work just the same.

I suspect that the reason your patch helps is not that you are using the
-graph_file option but rather that you are moving all strings manipulations
from makefile (very crappy language with a lot of unavoidable limitations)
to the shell (slightly less crappy language with a lot less limitations).

To deal with filenames in filter graphs, I would suggest to implement basic
${...} environment variables expansion after splitting. If this is done,
then you can write just 'amovie=${IN_FILE}' and make sure that the IN_FILE
environment variable is defined.

> ---
>  tests/fate-run.sh     | 19 +++++++++++++++++++
>  tests/fate/filter.mak |  6 ++----
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/fate-run.sh b/tests/fate-run.sh
> index ccd04a3..75bc1b7 100755
> --- a/tests/fate-run.sh
> +++ b/tests/fate-run.sh
> @@ -104,6 +104,25 @@ pcm(){
>      ffmpeg "$@" -vn -f s16le -
>  }
>  
> +metadata(){
> +    filter_type=$1
> +    name=$2
> +    src=$3
> +    filter=$4
> +
> +    graph_file=tests/data/$name.lavfi
> +    if [ $filter_type = audio ]; then
> +        printf "amovie='" > $graph_file
> +    else
> +        printf "sws_flags=+accurate_rnd+bitexact; movie='" > $graph_file
> +    fi
> +
> +    # two different commands to make sure the path is substituted by msys
> +    printf $src        >> $graph_file

This will break if $src contains shell meta-characters, because since it is
unquoted, it will be re-expanded. When programming in shell, you are
expected to double-quote almost any variable expansion.

This will also break if $src contains spaces, because then only its first
component will be taken as the printf format string, the rest will be
considered arguments for inexistent conversion specifications.
Double-quoting $src will also fix this.

This will also break if $src contains printf meta-characters: % and \, this
is the basic equivalent of a format-string vulnerability. It can be solved
by writing it like this: printf '%s' "$src"

This will also break if $src contains a single quote (and my guess is that
there is at least one localization of windows where "My Documents" is
spelled with an apostrophe). It could be solved using Stefano's ffescape,
but this is rather ugly. Thus my suggestion of using environment variables.

The idea is the same as if you want to produce a shell command to process
"frobnicate < input > output" where input and output can contain special
characters: you should not be trying to escape input and output, instead you
write:

execl("/bin/sh", "sh", "-c", "frobnicate < \"$1\" > \"$2\"", "sh",
      input, output, NULL);

and let the shell do its mojo.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121025/b4eef35e/attachment.asc>