Diego Biurrun <[email protected]> writes:
> On Sun, May 20, 2012 at 11:01:18PM +0100, Mans Rullgard wrote:
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -1,9 +1,5 @@
>> -AREF = fate-acodec-aref
>> -VREF = fate-vsynth1-vref fate-vsynth2-vref
>> -REFS = $(AREF) $(VREF)
>> -
>> -$(VREF): avconv$(EXESUF) tests/vsynth1/00.pgm tests/vsynth2/00.pgm
>> -$(AREF): avconv$(EXESUF) tests/data/asynth1.sw
>> +VREF = tests/vsynth1/00.pgm
>> +AREF = tests/data/asynth1.sw
>
> Dropping "tests/vsynth2/00.pgm" looks suspicous. Suddenly nothing
> depends on that file anymore, but the rule to generate it remains
> in place. Either the rule needs to go as well or something does
> still depend on the file.
The file is indeed not needed, or generated, any more.
>> @@ -22,7 +18,16 @@ tests/data/asynth1.sw: tests/audiogen$(HOSTEXESUF) |
>> tests/data
>>
>> -tests/data/asynth% tests/vsynth%/00.pgm: TAG = GEN
>> +tests/data/asynth% tests/data/vsynth%.yuv tests/vsynth%/00.pgm: TAG = GEN
>
> Will the following simpler but more broad pattern work as well:
>
> tests/data/% tests/vsynth%/00.pgm: TAG = GEN
>
> ?
That's a bit too generic and might match something it's not supposed to.
>> @@ -93,23 +92,14 @@ FATE += $(FATE_LIBAVUTIL)
>>
>> -$(FATE_LAVFI): $(REFS) tools/lavfi-showfiltfmts$(EXESUF)
>> -$(FATE_SEEK): fate-codec fate-lavf libavformat/seek-test$(EXESUF)
>> +$(FATE_LAVFI): $(VREF) tools/lavfi-showfiltfmts$(EXESUF)
>> +$(FATE_SEEK): fate-acodec fate-vsynth2 fate-lavf
>> libavformat/seek-test$(EXESUF)
>
> The previous dependency of FATE_LAVFI on AREF was incorrect?
Yes.
> Same for the previously missing dependency of FATE_SEEK on fate-vsynth2.
It wasn't missing. FATE_SEEK incorrectly depended on both vsynth1 and
vsynth2 via fate-vcodec.
> If so, this could be split out into a separate patch.
Not easily.
>> --- a/tests/fate-run.sh
>> +++ b/tests/fate-run.sh
>> @@ -148,7 +172,7 @@ seektest(){
>>
>> exec 3>&2
>> -$command > "$outfile" 2>$errfile
>> +eval $command >"$outfile" 2>$errfile
>
> You went to the trouble of removing the space after '>' here
> compared to the previous incarnation of the patch? tsk, tsk :)
That inconsistency has been bothering me for a while. Now I fixed it.
>> --- /dev/null
>> +++ b/tests/fate/vcodec.mak
>> --- a/tests/regression-funcs.sh
>> +++ b/tests/regression-funcs.sh
>> @@ -20,17 +20,13 @@ outfile="$datadir/$test_ref/"
>>
>> # various files
>> avconv="$target_exec ${target_path}/avconv"
>> -tiny_psnr="tests/tiny_psnr"
>> raw_src="${target_path}/$raw_src_dir/%02d.pgm"
>> raw_dst="$datadir/$this.out.yuv"
>> -raw_ref="$datadir/$test_ref.ref.yuv"
>> pcm_src="$target_datadir/asynth1.sw"
>> -pcm_dst="$datadir/$this.out.wav"
>> -pcm_ref="$datadir/$test_ref.ref.wav"
>> crcfile="$datadir/$this.crc"
>> target_crcfile="$target_datadir/$this.crc"
>
> Some of these variables are now unused in regression-funcs.sh, but only
> used in lavf-regression.sh (maybe among others, I did not check each one).
> They should be moved there.
All of those files will be removed soon enough. Cleaning them up now
would be wasted effort.
--
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
