obsproject
aja: Add audio channel selection to capture
Description
- Added audio channel selection and FC/LFE channel swapping UI to AJA Source plugin.
- Adapted audio repacker from DeckLink plugin, for use with 32-bit samples. Code has an ifdef to use SIMD or C version, and currently defaults to use the SIMD version.
The AJA source plugin was originally written to use the SPEAKERS_7POINT1 layout, with no support for additional layouts. This was done early in development, likely because AJA cards (in the OBS plugin) always process 8ch of audio at a time, regardless of how many channels are present on the wire. After some discussions with another OBS developer, "pkv", I have found that the AJA source plugin will need to implement the rest of the OBS speaker layouts to properly handle capturing a signal containing 2-8ch of audio.
To properly support all of the OBS speaker layouts in the AJA source plugin, we needed to implement an audio "repacker", similar to what the DeckLink capture plugin uses. The audio repacker is used to squash the AJA 8-channel audio buffers down to the sizes required by the various OBS speaker layouts (2 through 8 channels). The AJA plugin could not directly use the audio repacker code from the DeckLink plugin however, because AJA cards always process audio as 32-bit words, whereas the repacker in the DeckLink plugin was hard-coded in SIMD to consider 16-bit samples.
The audio repacker code in this PR has been adapted from the DeckLink audio repacker code, and contains both a new C version and SIMD version. I have tested both versions to ensure they work the same. There are functions to handle the repacking for 32-bit AJA samples, as well as the 16-bit samples from DeckLink. There are also functions to handle the channel swapping behavior found (swap FC and LFE channels in surround layouts) in the DeckLink plugin's audio channel selection.
I've had some discussions with "pkv", the maintainer of the DeckLink audio code, about placing the new repacking code in a more universal place and refactoring both DeckLink and AJA Source plugins to use it. We can discuss that further in this PR. For now the AJA repacker lives "plugins/aja", and the DeckLink repacker remains in "plugins/decklink", which pkv said was acceptable as well.
NOTE: To support arm64 we will need to use the C version of the repacker, or implement a Neon version of the repacker, if desired.
Motivation and Context
The AJA source plugin did not properly handle all of the various OBS speaker layouts. The plugin was hard-coded to use SPEAKERS_7POINT1 by mistake. As best I can tell this caused OBS to treat our audio buffers as 7.1 all of the time, even if the card was capturing fewer than 8ch of audio. The AJA source plugin must always capture a full 8ch buffer from the card, but it needs to be squashed to accommodate the OBS speaker layouts.
How Has This Been Tested?
Tested by capturing an 8ch audio test signal into an AJA io4K+ card, enabling and disabling individual audio channels in the test signal. I checked all of the speaker layouts in OBS and verified that the correct audio channel(s) from my test signal were heard in OBS on the corresponding channel(s).
Types of changes
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
This PR adapted the audio repacker code from DeckLink plugin, to work with the AJA 32-bit audio buffers. The version of the repacker in this PR has implementations for both 16-bit and 32-bit PCM samples, with the idea that it could eventually be placed into a common location for both plugins to use. This is something that @pkviet and I had been discussion prior to my submission of this PR. However I opted to hold off on that change, as modifications to another plugin should happen in a separate PR. This was also the advice I'd been given in the past from another OBS developer when I had made changes that crossed over into the DeckLink plugin.
That issue aside, I'd like to at least get this PR merged into the 28 beta soon so that I can provide builds to our QA department for testing. It has been difficult for me to keep our internal OBS CI builds up to date with all of the deps and other changes happening in OBS. It is much easier for me to point them to the OBS github CI builds.
I've added Seeking Testers to this PR so that QA can download the CI builds directly from this PR.
apart from the points raised by @norihiro , and my minor points, LGTM and i'm in favour of including this in beta
Did you check both HDMI and SDI ? Is the swap required LFE<=> FC required ? it could be for decklink only.
I haven't specifically tested with HDMI yet, only SDI. Were you seeing problems on DeckLink HDMI with certain HDMI surround sound audio signals?
I've added Seeking Testers to this PR so that QA can download the CI builds directly from this PR.
I didn't realize that was all that needed to be done in order to produce CI builds. In that case I will get these builds to our QA department today. I'm fine waiting until the next beta cycle to get this merged.
I've added Seeking Testers to this PR so that QA can download the CI builds directly from this PR.
I didn't realize that was all that needed to be done in order to produce CI builds. In that case I will get these builds to our QA department today. I'm fine waiting until the next beta cycle to get this merged.
Has this been tested/validated yet? Also, please resolve the merge conflicts.
I've added Seeking Testers to this PR so that QA can download the CI builds directly from this PR.
I didn't realize that was all that needed to be done in order to produce CI builds. In that case I will get these builds to our QA department today. I'm fine waiting until the next beta cycle to get this merged.
Has this been tested/validated yet? Also, please resolve the merge conflicts.
I've resolved the merge conflict.
I've tested this PR with AJA SDI capture by verifying that the audio channels were metering and monitoring properly in OBS for each of the speaker layouts. I have not yet done any live streaming or record-to-disk tests. Our QA lead did do some record-to-disk tests however, as well as similar audio monitoring tests. He also tested with SDI capture.
Neither of us have tested HDMI capture yet. I will check in with our QA lead after the Thanksgiving holiday and see if we can do some final tests on HDMI before we sign off.
This can be merged. Both the AJA QA lead and I have tested the changes with SDI and HDMI, with all of the speaker layout settings that I've added. I've also tested the FC/LFE swap and that works too. I've verified this by inspecting both the audio meters, monitoring and recording a file to disk with up to 8 channels of audio.
This can be merged. Both the AJA QA lead and I have tested the changes with SDI and HDMI, with all of the speaker layout settings that I've added. I've also tested the FC/LFE swap and that works too. I've verified this by inspecting both the audio meters, monitoring and recording a file to disk with up to 8 channels of audio.
Thanks for the checks.