PR #24238 opened by phyd URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24238 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24238.patch
# Summary of changes The mov/mp4 muxer derives the `dec3` box's `chan_loc` field from the E-AC-3 bitstream's `chanmap` with a plain shift, but the two fields use opposite bit orders. Every E-AC-3 stream carrying a dependent substream — i.e. 7.1 and above — is therefore muxed into MP4 with the wrong channel layout signalled in the container. `libavformat/movenc.c`, `handle_eac3()`: ```c info->substream[parent].chan_loc |= (hdr->channel_map >> 5) & 0x1f; ``` `chanmap` is a 16-bit field read MSB-first (`get_bits(gbc, 16)` in `ac3_parser.c`), so flag index *i* sits at bit `15 - i` — the same ordering `ff_eac3_custom_channel_map_locations` is indexed against a few lines above. `chan_loc` bit *j* carries `chanmap` flag index `5 + j` per ETSI TS 102 366 Annex F, i.e. bit `10 - j`. Converting between them is a bit reversal, not a shift. The `0x1f` mask is also 5 bits where 9 are needed, so some flags are dropped rather than merely misplaced. Effect on the layouts that use a dependent substream: | layout | `chanmap` | before | after | |---|---|---|---| | 7.1 | `0x1A00` | `0x010` | `0x002` | | 5.1.2 | `0x0010` | `0x000` | `0x040` | | 5.1.4 | `0x0210` | `0x010` | `0x042` | For 7.1 the field claimed `Lsd/Rsd` (surround direct) — a position no 7.1 speaker layout has — instead of `Lrs/Rrs`. This is invisible to FFmpeg itself, because our decoder reads the layout from the bitstream, so an affected file still probes as a correct 7.1. Demuxers that trust the container do act on it. # Steps to reproduce Using FFmpeg's own public sample, so no upload is needed: ``` curl -O https://samples.ffmpeg.org/A-codecs/AC3/eac3/7_pt_1.eac3 ffmpeg -i 7_pt_1.eac3 -c:a copy out.mp4 ``` Its dependent substream declares `chanmap = 0x1A00` (Ls, Rs, Lrs/Rrs). Reading `chan_loc` back out of the resulting `dec3`: ``` before this patch : chan_loc = 0x010 [Lsd/Rsd] after this patch : chan_loc = 0x002 [Lrs/Rrs] GPAC MP4Box : chan_loc = 0x002 [Lrs/Rrs] (same input, for comparison) ``` An independent muxer agreeing with the patched output seemed worth checking before assuming FFmpeg was the one in the wrong here. A decoder that reads the container shows the difference directly. Apple's AudioToolbox (`afconvert`/`afinfo`) on the same input: ``` unpatched : 7.1 (L C R Ls Rs LFE Lsd Rsd) patched : 7.1 (L C R Ls Rs LFE Rls Rrs) ``` On that decoder the practical result of the unpatched output is that the track collapses — measured on a commercially authored DD+ 7.1 title, the centre channel, all four surrounds and the LFE decoded to silence, leaving a quiet stereo downmix. Rewriting that one byte restores all eight channels. # Environment Reproduced and fixed against current master, `eb0bfa852e` (`avutil/opt: reformat set_string_binary`): ``` ffmpeg version git-2026-08-21-eb0bfa8 Copyright (c) 2000-2026 the FFmpeg developers built with Apple clang version 21.0.0 (clang-2100.1.1.101) configuration: --disable-everything --disable-autodetect --disable-doc --disable-network --disable-avdevice --disable-swscale --enable-protocol='file,pipe' --enable-demuxer='eac3,ac3,mov,matroska' --enable-muxer='mov,mp4,eac3' --enable-decoder='eac3,ac3' --enable-parser=ac3 ``` macOS 15 (Darwin 25.6.0), Apple Silicon. Also reproduced on the 8.1.2 release build. The line is unchanged in master; the decoder-side dependent-substream support added in 4.0 (`ae92970`) is unrelated. Nothing below 7.1 is affected — those carry no dependent substream, so `num_dep_sub` is 0 and `chan_loc` is never written. From 782e7b6e4cc50318911557041135bac2681bf23d Mon Sep 17 00:00:00 2001 From: Brian Leake <[email protected]> Date: Fri, 21 Aug 2026 16:03:12 -0700 Subject: [PATCH] avformat/movenc: fix dec3 chan_loc derivation for E-AC-3 dependent substreams chan_loc was derived from the bitstream's chanmap with a plain shift, but the two fields use opposite bit orders, so every E-AC-3 stream carrying a dependent substream (7.1 and above) was muxed with the wrong channel layout signalled in the container. chanmap is a 16-bit field read MSB-first, so flag index i sits at bit (15 - i); this ordering is the same one ff_eac3_custom_channel_map_locations is indexed against in ac3_parser.c. chan_loc bit j carries chanmap flag index 5 + j per ETSI TS 102 366 Annex F, i.e. bit (10 - j). Converting between them is a bit reversal rather than a shift. The 0x1f mask was also 5 bits where 9 are needed, so some flags were dropped rather than merely misplaced. Effect on the layouts that use a dependent substream: layout chanmap before after 7.1 0x1A00 0x010 0x002 5.1.2 0x0010 0x000 0x040 5.1.4 0x0210 0x010 0x042 For 7.1 the field claimed Lsd/Rsd (surround direct), a position no 7.1 speaker layout has, instead of Lrs/Rrs. This is invisible to FFmpeg, whose decoder reads the layout from the bitstream, but demuxers that trust the container act on it: Apple's AudioToolbox reported the affected files as "7.1 (L C R Ls Rs LFE Lsd Rsd)" and collapsed them to a stereo downmix, losing the centre channel, all four surrounds and the LFE. Verified with samples.ffmpeg.org/A-codecs/AC3/eac3/7_pt_1.eac3, whose dependent substream declares chanmap 0x1A00. After this change the muxed dec3 carries 0x002, matching GPAC MP4Box for the same input, and AudioToolbox reports "7.1 (L C R Ls Rs LFE Rls Rrs)" with all eight channels intact. Signed-off-by: Brian Leake <[email protected]> --- libavformat/movenc.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 367caecee9..9aef241a37 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -572,10 +572,18 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track) ret /= 8; /* get the dependent stream channel map, if exists */ - if (hdr->channel_map_present) - info->substream[parent].chan_loc |= (hdr->channel_map >> 5) & 0x1f; - else + if (hdr->channel_map_present) { + /* chanmap is a 16-bit field read MSB-first, so flag index + * i sits at bit (15 - i). chan_loc bit j carries chanmap + * flag index 5 + j, i.e. bit (10 - j). Converting between + * the two is a bit reversal, not a shift. */ + for (int j = 0; j < 9; j++) { + if ((hdr->channel_map >> (10 - j)) & 1) + info->substream[parent].chan_loc |= 1 << j; + } + } else { info->substream[parent].chan_loc |= hdr->channel_mode; + } cumul_size += hdr->frame_size; } } -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
