From bedc859d3c528e9853d41f546d8772c042fdbfc7 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Wed, 5 Aug 2026 15:16:46 +0200 Subject: [PATCH] fix(audio): zero-initialise the channel layouts before the resampler copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit av_channel_layout_copy() documents that it always uninitialises the destination first, and av_channel_layout_uninit() calls av_freep() on u.map. Declaring the layouts without {} therefore handed free() whatever pointer-shaped garbage occupied that stack slot. Not theoretical: UT-103 aborted with "free(): invalid pointer" in about 1 run in 4. Zero failures in 40 runs after the fix, against 10 in 40 before it. Two things kept it hidden, both worth remembering: - It is stack-dependent, so it disappears under an AddressSanitizer build and reads as a flake in the aggregate test binary, where the case usually passes. ctest, which runs each case in its own process, is what made it a reproducible failure rather than noise. - UT-103 is the only test that reaches this branch, because it is the only one whose input is stereo. The golden-vector tests use a mono 11025 Hz fixture chosen so the vector cannot depend on libswresample — which is right, and means bit-exactness against the golden vector is not evidence about the downmix path. TRACES: IR-004 | SR-003 --- src/audio_signature.cpp | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/audio_signature.cpp b/src/audio_signature.cpp index 6e7f6c2..27684a8 100644 --- a/src/audio_signature.cpp +++ b/src/audio_signature.cpp @@ -155,9 +155,24 @@ struct DecodeCtx { bool open_resampler(DecodeCtx& c, const AVFrame* f) { #if LIBAVUTIL_VERSION_INT >= AV_VERSION_INT(57, 24, 100) - AVChannelLayout out_layout; + // Both MUST be zero-initialised. av_channel_layout_copy documents that it + // "will always uninitialize the destination before copy", and + // av_channel_layout_uninit() calls av_freep() on u.map — so a declaration + // without {} hands free() whatever pointer-shaped garbage the stack frame + // happened to hold. That is a real crash ("free(): invalid pointer"), not a + // theoretical one: it reproduced in roughly 1 run in 4 of UT-103, the only + // test that exercises this branch, because it is the only one whose input + // is stereo and so the only one that reaches the downmix path at all. + // + // It hid for two reasons worth remembering. It is stack-dependent, so it + // vanishes under a sanitizer build and looks like a flake in the aggregate + // test binary; and the golden-vector tests (UT-101) pass a mono 11025 Hz + // fixture, which is chosen precisely so the vector does not depend on the + // resampler — so bit-exactness against the golden vector proves nothing + // about this function. + AVChannelLayout out_layout{}; av_channel_layout_default(&out_layout, 1); // mono - AVChannelLayout in_layout; + AVChannelLayout in_layout{}; if (av_channel_layout_copy(&in_layout, &f->ch_layout) < 0) return false; if (in_layout.nb_channels <= 0) { av_channel_layout_uninit(&in_layout);