3 proposals

There is a bug in default branch that prevents compiling 8bit multilib static library for 10bit+8bit version. Build script:

#!/bin/sh

mkdir -p 8-b 10m

cd 8-b
cmake -G "MSYS Makefiles" -DSTATIC_LINK_CRT=ON ../../../source -DEXPORT_C_API=OFF -DENABLE_SHARED=OFF -DENABLE_CLI=OFF
make
mv libx265.a ../10m/libx265-8b.a

cd ../10m
cmake -G "MSYS Makefiles" -DSTATIC_LINK_CRT=ON -DHIGH_BIT_DEPTH=ON ../../../source -DEXTRA_LIB="x265-8b.a" -DEXTRA_LINK_FLAGS=-L. -DLINKED_8BIT=ON -DENABLE_SHARED=OFF
make
strip x265.exe

cd ..
mv 10m/x265.exe ./x265.exe
rm -rf 8-b 10m

Error when compiling first 8bit part:

[ 49%] Building CXX object common/CMakeFiles/common.dir/x86/asm-primitives.cpp.obj
i:/t4/x265/source/common/x86/asm-primitives.cpp: In function 'void x265_8bit::setupAssemblyPrimitives(x265_8bit::EncoderPrimitiv
es&, int)':
i:/t4/x265/source/common/x86/asm-primitives.cpp:2331:26: error: 'x265_costC1C2Flag_sse2' was not declared in this scope
         p.costC1C2Flag = x265_costC1C2Flag_sse2;
                          ^
make[2]: *** [common/CMakeFiles/common.dir/x86/asm-primitives.cpp.obj] Error 1
make[1]: *** [common/CMakeFiles/common.dir/all] Error 2
make: *** [all] Error 2

So the first proposition is:

diff -r 6563218ce342 source/common/x86/asm-primitives.cpp
--- a/source/common/x86/asm-primitives.cpp  Mon Oct 26 12:13:53 2015 +0530
+++ b/source/common/x86/asm-primitives.cpp  Tue Oct 27 12:38:38 2015 +0100
@@ -2328,7 +2328,7 @@
         p.cu[BLOCK_8x8].idct = PFX(idct8_sse2);

         // TODO: it is passed smoke test, but we need testbench, so temporary disable
-        p.costC1C2Flag = x265_costC1C2Flag_sse2;
+        p.costC1C2Flag = PFX(costC1C2Flag_sse2);
 #endif
         p.idst4x4 = PFX(idst4_sse2);
         p.dst4x4 = PFX(dst4_sse2);

Second problem is different length of x265 file header that depends of build type: there are '[noasm]' and '+10bit+12bit' parts that are on some builds or not. For example 8bit build has shorter file header than 8bit+10bit+12bit multilib build when encoding for 8bit output. So the second proposition is to split 'build_info_str' to 2 parts: old 'build_info_str' without changes (for "x265 -V" command) and new 'build_info_str_fileheader' only for x265 file header:

diff -r 6563218ce342 source/common/primitives.h
--- a/source/common/primitives.h    Mon Oct 26 12:13:53 2015 +0530
+++ b/source/common/primitives.h    Tue Oct 27 12:49:31 2015 +0100
@@ -412,6 +412,7 @@
 extern const int   PFX(max_bit_depth);
 extern const char* PFX(version_str);
 extern const char* PFX(build_info_str);
+extern const char* PFX(build_info_str_fileheader);
 #endif

 #endif // ifndef X265_PRIMITIVES_H
diff -r 6563218ce342 source/common/version.cpp
--- a/source/common/version.cpp Mon Oct 26 12:13:53 2015 +0530
+++ b/source/common/version.cpp Tue Oct 27 12:49:31 2015 +0100
@@ -130,3 +130,4 @@

 const char* PFX(version_str) = XSTR(X265_VERSION);
 const char* PFX(build_info_str) = ONOS COMPILEDBY BITS ASM ATOMICS CHECKED BITDEPTH ADD8 ADD10 ADD12;
+const char* PFX(build_info_str_fileheader) = ONOS COMPILEDBY BITS ATOMICS CHECKED BITDEPTH;
diff -r 6563218ce342 source/encoder/encoder.cpp
--- a/source/encoder/encoder.cpp    Mon Oct 26 12:13:53 2015 +0530
+++ b/source/encoder/encoder.cpp    Tue Oct 27 12:49:31 2015 +0100
@@ -1379,13 +1379,13 @@
         if (opts)
         {
             char *buffer = X265_MALLOC(char, strlen(opts) + strlen(PFX(version_str)) +
-                                             strlen(PFX(build_info_str)) + 200);
+                                             strlen(PFX(build_info_str_fileheader)) + 200);
             if (buffer)
             {
                 sprintf(buffer, "x265 (build %d) - %s:%s - H.265/HEVC codec - "
                         "Copyright 2013-2015 (c) Multicoreware Inc - "
                         "http://x265.org - options: %s",
-                        X265_BUILD, PFX(version_str), PFX(build_info_str), opts);
+                        X265_BUILD, PFX(version_str), PFX(build_info_str_fileheader), opts);

                 bs.resetBits();
                 SEIuserDataUnregistered idsei;
diff -r 6563218ce342 source/x265.h
--- a/source/x265.h Mon Oct 26 12:13:53 2015 +0530
+++ b/source/x265.h Tue Oct 27 12:49:31 2015 +0100
@@ -1315,6 +1315,7 @@
 /* x265_build_info:
  *      A static string describing the compiler and target architecture */
 X265_API extern const char *x265_build_info_str;
+X265_API extern const char *x265_build_info_str_fileheader;

 /* Force a link error in the case of linking against an incompatible API version.
  * Glue #defines exist to force correct macro expansion; the final output of the macro

With this split the output file after encoding has more consistent length and we can compare output from different builds without problems.

Third proposal is to switch to SSE2 level for GCC 32-bit builds. MSVC 14 default compile arch for 32-bit is SSE2. Now 32-bit GCC build of x265 uses "-march=i686" option. We could switch to "-march=pentium4 -mtune=generic" and if user exports CXXFLAGS "-march" or "-mtune" option, we should use what user wants (for example export CXXFLAGS="-march=i686" should build for i686 arch):

diff -r 6563218ce342 source/CMakeLists.txt
--- a/source/CMakeLists.txt Mon Oct 26 12:13:53 2015 +0530
+++ b/source/CMakeLists.txt Tue Oct 27 12:51:39 2015 +0100
@@ -173,7 +173,14 @@
             add_definitions(-march=native)
         endif()
     elseif(X86 AND NOT X64)
-        add_definitions(-march=i686)
+        string(FIND "${CMAKE_CXX_FLAGS}" "-march" marchPos)
+        if(marchPos LESS "0")
+            add_definitions(-march=pentium4)
+            string(FIND "${CMAKE_CXX_FLAGS}" "-mtune" mtunePos)
+            if(mtunePos LESS "0")
+                add_definitions(-mtune=generic)
+            endif()
+        endif()
     endif()
     if(ARM)
         add_definitions(-march=armv6 -mfloat-abi=hard -mfpu=vfp)

Comments (2)