Klemens Nanni <k...@openbsd.org> writes:

> On Sun, Dec 19, 2021 at 08:41:01PM +0000, Klemens Nanni wrote:
>> This is the last dependency for our telegram desktop ports.
>> They ship a bundled version which also works but I figured using proper
>> ports where possible is best, so here it is.
>> 
>>      Information for inst:openh264-2.1.1
>> 
>>      Comment:
>>      Cisco implementation of H.264 codec
>> 
>>      Required by:
>>      tg_owt-0.0.0.20211212
>> 
>>      Description:
>>      OpenH264 is a codec library which supports H.264 encoding and decoding.
>>      It is suitable for use in real time applications such as WebRTC.
>> 
>>      Maintainer: Klemens Nanni <k...@openbsd.org>
>> 
>>      WWW: https://github.com/cisco/openh264
>> 
>> Enabling tests and using gtest from ports took a while, but I managed to
>> do so without patching, which seems rather nice.
>> 
>>      [==========] 453 tests from 60 test cases ran. (207344 ms total)
>>      [  PASSED  ] 448 tests.
>>      [  FAILED  ] 5 tests, listed below:
>> 
>> Builds and tests fine on amd64 and sparc64.
>> 
>> The initial port was done with Andrew Krasavin.
>> 
>> Feedback? OK?
>
> New tarball with HOMEPAGE set to https://www.openh264.org/ and
> PATCHFILES adding the commit id into the distname, i.e.
>
> -SIZE (openh264-frame-reorder-fix.patch) = 1151
> +SIZE (openh264-frame-reorder-fix-6cc269be.patch) = 1151


make port-lib-depends-check complains WANTLIB += c for h265enc

With CFLAGS_OPT='${CFLAGS}' the cflags are appended *twice*, i.e.

> c++ -O2 -pipe -O2 -pipe   -DNDEBUG ...

so I'd set CFLAGS_OPT='' instead.

Setting prefix explicitly in MAKE_FLAGS seems redundant too.

> # C++ devel/gtest
> COMPILER = base-clang ports-gcc

I'm not sure what that means.  At glance I can't really tell the minimum
C++ version this needs, but since you've tested on sparc64 (which is
a gcc arch IIRC) I assume it doesn't build with base gcc and thus it's
at least C++11.

pkg/PLIST:
> @so lib/libopenh264.so
> lib/libopenh264.so.0
> @lib lib/libopenh264.so.${LIBopenh264_VERSION}

that doesn't look right, it should install only the last file.  I
haven't found an obvious way to avoid that other than patching out a
couple of lines from the makefile.

I tried it i386 too because it seems to be treated differently in the
build, there are a lot of ifeq $(ARCH), x86 and there are some issue
there:

 - the build fails because it uses nasm.  I tried to use ASM=${CC} but
   then it complains about:

>codec/common/x86/deblock.asm:601:20: error: invalid operand for instruction
>    movd     xmm7, arg4d
>                   ^~~~~

   so I added devel/nasm as BDEP for i386.

 - it enables avx2.  I don't know what's going on, I have an i386 that
   should lack avx/avx2 (it's a pentium T2130) but the port builds and
   the test situation is the same as on amd64: 628 tests pass and 5
   fails.

 - it fails at linking, needs -Wl,-z,notext

For the record, I tried to build it using meson since upstream ships a
meson.build file, but quickly surrendered.  The the meson.build file
needs some serious tweaking because it supports only linux and windows
on i386/amd64 and throws errors otherwise.

I'm attaching a tarball with everything beside the avx2 stuff addressed,
if you agree on the changes it's OK op@ to import.  I'm attaching also a
diff against your makefile for clarity.

Thanks,

Omar Polo


P.S.: there's an extra `getf' file (a diff?) in the tarball, make sure
to remove that before importing :)


--- Makefile.orig       Mon Dec 20 00:02:09 2021
+++ Makefile    Mon Dec 20 00:04:13 2021
@@ -21,15 +21,19 @@
 # BSD 2-clause
 PERMIT_PACKAGE =       Yes
 
-WANTLIB =              ${COMPILER_LIBCXX} m
+WANTLIB = ${COMPILER_LIBCXX} c m
 
-# C++ devel/gtest
+# C++11
 COMPILER =             base-clang ports-gcc
 
+.if ${MACHINE_ARCH:Mi386}
+BUILD_DEPENDS =                devel/nasm
+.endif
+
 TEST_DEPENDS =         devel/gtest
 
 USE_GMAKE =            Yes
-MAKE_FLAGS +=          PREFIX=${PREFIX}
+
 # disable unneeded GMP API for now
 MAKE_FLAGS +=          HAVE_GMP_API=No
 
@@ -40,14 +44,18 @@
 
 # overwrite upstream "-O3" and "-g" defaults, ensure our DEBUG build works
 MAKE_FLAGS +=          CXX=${CXX} \
-                       CFLAGS_OPT='${CFLAGS}' \
+                       CFLAGS_OPT='' \
                        CFLAGS_DEBUG='${DEBUG}'
 
+.if ${MACHINE_ARCH:Mi386}
+MAKE_ENV +=            LDFLAGS=-Wl,-z,notext
+.endif
+
 TEST_FLAGS +=          HAVE_GTEST=Yes
 # find devel/gtest and remove libgtest.a to avoid its build in
 # ${WRKSRC}/build/gtest-targets.mk
-TEST_FLAGS +=  CODEC_UNITTEST_INCLUDES='-I${WRKSRC}/test -I/usr/local/include' 
\
-               CODEC_UNITTEST_LDFLAGS_SUFFIX=-L/usr/local/lib \
+TEST_FLAGS +=  CODEC_UNITTEST_INCLUDES='-I${WRKSRC}/test 
-I${LOCALBASE}/include' \
+               CODEC_UNITTEST_LDFLAGS_SUFFIX=-L${LOCALBASE}/lib \
                CODEC_UNITTEST_DEPS='libdecoder.a libencoder.a libprocessing.a 
libcommon.a'
 
 post-install:

Attachment: openh264.tar.gz
Description: Binary data

Reply via email to