Created attachment 247555 [details] update to 0.10.0 Upstream released a new version with improved FreeBSD support and added OSS input backend. This patch brings the port up to date.
Release notes: https://github.com/karlstav/cava/releases/tag/0.10.0
Looking at the diff it seems like this port previously supported pulseaudio and portaudio but no longer does. Is that correct? If so, what's the reason for that?
Removing portaudio and pulseaudio support was intentional. While I admit removing portaudio is probably unjustified, I have the firm believe that pulseaudio should never be enabled by default in a FreeBSD port if it is not a true hard dependency for the program to function. The sole reason it is installed on a system can interfere with the audio setup, because it has the rude behaviour of activating itself even without the user's explicit configuration and activation via /etc/rc.conf. Some users go to great length to suppress this unwanted behaviour and replace /usr/local/bin/pulseaudio with a symlink to /bin/true. The autospawn=no option in the pulseaudio configuration is not always sufficient. Even the linux ecosystem is starting to leave pulseaudio behind in favor for their new sound server system pipewire. Currently there is another bugreport in progress (not by me), bug #276222, in which pulseaudio gets demoted from an unconditional dependency to a port option. CAVA now supports the native OSS (in cooperation with virtual_oss if needed), the small, working, unobtrusive and BSD-spirited sndio, portaudio (I will add it back to the port) and probably soon JACK (I'm currently working on it). There are plenty of options to choose from without the problematic nature of pulseaudio. I will readd portaudio option and can readd pulseaudio option if desired. But I would strongly suggest to disable the pulseaudio port option by default.
I understand your despair with pulseaudio and so do many others indeed. I'm not arguing in favor of pulseaudio - as a committer my point of view is the port itself. As such, what I observe is that this port presumably was working with pulseaudio and portaudio for users who chose to do so previously and now wouldn't anymore without upstream having dropped support nor any port internal issue. I would suggest re-adding those options again and optionally discussing changing the default options with the maintainer. Most people trying to avoid pulseaudio just have OPTIONS_UNSET+=PULSEAUDIO anyway :p
Unless backends breaks the build or intended behaviour of the application there's no need to rip out <insert backend here> "just because". Removing as a default option is fine as there are multiple audio backends enabled and it leaves the user with the option to enable others if desired. "Good intentions" are always valuable but try to limit that to default options unless there's a security or other major issue behind the decision. In this case it doesn't seem like there is none more than the general design of Pulseuadio but that's neither a security or other issue calling for removal. In most cases we leave defaults to the maintainer as that's where bugs reports (opinions) will end up unless there will be breakage in tree of consumers. Best regards, Daniel
Created attachment 247655 [details] update to 0.10.0
Thank you two for your suggestions and explanations. I agree, good intentions can harm good results. I think it's more important to update this port for all the good additions the new version has (OSS, SDL, GLSL) instead of surprising existing users of CAVA that pulseaudio doesn't work anymore. I have uploaded a new patch. portaudio and pulseaudio are again available and enabled by default.
I missed the part where Pulseaudio and Portaudio was removed. I do agree removing them is a bit much, but if they are set as optional where if someone doesn't agree with the defaults they can build it from ports and disable them that is okay.
Created attachment 247779 [details] update to 0.10.0 During the last few days I consolidated my online profiles. I updated the username and email info inside the patch. Sorry for the noise.
Comment on attachment 247779 [details] update to 0.10.0 This looks good to me, the previous patch built just fine.
A remark on ALSA and pipewire support as discussed in the reviev D43535: Somebody has to test stuff. ALSA was disabled since the beginning of the port, I have no clue if it can actually work (does recording with alsa-lib in FreeBSD work?). And if somebody has an interessant on pipewire than somebody has to test it. But seriously, I have no idea if pipewire, despite being available in FreeBSD, is actually a *thing*, i.e. at least on the forums I have literally never seen anybody who actually uses pipewire on FreeBSD. Leaving ALSA and pipewire disabled in the port was not 'opinionated' by me. I actually have nothing against ALSA (it just gets routed to OSS transparently on FreeBSD AFAIK) and I'm in bliss about pipewire. I added OSS and JACK support upstream, improved sndio backend upstream and provided a patch to update this port. Sorry for not testing ALSA and pipewire for which I have zero interest.
Let me know if I can support the review process. If desired I can: - apply the {opt}_CONFIGURE_ENABLE change and test the build process - invest some time in Pipewire and ALSA testing (if that's a roadblock) Regarding -Wno-error=deprecated-non-prototype: the only warning I saw was because of "void getPulseDefaultSink();" in input/pulse.h. I *accidently* fixed this already upstream, but it's not in version 0.10.0. Upstream has released version 0.10.1 which includes (among other things) the prototype fix, JACK support and configurable parameters in the sndio backend. Do we want to finish up this update to 0.10.0 first, or do we update to 0.10.1 in this PR?
Apologies for not having gotten back to this earlier - Real life was crazy the last two weeks. If you can update directly to to 0.10.1 that would be great. Please do apply the {opt}_CONFIGURE_ENABLE change as well.
Created attachment 248168 [details] update to 0.10.1 No need to apologize, real life gets us all from time to time ;) I provide the patch for the 0.10.1 update: - minor extended patch notes - added JACK option - applied {opt}_CONFIGUE_ENABLE change and tested it - removed CFLAGS+=-Wno-error=deprecated-non-prototype I build CAVA with clang (from base), gcc10, ..., gcc14, on x86_64 14.0-RELEASE-p4 with quarterly packages. I didn't encounter any message regarding the deprecated-non-prototype error/warning. I guess it's fixed and not necessary any more, therefore I removed it from the Makefile. Feel free to readd it if you think it's still necessary (different architectures?).
Comment on attachment 248168 [details] update to 0.10.1 This looks good to me, and builds without issues in Poudiere for FreeBSD 13.x and 14.x.
For some reason, when I update the maintainer approval flag on the patch it does not go through.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=9807a6d4946fcb63ae15f3b2845bbb6c58bee24d commit 9807a6d4946fcb63ae15f3b2845bbb6c58bee24d Author: bsdcode <bsdcode@disroot.org> AuthorDate: 2024-02-04 01:33:21 +0000 Commit: Joel Bodenmann <jbo@FreeBSD.org> CommitDate: 2024-02-10 21:42:47 +0000 audio/cava: Update to 0.10.1 Changelogs: - 0.10.1: https://github.com/karlstav/cava/releases/tag/0.10.1 - 0.10.0: https://github.com/karlstav/cava/releases/tag/0.10.0 - 0.9.1: https://github.com/karlstav/cava/releases/tag/0.9.1 - 0.9.0: https://github.com/karlstav/cava/releases/tag/0.9.0 - 0.8.3: https://github.com/karlstav/cava/releases/tag/0.8.3 - 0.8.2: https://github.com/karlstav/cava/releases/tag/0.8.2 - 0.8.1: https://github.com/karlstav/cava/releases/tag/0.8.1 - 0.8.0: https://github.com/karlstav/cava/releases/tag/0.8.0 - 0.7.5: https://github.com/karlstav/cava/releases/tag/0.7.5 PR: 276228 Approved by: tcberner (mentor), Adam Jimerson <vendion@gmail.com> (maintainer) Differential Revision: https://reviews.freebsd.org/D43750 audio/cava/Makefile | 81 ++++++++++-------- audio/cava/distinfo | 6 +- audio/cava/files/patch-Makefile.am (gone) | 22 ----- audio/cava/files/patch-cava.c (gone) | 96 ---------------------- audio/cava/files/patch-config.c (gone) | 34 -------- audio/cava/files/patch-configure.ac (gone) | 13 --- .../cava/files/patch-example__files_config (gone) | 14 ---- audio/cava/files/patch-input_sndio.c (gone) | 22 ----- .../files/patch-output_terminal__ncurses.c (gone) | 16 ---- audio/cava/pkg-descr | 8 +- 10 files changed, 54 insertions(+), 258 deletions(-)
Committed - Thanks!