[x265] [PATCH] Add Regression Test

[Steve Borho]

On Fri, Jul 19, 2013 at 9:45 PM, <mahesh at multicorewareinc.com> wrote:

> # HG changeset patch
> # User maheshpittala
> # Date 1374288315 25200
> # Node ID 8f0e95ff94ee27772301ae6a4aeb047a6fa76a00
> # Parent  bcc2539665c8705117eb64c195381ecdcdd51394
> Add Regression Test
>
> diff -r bcc2539665c8 -r 8f0e95ff94ee source/BuildandRun.bat
> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/BuildandRun.bat    Fri Jul 19 19:45:15 2013 -0700
> @@ -0,0 +1,73 @@
>

this is still under source/, it should go under build/regression


> + at echo off
> +
> +cd ..\"%buildconfig%"
> +call:makesolution 8bit "ENABLE_PPA:BOOL=ON"
> +call:makesolution 16bit "HIGH_BIT_DEPTH:BOOL=ON"
>

After this last call to makesolution it needs to jump past the routine or
exit the script


> +
> +
> +:makesolution
> +
> +set build_folder=%~1
> +set cmake_option=%~2
> +if exist %build_folder% rd /s /q %build_folder%
> +mkdir %build_folder%
> +cd %build_folder%
> +
> +cmake -D %cmake_option% -G "%generator%" ..\..\..\source
>

this needs to enable tests


> +
> +if exist %solution% (
> +
> + call %vcvars%
> + MSBuild /property:Configuration="Release" x265.sln >>
> buildlog_release.txt
> +
> +  if %errorlevel% equ 1 (
> +   echo Release_mode:Build failed with %build_folder% for %generator%
> refer the build log  >> "%LOG%"
> +   exit 1
> +   )
>

indention should go back to the first line after this close brace


> +   echo Release_mode:Build successfull with %build_folder% for
> %generator% >> "%LOG%"
>

successful has one l

I prefer not to log about successes.  Success does not need to be logged.

+
> +  if exist Release\x265.exe (
> +   echo Release_mode:build Application with %build_folder% successfull
> for %generator% >> "%LOG%"
> +   ) else (
> +   echo Release_mode:build Application with %build_folder% failed for
> %generator%  refer the build log >> "%LOG%"
> +   )
> +
> +
> +
> + MSBuild /property:Configuration="Debug" x265.sln >> buildlog_debug.txt
> +
> +  if %errorlevel% equ 1 (
> +   echo Debug_mode:Build failed with %build_folder% for %generator% refer
> the build log  >> "%LOG%"
> +   exit 1
> +   )
> +   echo Debug_mode:Build with %build_folder% successfull for %generator%
> >> "%LOG%"
> +
> +  if exist Debug\x265.exe (
> +   echo Debug_mode:build Application with %build_folder% successfull for
> %generator% >> "%LOG%"
> +   ) else (
> +   echo Debug_mode:build Application with %build_folder% failed for
> %generator%  refer the build log >> "%LOG%"
>

These messages are too verbose.  It could simply be:

   echo %generator% %build_folder% debug build failed, see build log >>
%LOG%


> +   )
> +) else (
> +echo %build_folder% solution is not created for %generator% >> "%LOG%"
> +)
> +
> +
> +if exist Release\x265.exe (
> + Release\x265.exe  %video1%.y4m -f %frames% --hash 1 -o str.out -r
> rec.yuv >> %build_folder%.txt 2>&1
> + Release\x265.exe  %video2%.y4m -f %frames% --hash 1 -o str1.out -r
> rec1.yuv >> %build_folder%.txt 2>&1
> + Release\x265.exe  %video3%.y4m -f %frames% --hash 1 -o str2.out -r
> rec2.yuv >> %build_folder%.txt 2>&1
> + %decoder% -b str.out -o str.yuv >>
> "%workingdir%"\decoder_out_%build_folder%.txt
> + %decoder% -b str1.out -o str1.yuv >>
> "%workingdir%"\decoder_out_%build_folder%.txt
> + %decoder% -b str2.out -o str2.yuv >>
> "%workingdir%"\decoder_out_%build_folder%.txt
> + FC  rec.yuv str.yuv >> DiffBin.txt
> + FC  rec1.yuv str1.yuv >> DiffBin.txt
> + FC  rec2.yuv str2.yuv >> DiffBin.txt
>

Since the videos are numbered 1..3, the streams should be numbered 1..3

fc /b is recommended to force binary diff mode.  We do not really need to
keep the actual diffs, we just need to know whether the files are
identical.  The || operator will do this nicely:

fc /b rec.yuv str.yuv > NUL || echo Reconstructed frame mismatch
for %video1% >> %LOG%


> +)
> +
> +if exist test\Release\TestBench.exe (
> + echo %CD% >> "%workingdir%"\TestLog.txt
> + test\Release\TestBench.exe >> "%workingdir%"\TestLog.txt
>

The return code of the testbench should be checked


> +)
> + cd ..\
> diff -r bcc2539665c8 -r 8f0e95ff94ee source/Main.bat
> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/Main.bat   Fri Jul 19 19:45:15 2013 -0700
> @@ -0,0 +1,44 @@
> + at echo off
> +
> +for /f "tokens=1,2,3,4,5,6,7,8 delims==" %%a in (config.txt) do (
> +if %%a==repository set repository=%%b
> +if %%a==FramesToCheck_BinMismatch set frames=%%b
> +if %%a==FramesToRun_PSNRperformance set frames1=%%b
> +if %%a==decoder set decoder=%%b
> +if %%a==drpsnr set drpsnr=%%b
> +if %%a==video1 set video1=%%b
> +if %%a==video2 set video2=%%b
> +if %%a==video3 set video3=%%b
> +)
> +
> +set "save_path=%path%"
> +set "save_include=%INCLUDE%"
> +set "save_lib=%LIB%"
> +set "save_libpath=%LIBPATH%"
> +set solution="x265.sln"
> +set HG=hg
> +set workingdir=%CD%
> +
> +if exist "%workingdir%"\RegressionTester.log del
> "%workingdir%"\RegressionTester.log
> +set LOG="%workingdir%"\RegressionTester.log
>

simpler is better, just regression.log


> +if %errorlevel% equ 1 (
> + echo Working directory not created >> "%LOG%"
> + exit 1
> +)
> +echo Working directory created >> "%LOG%"
> +
> +"%HG%" pull "%repository%"
>

hg pull -u %repository%

This combines pull and update


> +"%HG%" update  >> "%LOG%"
> +"%HG%" summary >> "%LOG%"
> +
> +if %errorlevel% equ 1 (
> + echo Pull unsuccessfull >> "%LOG%"
>

"pull failed"


> + exit 1
> +)
> +echo Pull successfull >> "%LOG%"
>

remove this message


> +
> +call compilers.bat
> +call PSNR_Performance.bat
> +start python performance_csv.py
> +start python Send_Mail.py
> diff -r bcc2539665c8 -r 8f0e95ff94ee source/compilers.bat
> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/compilers.bat      Fri Jul 19 19:45:15 2013 -0700
> @@ -0,0 +1,28 @@
> + at echo off
> +
> +call:buildconfigs "%VS110COMNTOOLS%" vc11-x86_64 "Visual Studio 11 Win64"
> +call:buildconfigs "%VS110COMNTOOLS%" vc11-x86 "Visual Studio 11"
> +call:buildconfigs "%VS100COMNTOOLS%" vc10-x86_64 "Visual Studio 10 Win64"
> +call:buildconfigs "%VS100COMNTOOLS%" vc10-x86 "Visual Studio 10"
> +call:buildconfigs "%VS90COMNTOOLS%" vc9-x86_64 "Visual Studio 9 2008
> Win64"
> +call:buildconfigs "%VS90COMNTOOLS%" vc9-x86 "Visual Studio 9 2008 Win64"
>

again, it needs to exit here or it will call buildconfigs again


> +
> +:buildconfigs
> +
> +set compiler=%~1
> +set buildconfig=%~2
> +set generator=%~3
> +
> +echo running %generator% >> "%LOG%"
> +set vcvars="%compiler%\..\..\VC\vcvarsall.bat"
> +call BuildandRun.bat
>

these lines below are unnecessary


> +set path=""
> +set INCLUDE=""
> +set LIB=""
> +set LIBPATH=""
> +set "path=%save_path%"
> +set "INCLUDE=%save_include%"
> +set "LIB=%save_lib%"
> +set "LIBPATH=%save_libpath%"
>

You cannot check in a file that must be edited by the user in normal use.
 This
makes their repository always have a dirty file in it, causing it to get
accidentally
checked in multiple times a day.

What you have to do is check this file in as "config.example", and the
script
which requires config.txt should test if config.txt does not exist and tell
the
user to copy config.example to config.txt and edit it as necessary.

You also must add config.txt to the Mercurial ignore filter (.hgignore) so
it is never
checked in.


> diff -r bcc2539665c8 -r 8f0e95ff94ee source/config.txt
> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/config.txt Fri Jul 19 19:45:15 2013 -0700
> @@ -0,0 +1,8 @@
> +repository=https://maheshpittala@bitbucket.org/multicoreware/xhevc
> +FramesToCheck_BinMismatch=3
> +FramesToRun_PSNRperformance=30
> +decoder=D:\\TAppDecoder.exe
> +drpsnr=D:\\dr_psnr.exe
> +video1=D:\\BasketballDrive_1920x1080_50
> +video2=D:\\Kimono1_1920x1080_24
> +video3=D:\\FourPeople_1280x720_60
>

If comments are allowed in this file, you should add some explaining what
these fields are used for.

-- 
Steve Borho
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/private/x265-devel/attachments/20130719/bb9a990e/attachment.html>