[MPlayer-cvslog] CVS: main/TOOLS psnr-video.sh, NONE, 1.1 README, 1.9, 1.10

[Ivo]

On Monday 12 September 2005 18:36, Guillaume Poirier CVS wrote:
> Date: Aug 25, 2005 1:54 PM
> Subject: [MEncoder-users] [Script] PSNR between two video files

At first I thought this new CVS rule was a bit pointless, but after I 
searched my ML archive and couldn't find this as a patch, I noticed this 
line :)  I wasn't subscribed to mencoder-users, so that's why I missed it.

I have a few comments. It's maybe not that important since it's only a tool, 
but here they are anyway :)

> --- NEW FILE ---
> #!/bin/sh
> # Helper script to ease comparing two video files
> # Copyleft 2001 by Matthias Wieser

2001? Are you sure that's correct? It uses -vo pnm and that was added only 
about a year ago.

> TEMPDIR="/tmp/psnr_video"
> WORKDIR=`pwd`/

I'm not that fond of adding the trailing / to the variable. It gets 
confusing later on if a filename is added.

> mkdir -p ${TEMPDIR}/FILE1
> mkdir -p ${TEMPDIR}/FILE2

-vo pnm support writing to directories. Maybe that can be used?

> if [[ `ls -1 ${TEMPDIR}/FILE1/*ppm|wc -l` = `ls -1 
${TEMPDIR}/FILE2/*ppm|wc -l` ]] then

I'm not sure I like wc -l. What about tail -n 1?

>         pnmpsnr ../FILE1/$FILE $FILE 2> del.del
>         grep "Y" del.del | dd bs=1c count=5 skip=29 of=del2.del
> 2>/dev/null Y=`cat del2.del`
>                echo -n "$Y;">>../psnr.dat
>         grep "Cb" del.del | dd bs=1c count=5 skip=29 of=del2.del
> 2>/dev/null CB=`cat del2.del`
>                echo -n "$CB;">>../psnr.dat
>         grep "Cr" del.del | dd bs=1c count=5 skip=29 of=del2.del
> 2>/dev/null CR=`cat del2.del`
>                echo -n "$CR;">>../psnr.dat

I don't like the usage of dd here. Can you be sure it's always 5 characters?
The above could be replaced by something like:

YCBCR=`pnmpsnr ../FILE1/$FILE $FILE 2>&1 | tail -n 3 | cut -f 3 -d ':' | \
      ( read Y X; read CB X; read CR X; echo "$Y;$CB;$CR;")`
Y=`echo $YCBCR | cut -f 1 -d ';'`
CB=`echo $YCBCR | cut -f 2 -d ';'`
CR=`echo $YCBCR | cut -f 3 -d ';'`
echo $YCBCR >>../psnr.dat

>          ALL=`echo
> "(-10)*l((e(-$Y/10*l(10))+e(-$CB/10*l(10))/4+e(-$CR/10*l(10))/4)/1.5)/l(1
>0)"|bc -l` echo "$ALL">>../psnr.dat

I don't have bc on my system, so this will fail. Can't this be done with 
expr (emulate fixed point arithmetic) or maybe a small C program?

> if [[ `ls -1 ${TEMPDIR}/FILE1/*ppm|wc -l` = `ls -1
> ${TEMPDIR}/FILE2/*ppm|wc -l` ]] then
>         echo
> else
>         echo "Files have differing numbers of frames!"
>         echo "$FILE1 has `ls -1 ${TEMPDIR}/FILE1/*ppm|wc -l` frames,"
>         echo "$FILE2 has `ls -1 ${TEMPDIR}/FILE2/*ppm|wc -l` frames."
>         echo "Processed the first `ls -1 ${TEMPDIR}/FILE2/*ppm|wc -l`
> frames." echo
> fi

Code duplication.

> +              Be aware that psnr-video.sh needs a lot of temporal space
> +              inside /temp/.

I'm no English grammar and writing guru myself, so I leave the docs to those 
who are, but temporal should be temporary imho.

--Ivo