From 7a909b035a69edfb8c0192533be7ac4ba4ef4d35 Mon Sep 17 00:00:00 2001 From: Dr Michael Brooks Date: Sat, 29 Aug 2026 16:36:36 +0100 Subject: [PATCH 1/2] Make integer/float audio conversion lossless and symmetric filter_audioconvert scaled float to integer by the positive full-scale value while the integer to float direction divided by the negative one, and truncated the result instead of rounding it. An integer -> float -> integer round trip therefore came back 1 LSB low on 65535 of the 65536 s16 values, so any filter that works in float (filter_volume, the LADSPA wrapper, avfilter) measurably altered audio even when set to unity gain. Scale by the negative full-scale magnitude, round to nearest, and clamp after scaling rather than before, in shared helpers used by all eight float -> integer paths. This makes the round trip exact and removes the saturation asymmetry in the s32 path, which applied a different gain to positive and negative samples and so generated even harmonics. Also fix u8 -> float, which divided by 256 while u8 -> s16 shifts by 8, i.e. treated u8 as full scale at +/-256 rather than +/-128. Audio taking the float path came out 6 dB quieter than the same audio taking the integer path, and a u8 round trip through float did not return its input. Add test_audioconvert, which exhaustively round trips all 65536 s16 and all 256 u8 values through both the interleaved and planar float formats, and checks saturation, symmetry and rounding. All six cases fail before this change. --- src/modules/core/filter_audioconvert.c | 96 ++++----- src/tests/CMakeLists.txt | 1 + .../test_audioconvert/test_audioconvert.cpp | 194 ++++++++++++++++++ 3 files changed, 243 insertions(+), 48 deletions(-) create mode 100644 src/tests/test_audioconvert/test_audioconvert.cpp diff --git a/src/modules/core/filter_audioconvert.c b/src/modules/core/filter_audioconvert.c index a9e177a74..8a2f9b6df 100644 --- a/src/modules/core/filter_audioconvert.c +++ b/src/modules/core/filter_audioconvert.c @@ -1,6 +1,6 @@ /* * filter_audioconvert.c -- convert from one audio format to another - * Copyright (C) 2009-2016 Meltytech, LLC + * Copyright (C) 2009-2026 Meltytech, LLC * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -21,9 +21,38 @@ #include #include +#include #include #include +/* Convert a normalised float sample to integer PCM. + * + * The scale factor is the negative full-scale magnitude (32768 for s16, not + * 32767), matching the divisor used by the integer -> float conversions, so an + * integer -> float -> integer round trip is lossless. Rounding is to nearest + * rather than the truncation an implicit cast would do, and the clamp is + * applied after scaling so +1.0 saturates to the positive maximum instead of + * wrapping. + */ +static inline int16_t f32_to_s16(float f) +{ + long pcm = lrintf(f * 32768.0f); + return CLAMP(pcm, -32768, 32767); +} + +static inline int32_t f32_to_s32(float f) +{ + /* float has a 24-bit mantissa, so scale in double to round correctly. */ + int64_t pcm = llrint((double) f * 2147483648.0); + return CLAMP(pcm, -2147483648LL, 2147483647LL); +} + +static inline uint8_t f32_to_u8(float f) +{ + long pcm = lrintf(f * 128.0f) + 128; + return CLAMP(pcm, 0, 255); +} + static int convert_audio(mlt_frame frame, void **audio, mlt_audio_format *format, @@ -191,11 +220,8 @@ static int convert_audio(mlt_frame frame, float *q = (float *) *audio; int s, c; for (s = 0; s < samples; s++) - for (c = 0; c < channels; c++) { - float f = *(q + c * samples + s); - f = CLAMP(f, -1.0f, 1.0f); - *p++ = 32767 * f; - } + for (c = 0; c < channels; c++) + *p++ = f32_to_s16(*(q + c * samples + s)); *audio = buffer; error = 0; break; @@ -205,12 +231,8 @@ static int convert_audio(mlt_frame frame, int32_t *p = buffer; float *q = (float *) *audio; int i = samples * channels + 1; - while (--i) { - float f = *q++; - f = CLAMP(f, -1.0f, 1.0f); - int64_t pcm = (f > 0.0f ? 2147483647LL : 2147483648LL) * f; - *p++ = CLAMP(pcm, -2147483648LL, 2147483647LL); - } + while (--i) + *p++ = f32_to_s32(*q++); *audio = buffer; error = 0; break; @@ -221,12 +243,8 @@ static int convert_audio(mlt_frame frame, float *q = (float *) *audio; int s, c; for (s = 0; s < samples; s++) - for (c = 0; c < channels; c++) { - float f = *(q + c * samples + s); - f = CLAMP(f, -1.0f, 1.0f); - int64_t pcm = (f > 0.0f ? 2147483647LL : 2147483648LL) * f; - *p++ = CLAMP(pcm, -2147483648LL, 2147483647LL); - } + for (c = 0; c < channels; c++) + *p++ = f32_to_s32(*(q + c * samples + s)); *audio = buffer; error = 0; break; @@ -249,11 +267,8 @@ static int convert_audio(mlt_frame frame, float *q = (float *) *audio; int s, c; for (s = 0; s < samples; s++) - for (c = 0; c < channels; c++) { - float f = *(q + c * samples + s); - f = CLAMP(f, -1.0f, 1.0f); - *p++ = (127 * f) + 128; - } + for (c = 0; c < channels; c++) + *p++ = f32_to_u8(*(q + c * samples + s)); *audio = buffer; error = 0; break; @@ -340,11 +355,8 @@ static int convert_audio(mlt_frame frame, int16_t *p = buffer; float *q = (float *) *audio; int i = samples * channels + 1; - while (--i) { - float f = *q++; - f = CLAMP(f, -1.0f, 1.0f); - *p++ = 32767 * f; - } + while (--i) + *p++ = f32_to_s16(*q++); *audio = buffer; error = 0; break; @@ -373,10 +385,7 @@ static int convert_audio(mlt_frame frame, float *q = (float *) *audio + c; int i = samples + 1; while (--i) { - float f = *q; - f = CLAMP(f, -1.0f, 1.0f); - int64_t pcm = (f > 0.0f ? 2147483647LL : 2147483648LL) * f; - *p++ = CLAMP(pcm, -2147483648LL, 2147483647LL); + *p++ = f32_to_s32(*q); q += channels; } } @@ -389,12 +398,8 @@ static int convert_audio(mlt_frame frame, int32_t *p = buffer; float *q = (float *) *audio; int i = samples * channels + 1; - while (--i) { - float f = *q++; - f = CLAMP(f, -1.0f, 1.0f); - int64_t pcm = (f > 0.0f ? 2147483647LL : 2147483648LL) * f; - *p++ = CLAMP(pcm, -2147483648LL, 2147483647LL); - } + while (--i) + *p++ = f32_to_s32(*q++); *audio = buffer; error = 0; break; @@ -404,11 +409,8 @@ static int convert_audio(mlt_frame frame, uint8_t *p = buffer; float *q = (float *) *audio; int i = samples * channels + 1; - while (--i) { - float f = *q++; - f = CLAMP(f, -1.0f, 1.0f); - *p++ = (127 * f) + 128; - } + while (--i) + *p++ = f32_to_u8(*q++); *audio = buffer; error = 0; break; @@ -443,7 +445,7 @@ static int convert_audio(mlt_frame frame, uint8_t *q = (uint8_t *) *audio + c; int i = samples + 1; while (--i) { - *p++ = ((float) *q - 128) / 256.0f; + *p++ = ((float) *q - 128) / 128.0f; q += channels; } } @@ -478,10 +480,8 @@ static int convert_audio(mlt_frame frame, float *p = buffer; uint8_t *q = (uint8_t *) *audio; int i = samples * channels + 1; - while (--i) { - float f = ((float) *q++ - 128) / 256.0f; - *p++ = CLAMP(f, -1.0f, 1.0f); - } + while (--i) + *p++ = ((float) *q++ - 128) / 128.0f; *audio = buffer; error = 0; break; diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 1ea7be977..10555caab 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -34,6 +34,7 @@ endfunction() add_qt_test(TEST_NAME animation) add_qt_test(TEST_NAME audio) +add_qt_test(TEST_NAME audioconvert) add_qt_test(TEST_NAME events) add_qt_test(TEST_NAME filter) add_qt_test(TEST_NAME frame) diff --git a/src/tests/test_audioconvert/test_audioconvert.cpp b/src/tests/test_audioconvert/test_audioconvert.cpp new file mode 100644 index 000000000..87a330df8 --- /dev/null +++ b/src/tests/test_audioconvert/test_audioconvert.cpp @@ -0,0 +1,194 @@ +/* + * Copyright (C) 2026 Meltytech, LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with consumer library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include +#include +#include +using namespace Mlt; + +class TestAudioConvert : public QObject +{ + Q_OBJECT + +public: + TestAudioConvert() { Factory::init(); } + + ~TestAudioConvert() { Factory::close(); } + +private: + /** Run one conversion through the audioconvert filter and copy the result out. + * + * The filter installs itself as the frame's convert_audio callback, which is + * the same entry point mlt_frame_get_audio() uses. + */ + void convert(const void *in, + mlt_audio_format from, + void *out, + mlt_audio_format to, + int samples, + int channels) + { + int in_size = mlt_audio_format_size(from, samples, channels); + int out_size = mlt_audio_format_size(to, samples, channels); + Profile profile; + mlt_frame frame = mlt_frame_init(NULL); + QVERIFY(frame != NULL); + mlt_properties properties = MLT_FRAME_PROPERTIES(frame); + mlt_properties_set_int(properties, "audio_channels", channels); + mlt_properties_set_int(properties, "audio_samples", samples); + + void *buffer = mlt_pool_alloc(in_size); + memcpy(buffer, in, in_size); + mlt_frame_set_audio(frame, buffer, from, in_size, mlt_pool_release); + + mlt_filter filter = mlt_factory_filter(profile.get_profile(), "audioconvert", NULL); + QVERIFY(filter != NULL); + mlt_filter_process(filter, frame); + QVERIFY(frame->convert_audio != NULL); + + mlt_audio_format format = from; + QCOMPARE(frame->convert_audio(frame, &buffer, &format, to), 0); + QCOMPARE((int) format, (int) to); + memcpy(out, buffer, out_size); + + mlt_filter_close(filter); + mlt_frame_close(frame); + } + +private Q_SLOTS: + + /** Every s16 value must survive a trip through interleaved float. + * + * This failed before the conversion was made symmetric: float -> s16 scaled + * by 32767 while s16 -> float divided by 32768, and the result was truncated + * rather than rounded, so 65535 of the 65536 values came back 1 LSB low. + */ + void S16ToF32leRoundTripIsLossless() + { + const int samples = 65536; + QVector in(samples), out(samples); + QVector mid(samples); + for (int i = 0; i < samples; i++) + in[i] = (int16_t) (i - 32768); + + convert(in.data(), mlt_audio_s16, mid.data(), mlt_audio_f32le, samples, 1); + convert(mid.data(), mlt_audio_f32le, out.data(), mlt_audio_s16, samples, 1); + + for (int i = 0; i < samples; i++) + if (in[i] != out[i]) + QFAIL(qPrintable( + QString("sample %1: %2 came back as %3").arg(i).arg(in[i]).arg(out[i]))); + } + + /** The same, through the planar float format, which takes a different path. */ + void S16ToFloatRoundTripIsLossless() + { + const int channels = 2; + const int samples = 32768; + const int count = samples * channels; + QVector in(count), out(count); + QVector mid(count); + for (int i = 0; i < count; i++) + in[i] = (int16_t) (i - 32768); + + convert(in.data(), mlt_audio_s16, mid.data(), mlt_audio_float, samples, channels); + convert(mid.data(), mlt_audio_float, out.data(), mlt_audio_s16, samples, channels); + + for (int i = 0; i < count; i++) + if (in[i] != out[i]) + QFAIL(qPrintable( + QString("sample %1: %2 came back as %3").arg(i).arg(in[i]).arg(out[i]))); + } + + /** u8 is full scale at +/-128, the same as the u8 -> s16 path assumes. */ + void U8RoundTripIsLossless() + { + const int samples = 256; + QVector in(samples), out(samples); + QVector mid(samples); + for (int i = 0; i < samples; i++) + in[i] = (uint8_t) i; + + convert(in.data(), mlt_audio_u8, mid.data(), mlt_audio_f32le, samples, 1); + convert(mid.data(), mlt_audio_f32le, out.data(), mlt_audio_u8, samples, 1); + + for (int i = 0; i < samples; i++) + if (in[i] != out[i]) + QFAIL(qPrintable( + QString("sample %1: %2 came back as %3").arg(i).arg(in[i]).arg(out[i]))); + } + + /** Full scale and beyond must saturate, never wrap. */ + void FloatToIntegerSaturates() + { + const float in[4] = {1.0f, -1.0f, 2.0f, -2.0f}; + int16_t s16[4]; + int32_t s32[4]; + uint8_t u8[4]; + + convert(in, mlt_audio_f32le, s16, mlt_audio_s16, 4, 1); + QCOMPARE(s16[0], (int16_t) 32767); + QCOMPARE(s16[1], (int16_t) -32768); + QCOMPARE(s16[2], (int16_t) 32767); + QCOMPARE(s16[3], (int16_t) -32768); + + convert(in, mlt_audio_f32le, s32, mlt_audio_s32le, 4, 1); + QCOMPARE(s32[0], (int32_t) 2147483647); + QCOMPARE(s32[1], (int32_t) -2147483647 - 1); + + convert(in, mlt_audio_f32le, u8, mlt_audio_u8, 4, 1); + QCOMPARE(u8[0], (uint8_t) 255); + QCOMPARE(u8[1], (uint8_t) 0); + } + + /** Equal positive and negative inputs must give equal magnitudes. + * + * The old s32 path scaled positives by 2147483647 and negatives by + * 2147483648, which is an asymmetric gain and so generates even harmonics. + */ + void FloatToIntegerIsSymmetric() + { + const float in[2] = {0.5f, -0.5f}; + int16_t s16[2]; + int32_t s32[2]; + + convert(in, mlt_audio_f32le, s16, mlt_audio_s16, 2, 1); + QCOMPARE(s16[0], (int16_t) 16384); + QCOMPARE(s16[1], (int16_t) -16384); + + convert(in, mlt_audio_f32le, s32, mlt_audio_s32le, 2, 1); + QCOMPARE(s32[0], (int32_t) 1073741824); + QCOMPARE(s32[1], (int32_t) -1073741824); + } + + /** Narrowing must round to nearest, not truncate toward zero. */ + void FloatToIntegerRoundsToNearest() + { + const float in[2] = {100.6f / 32768.0f, -100.6f / 32768.0f}; + int16_t s16[2]; + + convert(in, mlt_audio_f32le, s16, mlt_audio_s16, 2, 1); + QCOMPARE(s16[0], (int16_t) 101); + QCOMPARE(s16[1], (int16_t) -101); + } +}; + +QTEST_APPLESS_MAIN(TestAudioConvert) + +#include "test_audioconvert.moc" From 37c2f2c1c8a98ef7a0477ea0f8a643fb975b51ff Mon Sep 17 00:00:00 2001 From: Dan Dennedy Date: Tue, 1 Sep 2026 14:30:15 -0700 Subject: [PATCH 2/2] Fix lrintf/llrint overflow very large positive to negative Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/modules/core/filter_audioconvert.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/modules/core/filter_audioconvert.c b/src/modules/core/filter_audioconvert.c index 8a2f9b6df..cf303b66e 100644 --- a/src/modules/core/filter_audioconvert.c +++ b/src/modules/core/filter_audioconvert.c @@ -36,6 +36,7 @@ */ static inline int16_t f32_to_s16(float f) { + f = CLAMP(f, -1.0f, 1.0f); long pcm = lrintf(f * 32768.0f); return CLAMP(pcm, -32768, 32767); } @@ -43,12 +44,14 @@ static inline int16_t f32_to_s16(float f) static inline int32_t f32_to_s32(float f) { /* float has a 24-bit mantissa, so scale in double to round correctly. */ + f = CLAMP(f, -1.0f, 1.0f); int64_t pcm = llrint((double) f * 2147483648.0); return CLAMP(pcm, -2147483648LL, 2147483647LL); } static inline uint8_t f32_to_u8(float f) { + f = CLAMP(f, -1.0f, 1.0f); long pcm = lrintf(f * 128.0f) + 128; return CLAMP(pcm, 0, 255); }