[libav-devel] [PATCH 3/3] fate: convert codec-regression.sh to makefile rules

Måns Rullgård mans at mansr.com
Fri May 18 16:20:20 CEST 2012


Diego Biurrun <diego at biurrun.de> writes:

> On Fri, May 18, 2012 at 02:57:47PM +0100, Måns Rullgård wrote:
>> Diego Biurrun <diego at biurrun.de> writes:
>> > On Fri, May 18, 2012 at 10:32:06AM +0100, Mans Rullgard wrote:
>> >> --- a/tests/Makefile
>> >> +++ b/tests/Makefile
>> >> @@ -22,7 +19,16 @@ tests/data/asynth1.sw: tests/audiogen$(HOSTEXESUF) | tests/data
>> >>  
>> >> -tests/data/asynth% tests/vsynth%/00.pgm: TAG = GEN
>> >> +tests/data/vsynth1.ref.yuv: tests/videogen$(HOSTEXESUF) | tests/data
>> >> +	$(M)$< >$@
>> >> +
>> >> +tests/data/vsynth2.ref.yuv: tests/rotozoom$(HOSTEXESUF) | tests/data
>> >> +	$(M)$< $(SRC_PATH)/tests/lena.pnm >$@
>> >
>> > Please leave a space after '>', it's easy to confuse it with automatic
>> > variables otherwise.
>> 
>> I prefer it that way.
>
> Give in.  We have spaces around operators in the C code as well.

For redirections we have many more without the space than with it.
Shell script is not C, as you yourself point out.  No space is also
consistent with all examples in the POSIX spec.

>> >> --- a/tests/fate-run.sh
>> >> +++ b/tests/fate-run.sh
>> >> @@ -111,6 +111,34 @@ enc_dec_pcm(){
>> >>  
>> >> +enc_dec(){
>> >> +    src_fmt=$1
>> >> +    srcfile=$2
>> >
>> > src_file would be more consistent here.
>> 
>> But then it wouldn't be nicely aligned, and I thought that was important
>> to you.
>
> Vertical alignment is overrated.

I'll remember you said that.

> It's hard to achieve in shell scripts anyway due to the synctactical
> rule of not having spaces around '='.
>
>> >> +    tsrcfile=$(target_path $srcfile)
>> >> +    tencfile=$(target_path $encfile)
>> >> +    tdecfile=$(target_path $decfile)
>> >
>> > Prefixing the target variables with 't' is a bit terse for my taste
>> 
>> Keeping already long lines somewhat manageable is more important.
>
> No problem:
>
>     avconv -f $src_fmt $DEC_OPTS -i $target_srcfile $ENC_OPTS $enc_opt \
>         $FLAGS -f $enc_fmt -y $target_encfile || return

But this line splitting is much less logical than mine.  Your problem is
that you look more at typographical details than at the larger semantics
of what the text is doing.

-- 
Måns Rullgård
mans at mansr.com


More information about the libav-devel mailing list