[PR] avformat/movenc: fix dec3 chan_loc derivation for E-AC-3 dependent substreams (PR #24238)

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]