fix(audio): zero-initialise the channel layouts before the resampler copy
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
This commit is contained in:
+17
-2
@@ -155,9 +155,24 @@ struct DecodeCtx {
|
|||||||
|
|
||||||
bool open_resampler(DecodeCtx& c, const AVFrame* f) {
|
bool open_resampler(DecodeCtx& c, const AVFrame* f) {
|
||||||
#if LIBAVUTIL_VERSION_INT >= AV_VERSION_INT(57, 24, 100)
|
#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
|
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 (av_channel_layout_copy(&in_layout, &f->ch_layout) < 0) return false;
|
||||||
if (in_layout.nb_channels <= 0) {
|
if (in_layout.nb_channels <= 0) {
|
||||||
av_channel_layout_uninit(&in_layout);
|
av_channel_layout_uninit(&in_layout);
|
||||||
|
|||||||
Reference in New Issue
Block a user