On Mon, Dec 20, 2021 at 12:05:11AM +0100, Omar Polo wrote: > 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
Thanks, I added the programs after creating wantlib.
>
> With CFLAGS_OPT='${CFLAGS}' the cflags are appended *twice*, i.e.
>
> > c++ -O2 -pipe -O2 -pipe -DNDEBUG ...
>
> so I'd set CFLAGS_OPT='' instead.
Yeah, doesn't hurt but ugly. I've confirmed that we can just set empty
both CFLAGS_OPT and CFLAGS_DEBUG and have our CFLAGS value take care of
the rest.
> Setting prefix explicitly in MAKE_FLAGS seems redundant too.
The port defaults to /usr/local, I'm happy to rely on that.
>
> > # 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.
`make build' works on sparc64 with gcc 4.2.1 but `make test' builds
against the gtest c++ headers and fails, hence the need for a newer
compiler, but only due to tests.
I tried clarifying that.
>
> 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.
Yes, most ports ship a single file but we do have ports that contain
symlinks, so I did not outright reject it.
I tried improving it but did get anywhere and the ports using openh264
do work, so...
>
> 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
Thanks, I squashed your i386 specific code into one block and made it
amend our LDFLAGS which is now unconditionally passed to the port.
>
> 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 never tried that, the Makefile was good enough and has less
dependencies.
>
> I'm attaching a tarball with everything beside the avx2 stuff addressed,
No idea about i386, really; I wouldn't treat it as a blocker, either.
> if you agree on the changes it's OK op@ to import. I'm attaching also a
> diff against your makefile for clarity.
Thanks. I have left out your LOCALBASE addition; using the default
PREFIX=/usr/local but keeping the include path modular makes no sense.
It depends on how gtest was built anyway... this PREFIX/LOCALBASE thing
is nothing but handwaving, so I'd like to hardcode things in new ports
(of mine), which seemed to be the rough consensus of porters anyway.
>
> Thanks,
>
> Omar Polo
>
>
> P.S.: there's an extra `getf' file (a diff?) in the tarball, make sure
> to remove that before importing :)
Done.
Feedback? OK?
I'll do an arm64 build soon.
openh264.tgz
Description: Binary data
