WIP: Replace bass with Qtmultimedia - #1140
Conversation
`unzip -l | grep -q` under `set -o pipefail` could kill unzip with SIGPIPE once grep matched, making the pipeline report failure even when the file was present. Snapshot the listing into a variable so the check no longer races against unzip. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous commit wired CMAKE_MODULE_PATH to ${SCRIPT_DIR}/cmake but
never added the override file, so Qt's bundled FindWrapOpenGL.cmake
(with the -framework AGL line) was still being used.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Drop the "if header exists, skip" short-circuits in get_bass / get_bassopus / get_discordrpc so every configure run re-extracts a fresh copy. Add `unzip -o` so re-extraction overwrites without prompting. - Detect host arch and gate the macOS Discord RPC download on x86_64. The official v3.4.0 release ships an x86_64-only dylib and the repo has been archived since 2018, so arm64 has no usable binary. Pass -DAO_ENABLE_DISCORD_RPC=OFF on arm64 macOS to disable it at build. - Fix a pre-existing typo where the macOS discord-rpc extract listed discord_rpc.h twice instead of also pulling discord_register.h. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Re-introduces the early-return in get_bass / get_bassopus / get_discordrpc when the header is already in ./lib. Use `configure.sh clean` (which already wipes ./lib, ./bin, ./tmp) to force a fresh download. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Useful when multiple Qt versions are installed and you need to pin a
specific one (e.g. for testing against an older LTS) without bumping
QT_MIN_VERSION or hiding other installs. Both QT_ROOT= and QT_PATH= can
now be passed in any order; QT_ROOT is still required for locating the
bundled CMake/MinGW/Ninja under ${QT_ROOT}/Tools.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
First step of the BASS → QtMultimedia migration. No behavior change yet — this just validates the dependency before swapping the audio player classes over. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the 5 BASS HSTREAMs with 5 QSoundEffect instances cycled the same way. Non-wav blips (opus, mp3, ogg) are decoded once via QAudioDecoder and cached as a temp .wav under ./tmp/blip-cache/<hash>.wav so QSoundEffect can play them — blips are small enough that the one-shot synchronous decode is negligible. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
5 HSTREAMs replaced with 5 QSoundEffect instances. Round-robin allocation, looping toggle (loopCount = Infinite vs 1), and stopAllLoopingStream all preserved. SFX is now wav-only — content packs shipping .opus or .mp3 SFX will no longer play; documented as an accepted regression. Blips are unaffected (AOBlipPlayer pre-decodes non-wav sources). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
FINALLY fuck bass, that shit haunted me for so long. Great stuff! |
Replaces HSTREAM[2] with QMediaPlayer/QAudioOutput pairs. Highlights: - Custom .txt sidecar loop points now in ms instead of bytes; legacy non-seconds form converts via a one-shot QAudioDecoder sample-rate probe. seconds=true form is direct. - Loop wrap is a 5 ms QTimer poll on QMediaPlayer::position() (vs BASS sample-accurate sync — accepted ~10 ms jitter). No custom points: QMediaPlayer::setLoops(Infinite). - Crossfade: SYNC_POS reads the old player's position and seeks the new one once it reaches LoadedMedia. FADE_OUT animates the old player's volume to 0 over 4 s (InExpo) then deleteLater. FADE_IN ramps from 0 to target over 1 s (OutExpo). - HTTP streaming preserved via QUrl(httpUrl); Options::streamingEnabled gate unchanged. - ~stop.mp3 sentinel handled explicitly: destroy stream, return "None". playStream is now synchronous. The QFutureWatcher<QString> wrapper and the Courtroom::update_ui_music_name slot are removed — QMediaPlayer setSource is non-blocking, so the worker-thread offload no longer serves a purpose. Audio device routing temporarily uses the default output; the picker in the options dialog is wired up in the next commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces BASS_GetDeviceInfo enumeration in the options dialog with QMediaDevices::audioOutputs(). Devices are still stored by description string, so existing user prefs migrate transparently. Actual routing of the players through the selected device is wired up in the next commit once AOApplication grows currentAudioDevice(). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- AOApplication: drop #include <bass.h>, initBASS, load_bass_plugins, BASSreset, doBASSreset. Add QAudioDevice currentAudioDevice() that resolves Options::audioOutputDevice() against QMediaDevices, falling back to defaultAudioOutput(). - Courtroom: drop ao_app->initBASS() from the constructor. - AOMusicPlayer, AOSfxPlayer, AOBlipPlayer: route through ao_app->currentAudioDevice() so the options dialog picker actually selects the output device. - Lobby about box: update the "BASS audio engine" line to QtMultimedia. After this commit there are no BASS references left in src/. The configure.sh download and the CMake target_link_libraries entries are removed in the next commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Final step of the BASS → QtMultimedia migration. Removes the target_link_libraries entries and the configure.sh download functions plus their call sites. lib/bass.h, lib/libbass.dylib, etc. on the contributor's filesystem are gitignored and get cleaned by `./configure.sh clean`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Each platform job now collapses to: checkout → install Qt → run ./configure.sh → deploy + upload. The inline BASS/discord-rpc/QtApng/ themes downloads and the cmake/ninja invocations are gone — configure.sh already does all of that, and on Windows runs windeployqt too. To make configure.sh usable from CI: - Add BUILD_TYPE= CLI override (replaces the hardcoded Debug default). - find_ninja falls back to ninja on PATH when none is bundled with Qt; GitHub runners have ninja from the system rather than under Qt/Tools. The Linux job keeps its patchelf + tar packaging and AppImage build, since those are deployment artifacts that configure.sh isn't in the business of producing. It also stages the qtapng .so into Qt's plugins dir so appimagetool can pick it up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
QT_PATH= already says "use exactly this Qt install" — QT_ROOT was
redundant once that existed. The script still uses QT_ROOT internally
(find_cmake / find_mingw look under ${QT_ROOT}/Tools), but it's now
derived from QT_PATH (two dirname levels up) instead of being a second
flag the caller has to remember.
CI invocations collapse to a single line.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Formatting-only pass over the files touched by the BASS → QtMultimedia migration so the CI clang-format check passes. No behavior change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
find_cmake and find_qt both used a `return 1` when no path was found, which combined with `set -e` and `VAR=\$(find_cmake)` exited the script before the caller's "fall back to PATH" branch could run. CI hit this on both Windows and Linux because install-qt-action doesn't install the Tools/CMake_64 (Windows) or Tools/CMake (Linux) directories by default. Both helpers now emit empty stdout on miss and always return 0; callers already inspect the string and handle empty as "not found". Also folded each helper's nested check_path closure into a single platform switch + test for clarity. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two related fixes for the Windows job:
- build.yml: request tools_mingw1120 via install-qt-action so MinGW 11.2
(the toolchain Qt 6.5.3 was built with) ends up under \${QT_ROOT}/Tools.
install-qt-action doesn't install matching MinGW automatically — the
previous workflow worked only because it called cmake with the runner's
PATH-resident MinGW. configure.sh looks under Tools/ specifically.
- configure.sh: guard find_mingw against a missing Tools/ dir (was
shelling out to \`find\` which printed "No such file or directory" then
tripped set -e), and add a PATH fallback for gcc/g++ on Windows when
no MinGW is bundled. Mirrors the same logic find_cmake already had.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- tools_mingw1120 → tools_mingw1310. MinGW 11.2 isn't reliably available
via aqt anymore; 13.1 is what install-qt-action ships for Qt 6.5.3
on Windows (confirmed against akashi's working CI config).
- Copy libgcc_s_seh-1.dll / libstdc++-6.dll / libwinpthread-1.dll from
${IQTA_TOOLS}/mingw1310_64/bin into ./bin/ after the build. configure.sh
runs windeployqt with --no-compiler-runtime (matches the pre-migration
flags), so these need to be staged separately for the artifact to run
on a machine without MinGW installed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
lgtm. CI runs clean. artifacts need to be tested |
The bytes-to-ms conversion in AOMusicPlayer's loop-sidecar parsing is
the trickiest pure logic in the migration; this lifts it into a
standalone helper so it can be exercised in isolation.
- src/loopsidecar.{h,cpp}: pure parseLoopSidecarText(text, rate_provider)
→ LoopPoints {start_ms, end_ms}. The sample-rate provider is invoked
lazily, only when the legacy non-seconds form is encountered.
AOMusicPlayer::parseLoopSidecar is now a thin wrapper.
- test/test_loopsidecar.cpp: 16 QTest cases covering the seconds and
legacy forms, loop_length vs loop_end semantics, lazy probe behaviour
(called at most once, skipped entirely for seconds form, treated as
failure when returning 0 or null), malformed/unknown line handling,
whitespace tolerance, and the override / mid-stream toggle semantics.
- enable_testing() moved to the top-level CMakeLists so ctest discovers
tests from the build root, not just from test/.
- build.yml: add a "Run tests" step (ctest --output-on-failure) to both
the Windows and Linux jobs.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
test_aopacket uses QTEST_MAIN, which builds a QGuiApplication and needs a platform plugin. The Linux runner is headless — xcb can't connect to a display and the test aborts before its first assertion. Force the offscreen QPA for the ctest step so QGuiApplication initialises against a memory-backed surface. test_loopsidecar (QTEST_APPLESS_MAIN) was unaffected. Windows runners have a real display so no equivalent fix is needed there. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Switches both the main build and the qtapng sub-build from invoking ninja directly to \`cmake --build . --parallel\`. cmake's --parallel defers to the native build tool's default job count when no number is given (Ninja: nproc+2, MinGW Makefiles: nproc), so this gives explicit parallelism without coupling the script to a specific generator. Matches the idiom akashi uses; harmless on platforms where ninja was already saturating cores. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This reverts commit d0ef7f4.
|
(cons: it couldn't read metadata from streaming..) |
|
@Ganty1999 For what it's worth,I looked into this because I was curious about the metadata concern too. Here's what I found after a bit of digging through the code: BASS can parse metadata (via BASS_StreamGetTags()), but AO2 doesn't actually use it at least that's what I could gather from a brief glance at bass documentation. https://documentation.help/BASS/documentation.pdf The current implementation in aomusicplayer.cpp ONLY uses BASS for basic playback loading files with BASS_StreamCreateFile() and playing them with BASS_ChannelPlay(). The "song title" displayed in the chat is literally just the filename without extension |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Please stop with the back and forth. There are channels for that. |
|
why is it called loop sidecar |
|
'tbh'.. why we matter about the "license" if the library are allowed anyone to uses.. if it really "incompatible" with something... surely we nor the who using bass lib will getting sue by the bass team.. (pardon if I soundly something weird or such 🙏) |
tbh, you're probably right that nobody's getting sued over BASS LOL. The "incompatibility" is more about project hygiene than actual legal risk. it's about keeping things simple and consistent. One less proprietary DLL to ship, one less dependency to manage, and one less thing to explain to new contributors. Again all depends on what the maintainers want for the project. So yeah you're all good. So both sides are right really. Unless it's comments regarding code not much more I can contribute currently, it's up to the maintainers what direction they so go with. I'll agree to disagree about whether we should stay with bass or not even if I'm on the side of the latter, no hard feelings. |
|
There are kind of two perspectives here Pedantic: "technically the license isn't legal for distribution and you should in principle stick to framework tools and not 3rd party libraries" I'm inclined towards going the pragmatic route, and key contributors seem to swing that way too. If there are no further comments, I'll close this in a few days. |
|
The switch to QtMultimedia is the definitive route. |
|
As sD has said, the concept in of itself is not bad. It's just that it would take a serious amount of effort to attempt to match up to what bass already does. I can't imagine it's easy for one person, even with third party assistance. Not that bass itself is even the best thing out there, it's just what was used. There's nothing that dictates that it must stay that way forever though. Thank you for acknowledging the perspectives on it. Whatever the case, this is very much a breaking change and will need alot of testing even if it was very cleanly implemented without use of third-party-code. I would offer to close this draft and place this work onto a branch i.e "QtMultimedia-Audio", so you and other users may work towards that end goal, whenever that may be. You may look towards the contribution.md if you'd like to know what you're getting into. Regardless of the outcome, the point is still made, there are grievances with bass and even if it is difficult, a future without it is not unthinkable. This draft could be a hop off point for that. |
| namespace | ||
| { | ||
| // Standard 44-byte WAV header for PCM data. | ||
| void writeWavHeader(QFile &out, const QAudioFormat &fmt, qint32 pcmBytes) |
There was a problem hiding this comment.
why are we writing wav files
| { | ||
| BASS_ChannelStop(m_stream_list[streamId]); | ||
| return QObject::tr("[MISSING] Streaming disabled."); | ||
| qWarning() << "Failed to probe sample rate for" << mediaPath << "— legacy byte-form loop points will be ignored."; |
There was a problem hiding this comment.
did we ever user byte form loop points ??
|
One question that remains is media format support. I don't want to go back to the dark ages and use wav files for blips instead of opus or ogg. |
|
This should not be merged with any known shortcomings or breaking changes. To weed this out it'd be helpful to try finding bugs in the latest build artifact: https://github.com/AttorneyOnline/AO2-Client/actions/runs/32835861133/artifacts/9558676398 |
The obvious consequence of BASS being GPL-incompatible is that enforcing the license becomes hard. BASS' source code being unavailable also has problems. For example, I am using a 64-bit ARM system with musl libc. There are no precompiled BASS libraries for my platform, so it is currently impossible for me to use the client with sound. BASS does not "work fine", it's a major pain in the ass. |
|
@cn-cn-cn |
This is so true. Client would be so much better off without it. Building it is more annoying than necessary |
7 participants
long live qt or something
got carried away and reworked CI to use the now-hopefully-universal-and-portable
configure.shtoo. let me know if this is not okay and I'll revert it