Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libobs/media-io: Change speaker layout to match FFmpeg aac. #1182

Closed
wants to merge 1 commit into from

Conversation

pkviet
Copy link
Member

@pkviet pkviet commented Feb 2, 2018

(also changes obs-ffmpeg).
The default channel layouts from aac spec are implemented in FFmpeg native
aac encoder as:
AV_CH_LAYOUT_MONO,
AV_CH_LAYOUT_STEREO,
AV_CH_LAYOUT_SURROUND,
AV_CH_LAYOUT_4POINT0,
AV_CH_LAYOUT_5POINT0_BACK,
AV_CH_LAYOUT_5POINT1_bACK,
AV_CH_LAYOUT_7POINT1,

The correspondence of speaker layouts to AV_CH_LAYOUT from FFmpeg is
changed to reflect the previous table.
Although FFmpeg native aac encoder can now encode all the layouts listed
in avutil channel_layout.h (on master), there might be issues with older FFmpeg
binaries. This PR fixes those issues for 2.1 and 5.1 layouts.

NB:
Note that 2.1 speaker layout will be encoded as AV_CH_LAYOUT_SURROUND
(FL FR FC) because it is not listed as the default layout for three
channels.
This just means some optimizations for LFE channel will not be used by
the encoder which will treat it as an SCE (single channel element).

(also changes obs-ffmpeg).
The default channel layouts from aac spec are implemented in FFmpeg native
aac encoder as:
    AV_CH_LAYOUT_MONO,
    AV_CH_LAYOUT_STEREO,
    AV_CH_LAYOUT_SURROUND,
    AV_CH_LAYOUT_4POINT0,
    AV_CH_LAYOUT_5POINT0_BACK,
    AV_CH_LAYOUT_5POINT1_bACK,
    AV_CH_LAYOUT_7POINT1,

The correspondence of speaker layouts to AV_CH_LAYOUT from FFmpeg is
changed to reflect the previous table.
Although FFmpeg native aac encoder can now encode all the layouts listed
in avutil channel_layout.h (on master), there might be issues with older FFmpeg
binaries.
Note that 2.1 speaker layout will be encoded as AV_CH_LAYOUT_SURROUND
(FL FR FC) because it is not listed as the default layout for three
channels.
This just means some optimizations for LFE channel will not be used by
the encoder which will treat it as an SCE (single channel element).
@SuslikV
Copy link
Contributor

SuslikV commented Feb 2, 2018

Related bug report: #1163

I think 3 channels setup not required at all. The controversial info placed in the code intentionally may cause future problems in the development. Also, it should change the 48a8e75 for OBS SPEAKERS_2POINT1 description and MS KSAUDIO_SPEAKER_2POINT1 (custom made) value for audio monitoring. The forum has number of logs that shows only 2 channels usage. It is wise to wait to gather more info on multi-channel formats usage requested by end-users.
In general, more restrictions in code - more stable software is.

@pkviet
Copy link
Member Author

pkviet commented Feb 2, 2018

I think 3 channels setup not required at all. The controversial info placed in the code intentionally may cause future problems in the development.

That's your opinion. Please could you not conflate your own usage to other people's case-uses ?

Also, it should change the 48a8e75 for OBS SPEAKERS_2POINT1 description

No need to change anything as I explained in detail. This will be properly encoded as 2.1 by FFmpeg native aac if ffmpeg is on git head (or later than commit fc9dcfe7d50d7 dated nov 9 2017); on earlier FFmpeg with this commit it will be encoded as 3.0 which just means some low-pass filter is not applied by the encoder.
The other encoders coreaudio aac or lib-fdk-aac will also encode as 3.0 without any issues.
The opus encoders, pcm, ac3 will encode correctly 2.1.
It is a bit hasty to judge there is no case use for 2.1 surround sound.

and MS KSAUDIO_SPEAKER_2POINT1 (custom made) value for audio monitoring.

This requires a separate commit . You can resubmit that if you want. But that would be purely cosmetic with no impact.

The forum has number of logs that shows only 2 channels usage. It is wise to wait to gather more info on multi-channel formats usage requested by end-users.

The release is what ? 11 days old ...

@jp9000
Copy link
Member

jp9000 commented Feb 6, 2018

You had a bit of a submodule issue there, I fixed it and will merge shortly.

@jp9000 jp9000 closed this in 645d6ae Feb 6, 2018
@pkviet pkviet deleted the layout branch April 2, 2018 12:19
Andersama pushed a commit to Andersama/obs-studio that referenced this pull request Sep 22, 2018
(This commit also modifies the obs-ffmpeg module)

The default channel layouts from aac spec are implemented in FFmpeg
native aac encoder as follows:

    AV_CH_LAYOUT_MONO,
    AV_CH_LAYOUT_STEREO,
    AV_CH_LAYOUT_SURROUND,
    AV_CH_LAYOUT_4POINT0,
    AV_CH_LAYOUT_5POINT0_BACK,
    AV_CH_LAYOUT_5POINT1_BACK,
    AV_CH_LAYOUT_7POINT1,

The correspondence of speaker layouts to AV_CH_LAYOUT from FFmpeg is
changed to reflect the previous table.

Although FFmpeg native aac encoder can now encode all the layouts listed
in avutil channel_layout.h (on master), there might be issues with older
FFmpeg binaries.

Note that 2.1 speaker layout will be encoded as AV_CH_LAYOUT_SURROUND
(FL FR FC) because it is not listed as the default layout for three
channels.

This just means some optimizations for LFE channel will not be used by
the encoder which will treat it as an SCE (single channel element).

Closes obsproject#1182
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants