Update AudioDecoderConfig usage in media/ to provide a ChannelLayoutConfig [chromium/src : main]

0 views

Skip to first unread message

Syed AbuTalib (Gerrit)

unread,

Feb 11, 2026, 5:08:52 PM (10 hours ago) Feb 11

to Thomas Guilbert, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org

Attention needed from Thomas Guilbert

Syed AbuTalib added 2 comments

File media/formats/mp2t/es_parser_adts.cc

Line 247, Patchset 2: {channel_layout, ChannelLayoutToChannelCount(channel_layout)},

Syed AbuTalib . unresolved

For the es_parser, it seems that `channel_layout` is determined by `adts_parser_.ParseFrameHeader`. Taking a look, this ends up [setting channel_layout](https://source.chromium.org/chromium/chromium/src/+/main:media/formats/mpeg/adts_stream_parser.cc;l=91-92;drc=4544c31a1f99f70e789bf6b278faaa0443d9306e) using [this array](https://source.chromium.org/chromium/chromium/src/+/main:media/formats/mpeg/adts_constants.h;l=27-31;drc=bc7cf067ca6372d86010c2a212dcc28ea0b83df3). I am thinking this is safe, but what do you think?

File media/formats/mp2t/es_parser_mpeg1audio.cc

Line 177, Patchset 2: ChannelLayoutToChannelCount(header.channel_layout)},

Syed AbuTalib . unresolved

Open in Gerrit

Related details

Attention is currently required from:

Thomas Guilbert

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Thomas Guilbert (Gerrit)

unread,

Feb 11, 2026, 5:20:33 PM (10 hours ago) Feb 11

to Syed AbuTalib, Thomas Guilbert, Chromium LUCI CQ, AyeAye, chromium...@chromium.org, feature-me...@chromium.org

Attention needed from Syed AbuTalib

Thomas Guilbert voted and added 5 comments

Votes added by Thomas Guilbert

Code-Review

5 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Thomas Guilbert . resolved

LGTM in general. I would hold off on submitting if you are going to add a `FromLayout` helper.

Commit Message

Line 6, Patchset 4 (Latest):


Update AudioDecoderConfig usage in media/ to provide a ChannelLayoutConfig

Thomas Guilbert . unresolved

The title should be shortened so it at least fits in 80 char, ideally less.

File media/formats/mp2t/es_parser_adts.cc

Line 247, Patchset 2: {channel_layout, ChannelLayoutToChannelCount(channel_layout)},

Syed AbuTalib . resolved

For the es_parser, it seems that `channel_layout` is determined by `adts_parser_.ParseFrameHeader`. Taking a look, this ends up [setting channel_layout](https://source.chromium.org/chromium/chromium/src/+/main:media/formats/mpeg/adts_stream_parser.cc;l=91-92;drc=4544c31a1f99f70e789bf6b278faaa0443d9306e) using [this array](https://source.chromium.org/chromium/chromium/src/+/main:media/formats/mpeg/adts_constants.h;l=27-31;drc=bc7cf067ca6372d86010c2a212dcc28ea0b83df3). I am thinking this is safe, but what do you think?

Thomas Guilbert

If it's been working so far, this seems ok.

File media/formats/mp2t/es_parser_mpeg1audio.cc

Line 177, Patchset 2: ChannelLayoutToChannelCount(header.channel_layout)},

Syed AbuTalib . resolved

Similar story, looks like this will [only be mono or stereo](https://source.chromium.org/chromium/chromium/src/+/main:media/formats/mpeg/mpeg1_audio_stream_parser.cc;l=196-197;drc=4544c31a1f99f70e789bf6b278faaa0443d9306e).

Thomas Guilbert

Acknowledged

File media/formats/mpeg/mpeg_audio_stream_parser_base.cc

Line 289, Patchset 4 (Latest): {channel_layout, ChannelLayoutToChannelCount(channel_layout)},

Thomas Guilbert . unresolved

This pattern occurs a lot. We could potentially add a non templated `FromLayout(ChannelLayout layout)` static helper which does exactly this.