[FFmpeg-devel] [RFC][PATCH] Tool to sort files a bit

[Stefano Sabatini]

On date Saturday 2010-10-09 19:52:52 +0200, Michael Niedermayer encoded:
> On Sat, Oct 09, 2010 at 11:23:30AM +0200, Stefano Sabatini wrote:
> > On date Saturday 2010-10-09 00:12:17 +0200, Michael Niedermayer encoded:
> > > Hi
> > > 
> > > The attached script creates directories and symlinks for multimedia files
> > > based on container and codec in the file. I hope ive not done this work
> > > redundantly
> > > it needs to be checked by someone who knows shell escaping to make it
> > > secure ive made no attempt at making it secure.
> > > 
> > > Also if people are ok with it id like to commit this so more people can work on
> > > it, iam not an expert on shell programming ...
> > > 
> > > yes its intended to help with incoming / samples maybe at some point or even
> > > to help people sort their personal file collections
> > > 
> > > 
> > > commit ca9a964c2eba0ba6cda767cfe2d74b80c245dd4c
> > > Author: Michael Niedermayer <michaelni at gmx.at>
> > > Date:   Fri Oct 8 23:58:51 2010 +0200
> > > 
> > >     Tool to analyze multimedia files and create directories and symlinks for the
> > >     container type and codecs in each file that point back to the file.
> > > 
> > > diff --git a/tools/jauche_sortierer.sh b/tools/jauche_sortierer.sh
> > > new file mode 100755
> > > index 0000000..517353f
> > > --- /dev/null
> > > +++ b/tools/jauche_sortierer.sh
> > > @@ -0,0 +1,19 @@
> > > +#!/bin/sh
> > > +#GPL
> > > +#TODO
> > > +#add pixelformat/sampleformat into the path of the codecs
> > > +
> > > +FFP=../ffprobe
> > > +TMP=./misthaufen
> > 
> > The name of the script and of $TMP are weird, as already noted...
> 
> I like the name of the script.

It looks like French, but I won't argue with that, if it makes you
feel happier that's allright...

> > Also $TMP should contain at least the pid, in order to avoid conflicts
> > with other concurrent instances of the script, so I suggest:
> > TMP=$TMPDIR/$(basename $0)-$$
> 
> fixed differently, your variant is insecure
> 
> 
> > 
> > > +TARGET=$1
> > > +shift
> > > +
> > > +for v do
> > 
> > what is v?
> 
> I probably dont understand your question
> its a variable

I mean... how it is passed to the script (is the user supposed to set
v=FILE1 FILE2 ...) or what?

> > > +    BASE=`basename $v`
> > > +    echo $v | egrep -i '(public|private)' >/dev/null && echo Warning $v may be private
> > > +    $FFP $v 2> $TMP
> > > +    FORM=`(grep 'Input #0, ' -m1 $TMP || echo 'Input #0, unknown') | sed 's/Input #0, \([a-zA-Z0-9_]*\).*/\1/' `
> > 
> > Since we're using ffprobe the parsing can be simplified a lot (and
> > made more robust), using ffprobe -show_streams -show_format, also it
> > should make trivial to add support for other features like pixfmts,
> > samplefmts, codec tags.
> 
> robuster yes, simpler iam not so sure
> but suggestion or better patches are welcome if you know how to do it simpler

uhm... I suppose perl is not acceptable?

> > Also I tend to prefer the $(...) syntax over `...`, as it is generally
> > more readable (and my editor allows to match opening and closing
> > paren).
> 
> changed
> 
> 
> > 
> > > +    mkdir -p $TARGET/container/$FORM
> > > +    ln -s $v $TARGET/container/$FORM/$BASE
> > > +    eval `grep 'Stream #0\.[^:]*: [a-zA-Z0-9][^:]*: [a-zA-Z0-9]' $TMP | sed 's#[^:]*: \([a-zA-Z0-9]*\)[^:]*: \([a-zA-Z0-9]*\).*#mkdir -p '$TARGET'/\1/\2 ; ln -s '$v' '$TARGET'/\1/\2/'$BASE' ; #'`
> > > +done
> > 
> > Missing rm $TMP at the end.
> 
> fixed

[...]

I have no other comments on the patch.
-- 
FFmpeg = Freak and Fierce Mortal Powered Evanescent Gymnast