Skip to content

Commit 2c14c8f

Browse files
OmniTroidclaude
OmniTroid
and
claude
committed
Add loop-sidecar parser unit tests + run ctest in CI
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>
1 parent 1482a31 commit 2c14c8f

7 files changed

Lines changed: 279 additions & 60 deletions

File tree

‎.github/workflows/build.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ jobs:
4545
shell: bash
4646
run: ./configure.sh QT_PATH="$QT_ROOT_DIR" BUILD_TYPE=Release
4747

48+
- name: Run tests
49+
shell: bash
50+
run: ctest --output-on-failure
51+
4852
- name: Stage MinGW runtime DLLs
4953
shell: pwsh
5054
run: |
@@ -84,6 +88,9 @@ jobs:
8488
- name: Configure and build
8589
run: ./configure.sh QT_PATH="$QT_ROOT_DIR" BUILD_TYPE=Release
8690

91+
- name: Run tests
92+
run: ctest --output-on-failure
93+
8794
- name: Stage APNG plugin for AppImage
8895
run: |
8996
mkdir -p "$QT_ROOT_DIR/plugins/imageformats"

‎CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ qt_add_executable(Attorney_Online
7777
src/hardware_functions.h
7878
src/lobby.cpp
7979
src/lobby.h
80+
src/loopsidecar.cpp
81+
src/loopsidecar.h
8082
src/main.cpp
8183
src/network/websocketconnection.cpp
8284
src/network/websocketconnection.h
@@ -134,6 +136,7 @@ if(AO_ENABLE_DISCORD_RPC)
134136
endif()
135137

136138
if(AO_BUILD_TESTS)
139+
enable_testing()
137140
add_subdirectory(test)
138141
endif()
139142

‎src/aomusicplayer.cpp‎

Lines changed: 10 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "datatypes.h"
44
#include "file_functions.h"
5+
#include "loopsidecar.h"
56
#include "options.h"
67

78
#include <QAudioBuffer>
@@ -131,66 +132,17 @@ void AOMusicPlayer::fadeOutAndDelete(QMediaPlayer *player, QAudioOutput *output,
131132
void AOMusicPlayer::parseLoopSidecar(int streamId, const QString &dataPath, const QString &mediaPath)
132133
{
133134
Stream &s = m_streams[streamId];
134-
s.loop_start_ms = 0;
135-
s.loop_end_ms = 0;
136-
137-
QStringList lines = ao_app->read_file(dataPath).split("\n");
138-
bool seconds_mode = false;
139-
int sample_rate = 0; // probed lazily; only needed for legacy non-seconds form
140-
const int sample_size = 2; // 16-bit
141-
const int num_channels = 2;
142-
143-
for (const QString &line : lines)
144-
{
145-
QStringList args = line.split("=");
146-
if (args.size() < 2)
135+
QString text = ao_app->read_file(dataPath);
136+
LoopPoints lp = parseLoopSidecarText(text, [&]() {
137+
int rate = probeSampleRate(mediaPath);
138+
if (rate == 0)
147139
{
148-
continue;
140+
qWarning() << "Failed to probe sample rate for" << mediaPath << "— legacy byte-form loop points will be ignored.";
149141
}
150-
QString arg = args[0].trimmed();
151-
QString val = args[1].trimmed();
152-
153-
if (arg == "seconds")
154-
{
155-
seconds_mode = (val == "true");
156-
continue;
157-
}
158-
159-
qint64 ms = 0;
160-
if (seconds_mode)
161-
{
162-
ms = static_cast<qint64>(val.toDouble() * 1000.0);
163-
}
164-
else
165-
{
166-
// Legacy form: value is a sample count, converted to ms via probed sample rate.
167-
if (sample_rate == 0)
168-
{
169-
sample_rate = probeSampleRate(mediaPath);
170-
if (sample_rate == 0)
171-
{
172-
qWarning() << "Failed to probe sample rate for" << mediaPath << "— legacy byte-form loop points will be ignored.";
173-
continue;
174-
}
175-
}
176-
quint64 bytes = static_cast<quint64>(val.toUInt()) * sample_size * num_channels;
177-
quint64 frame_bytes = static_cast<quint64>(sample_rate) * sample_size * num_channels;
178-
ms = static_cast<qint64>(bytes * 1000 / frame_bytes);
179-
}
180-
181-
if (arg == "loop_start")
182-
{
183-
s.loop_start_ms = ms;
184-
}
185-
else if (arg == "loop_length")
186-
{
187-
s.loop_end_ms = s.loop_start_ms + ms;
188-
}
189-
else if (arg == "loop_end")
190-
{
191-
s.loop_end_ms = ms;
192-
}
193-
}
142+
return rate;
143+
});
144+
s.loop_start_ms = lp.start_ms;
145+
s.loop_end_ms = lp.end_ms;
194146
}
195147

196148
void AOMusicPlayer::armLoopWatcher(int streamId)

‎src/loopsidecar.cpp‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#include "loopsidecar.h"
2+
3+
#include <QStringList>
4+
5+
LoopPoints parseLoopSidecarText(const QString &sidecar_text, const std::function<int()> &sample_rate_provider)
6+
{
7+
LoopPoints out;
8+
bool seconds_mode = false;
9+
int sample_rate = 0;
10+
constexpr int sample_size = 2; // 16-bit
11+
constexpr int num_channels = 2;
12+
13+
const QStringList lines = sidecar_text.split("\n");
14+
for (const QString &line : lines)
15+
{
16+
QStringList args = line.split("=");
17+
if (args.size() < 2)
18+
{
19+
continue;
20+
}
21+
QString arg = args[0].trimmed();
22+
QString val = args[1].trimmed();
23+
24+
if (arg == "seconds")
25+
{
26+
seconds_mode = (val == "true");
27+
continue;
28+
}
29+
30+
qint64 ms = 0;
31+
if (seconds_mode)
32+
{
33+
ms = static_cast<qint64>(val.toDouble() * 1000.0);
34+
}
35+
else
36+
{
37+
if (sample_rate == 0)
38+
{
39+
sample_rate = sample_rate_provider ? sample_rate_provider() : 0;
40+
if (sample_rate == 0)
41+
{
42+
continue;
43+
}
44+
}
45+
quint64 bytes = static_cast<quint64>(val.toUInt()) * sample_size * num_channels;
46+
quint64 frame_bytes = static_cast<quint64>(sample_rate) * sample_size * num_channels;
47+
ms = static_cast<qint64>(bytes * 1000 / frame_bytes);
48+
}
49+
50+
if (arg == "loop_start")
51+
{
52+
out.start_ms = ms;
53+
}
54+
else if (arg == "loop_length")
55+
{
56+
out.end_ms = out.start_ms + ms;
57+
}
58+
else if (arg == "loop_end")
59+
{
60+
out.end_ms = ms;
61+
}
62+
}
63+
return out;
64+
}

‎src/loopsidecar.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#pragma once
2+
3+
#include <QString>
4+
5+
#include <functional>
6+
7+
struct LoopPoints
8+
{
9+
qint64 start_ms = 0;
10+
qint64 end_ms = 0;
11+
};
12+
13+
// Parse loop-point metadata from an AO music sidecar (the `.txt` that lives
14+
// next to a music file). Recognises `seconds=(true|false)`, `loop_start=N`,
15+
// `loop_end=N`, `loop_length=N`.
16+
//
17+
// In the legacy form (`seconds=false` or absent) N is a sample count and
18+
// must be divided by the file's sample rate to get a ms timestamp.
19+
// `sample_rate_provider` is invoked lazily — at most once, only if the
20+
// sidecar actually contains a non-seconds entry — and should return the
21+
// file's sample rate, or 0 to signal probe failure. When 0, legacy entries
22+
// are silently ignored.
23+
LoopPoints parseLoopSidecarText(const QString &sidecar_text, const std::function<int()> &sample_rate_provider);

‎test/CMakeLists.txt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ find_package(Qt${QT_VERSION_MAJOR} COMPONENTS Test REQUIRED)
44

55
set(CMAKE_INCLUDE_CURRENT_DIR ON)
66

7-
enable_testing(true)
8-
97
set(SKIP_AUTOMOC ON)
108

119
function(ao_declare_test test_id)
@@ -19,3 +17,4 @@ function(ao_declare_test test_id)
1917
endfunction()
2018

2119
ao_declare_test(test_aopacket test_aopacket.cpp ../src/aopacket.cpp)
20+
ao_declare_test(test_loopsidecar test_loopsidecar.cpp ../src/loopsidecar.cpp)

‎test/test_loopsidecar.cpp‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
#include "loopsidecar.h"
2+
3+
#include <QtTest/QTest>
4+
5+
class test_LoopSidecar : public QObject
6+
{
7+
Q_OBJECT
8+
9+
private:
10+
// 44100 Hz, 16-bit stereo: 1 second = 44100 samples = 176400 bytes.
11+
// The parser's legacy form takes sample counts, so a value of 44100 → 1000 ms.
12+
static constexpr int SAMPLE_RATE = 44100;
13+
14+
static std::function<int()> fixedRate(int rate)
15+
{
16+
return [rate]() {
17+
return rate;
18+
};
19+
}
20+
21+
private Q_SLOTS:
22+
void emptyInput()
23+
{
24+
LoopPoints lp = parseLoopSidecarText("", fixedRate(SAMPLE_RATE));
25+
QCOMPARE(lp.start_ms, qint64(0));
26+
QCOMPARE(lp.end_ms, qint64(0));
27+
}
28+
29+
void onlyWhitespace()
30+
{
31+
LoopPoints lp = parseLoopSidecarText("\n\n \n", fixedRate(SAMPLE_RATE));
32+
QCOMPARE(lp.start_ms, qint64(0));
33+
QCOMPARE(lp.end_ms, qint64(0));
34+
}
35+
36+
void secondsFormStartEnd()
37+
{
38+
LoopPoints lp = parseLoopSidecarText("seconds=true\nloop_start=2.5\nloop_end=10", fixedRate(SAMPLE_RATE));
39+
QCOMPARE(lp.start_ms, qint64(2500));
40+
QCOMPARE(lp.end_ms, qint64(10000));
41+
}
42+
43+
void secondsFormStartLength()
44+
{
45+
LoopPoints lp = parseLoopSidecarText("seconds=true\nloop_start=2.5\nloop_length=7.5", fixedRate(SAMPLE_RATE));
46+
QCOMPARE(lp.start_ms, qint64(2500));
47+
QCOMPARE(lp.end_ms, qint64(10000));
48+
}
49+
50+
void legacyFormStartEnd()
51+
{
52+
// 44100 samples = 1 second at 44.1 kHz; 88200 samples = 2 seconds.
53+
LoopPoints lp = parseLoopSidecarText("loop_start=44100\nloop_end=88200", fixedRate(SAMPLE_RATE));
54+
QCOMPARE(lp.start_ms, qint64(1000));
55+
QCOMPARE(lp.end_ms, qint64(2000));
56+
}
57+
58+
void legacyFormStartLength()
59+
{
60+
LoopPoints lp = parseLoopSidecarText("loop_start=22050\nloop_length=22050", fixedRate(SAMPLE_RATE));
61+
QCOMPARE(lp.start_ms, qint64(500));
62+
QCOMPARE(lp.end_ms, qint64(1000));
63+
}
64+
65+
void explicitSecondsFalseIsLegacy()
66+
{
67+
LoopPoints lp = parseLoopSidecarText("seconds=false\nloop_start=44100\nloop_end=88200", fixedRate(SAMPLE_RATE));
68+
QCOMPARE(lp.start_ms, qint64(1000));
69+
QCOMPARE(lp.end_ms, qint64(2000));
70+
}
71+
72+
void secondsFormSkipsProbe()
73+
{
74+
int probeCalls = 0;
75+
auto provider = [&]() {
76+
++probeCalls;
77+
return SAMPLE_RATE;
78+
};
79+
LoopPoints lp = parseLoopSidecarText("seconds=true\nloop_start=1\nloop_end=2", provider);
80+
QCOMPARE(lp.start_ms, qint64(1000));
81+
QCOMPARE(lp.end_ms, qint64(2000));
82+
QCOMPARE(probeCalls, 0);
83+
}
84+
85+
void legacyFormProbesAtMostOnce()
86+
{
87+
int probeCalls = 0;
88+
auto provider = [&]() {
89+
++probeCalls;
90+
return SAMPLE_RATE;
91+
};
92+
LoopPoints lp = parseLoopSidecarText("loop_start=44100\nloop_length=44100\nloop_end=132300", provider);
93+
QCOMPARE(lp.start_ms, qint64(1000));
94+
QCOMPARE(lp.end_ms, qint64(3000)); // loop_end overrides the loop_length computation
95+
QCOMPARE(probeCalls, 1);
96+
}
97+
98+
void probeFailureSilencesLegacyEntries()
99+
{
100+
LoopPoints lp = parseLoopSidecarText("loop_start=44100\nloop_end=88200", fixedRate(0));
101+
QCOMPARE(lp.start_ms, qint64(0));
102+
QCOMPARE(lp.end_ms, qint64(0));
103+
}
104+
105+
void nullProviderTreatedAsFailure()
106+
{
107+
LoopPoints lp = parseLoopSidecarText("loop_start=44100\nloop_end=88200", nullptr);
108+
QCOMPARE(lp.start_ms, qint64(0));
109+
QCOMPARE(lp.end_ms, qint64(0));
110+
}
111+
112+
void malformedLinesSkipped()
113+
{
114+
QString input = "this is not a kv line\n"
115+
"seconds=true\n"
116+
"loop_start=2.5\n"
117+
"garbage_without_equals\n"
118+
"loop_end=10\n";
119+
LoopPoints lp = parseLoopSidecarText(input, fixedRate(SAMPLE_RATE));
120+
QCOMPARE(lp.start_ms, qint64(2500));
121+
QCOMPARE(lp.end_ms, qint64(10000));
122+
}
123+
124+
void unknownKeysIgnored()
125+
{
126+
QString input = "seconds=true\n"
127+
"loop_start=2.5\n"
128+
"something_else=42\n"
129+
"loop_end=10\n";
130+
LoopPoints lp = parseLoopSidecarText(input, fixedRate(SAMPLE_RATE));
131+
QCOMPARE(lp.start_ms, qint64(2500));
132+
QCOMPARE(lp.end_ms, qint64(10000));
133+
}
134+
135+
void whitespaceAroundKvIsTrimmed()
136+
{
137+
QString input = " seconds = true \n"
138+
" loop_start = 2.5\n"
139+
"loop_end= 10 \n";
140+
LoopPoints lp = parseLoopSidecarText(input, fixedRate(SAMPLE_RATE));
141+
QCOMPARE(lp.start_ms, qint64(2500));
142+
QCOMPARE(lp.end_ms, qint64(10000));
143+
}
144+
145+
void lastLoopStartWins()
146+
{
147+
QString input = "seconds=true\n"
148+
"loop_start=1\n"
149+
"loop_start=5\n"
150+
"loop_end=10\n";
151+
LoopPoints lp = parseLoopSidecarText(input, fixedRate(SAMPLE_RATE));
152+
QCOMPARE(lp.start_ms, qint64(5000));
153+
QCOMPARE(lp.end_ms, qint64(10000));
154+
}
155+
156+
void secondsToggleMidStreamApplies()
157+
{
158+
// Mode change mid-file: loop_start uses legacy (sample count at 44.1k),
159+
// then `seconds=true` flips the mode, and loop_end uses seconds.
160+
QString input = "loop_start=44100\n"
161+
"seconds=true\n"
162+
"loop_end=10\n";
163+
LoopPoints lp = parseLoopSidecarText(input, fixedRate(SAMPLE_RATE));
164+
QCOMPARE(lp.start_ms, qint64(1000));
165+
QCOMPARE(lp.end_ms, qint64(10000));
166+
}
167+
};
168+
169+
#include "test/test_loopsidecar.moc"
170+
171+
QTEST_APPLESS_MAIN(test_LoopSidecar)

0 commit comments

Comments
 (0)