roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

Freebsd #284

Closed strfry closed 4 years ago

strfry commented 4 years ago

These are some necessary changes to make it work on FreeBSD. I added an additional target that is explicitly considered at several points. Just hope it won"t get too messy with more target OS's.

Note that despite the unit tests finishing without errors, this is little tested besides a quick try with roc-send.

gavv commented 4 years ago

Cool!

I added an additional target that is explicitly considered at several points. Just hope it won"t get too messy with more target OS's.

I think this is OK.

gavv commented 4 years ago

There are also other places in SConstruct where we could add freebsd.

        if platform in ['linux'] and not GetOption('disable_pulseaudio'):
            env.Append(ROC_TARGETS=[
                'target_pulseaudio',
            ])
if ((not GetOption('disable_tools') \
        or not GetOption('disable_examples')) \
    and not GetOption('disable_pulseaudio')) \
  or GetOption('enable_pulseaudio_modules'):
    if platform in ['linux', 'android']:
        all_dependencies.add('alsa')
        all_dependencies.add('pulseaudio')

Since, AFAIK, pulseaudio is available for FreeBSD, I think we should support it. But since it's rarely used there, I guess, we should disable pulseaudio by default on FreeBSD (but not on Linux). We could add --enable-pulseaudio option to enable it.

    if platform in ['linux', 'darwin']:
        env.Append(LIBS=[
            'pthread',
        ])

This adds -pthread (without -l). Don't we need it on FreeBSD as well?

    if platform in ['linux', 'android']:
        test_env['RPATH'] = test_env.Literal('\\$$ORIGIN')

        lib_env['SHLIBSUFFIX'] = '%s.%s' % (lib_env['SHLIBSUFFIX'], abi_version)
        lib_env.Append(LINKFLAGS=[
            '-Wl,-soname,libroc%s' % lib_env['SHLIBSUFFIX'],
        ])

        if variant == 'release':
            lib_env.Append(LINKFLAGS=[
                '-Wl,--version-script=%s' % env.File('#src/lib/roc.version').path
            ])

This adds soname and hides some symbols from the shared library. I think we can enable this on FreeBSD too?

    if platform in ['linux', 'android']:
        if compiler_ver[:2] >= (6, 0):
            for var in ['CXXFLAGS', 'CFLAGS']:
                env.Append(**{var: [
                    '-Wno-redundant-parens',
                ]})

    if platform in ['linux']:
        if compiler_ver[:2] >= (8, 0):
            for var in ['CXXFLAGS', 'CFLAGS']:
                env.Append(**{var: [
                    '-Wno-extra-semi-stmt',
                    '-Wno-atomic-implicit-seq-cst',
                ]})
    if platform in ['linux', 'android']:
        env.Append(LINKFLAGS=[
            '-Wl,--no-undefined',
        ])
if platform in ['linux']:
    tool_env.Append(LINKFLAGS=[
        '-Wl,-rpath-link,%s' % env.Dir('#3rdparty/%s/%s/rpath' % (
            host, thirdparty_compiler_spec)).abspath,
    ])

As well as this?

gavv commented 4 years ago

BTW did you try to build Roc with different options? Debug/release, enable/disable sanitizers, enable -Werror, enable/disable different dependencies? I'm not saying we need to test all these combinations right now, but this would be a long-term goal to add FreeBSD to the list of officially supported platforms.

Also, we'll need (again, we can do it later) to add FreeBSD to CI. We could use qemu in travis or maybe use Cirrus CI (I don't know much about them, needs research). It would be preferable to use a single CI for all build though. Are you aware of any other options?

gavv commented 4 years ago

Finally, could you please re-target the PR to develop branch? See here: https://roc-project.github.io/roc/docs/development/version_control.html

(If github don't allow you to do this, I can do it for you. I have the button, but since it can change commit history and create conflicts, I'm not doing this without your confirmation).

strfry commented 4 years ago

You're probably right about those further instances of 'linux' tests also applying to FreeBSD. I'll look into them later.

The pulseaudio option isn't much of a problem IMHO. The goal is to submit an actual Port Makefile upstream, and dealing with optional dependencies should be part of that.

I didn't do testing with various configurations yet. Maybe that can serve as a motivation, to move the CI at a later point to a more cross-platform solution ;)

Feel free to do change the PR to develop. I actually rebased the thing on develop after i noticed it exists, so it must have been a missclick that i can't correct now.

gavv commented 4 years ago

I've cherry-picked cab8dbbc360fc6960cd7cfcdad4ecf3a679841c9 into develop because it seems to be a good idea even without freebsd support.

gavv commented 4 years ago

Closing this for now. Feel free to reopen if you decide to continue working on it.

gavv commented 4 years ago

I've created an issue: #360.