obsproject
Honor go live config audio channels property
Description
Changes libobs to allow setting per (audio) encoder speaker layout, and updates the multitrack video output to configure its audio encoders based on the received manifest
Motivation and Context
The Twitch iOS app (and/or HLS playback on iOS in general?) requires audio to be stereo or mono; with this change the requested number of channels from the go live config is being honored, so users can still set up their recordings to contain audio in whichever speaker layout they desire, while the format for streamed audio adheres to service requirements
How Has This Been Tested?
Twitch Enhanced Broadcasting beta (since v22)
Types of changes
- New feature (non-breaking change which adds functionality)
- Tweak (non-breaking change to improve existing 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.
I agree with @derrod that given how late in the stage we are, this should hold off until a later version. I also think that something like this may be better implemented as a generic "audio conversion" mode like the obs_output_set_audio_conversion() feature for raw outputs.
This seems like a pretty big change that I'm not super comfortable making at this stage of the beta. And it's implicit behaviour that's also not really visible to the user.
For now I'd rather we just error out if the configured number of channels on the client side does not match what is expected from the encoder configuration in the JSON response.
We can then revisit this for 31.0. I imagine not too many people will run into this anyway.
@derrod: we could probably make this fairly visible via the "incompatible settings" dialog even; will discuss internally how we want to handle this
I also think that something like this may be better implemented as a generic "audio conversion" mode like the
obs_output_set_audio_conversion()feature for raw outputs.
@tt2468: possibly, though neither audio nor video encoders currently have an outside API for specifying a full audio/video conversion. hard to say if this should change since validation of a full conversion parameter set is somewhat more involved than just validating single parameters, though the underlying mechanism (new "fake" video_t/audio_t) probably has to stay the same to at least allow (binary) api compatibility/transparency
@derrod: opened https://github.com/obsproject/obs-studio/pull/10874 which goes a bit beyond "showing an error message", but should be a smaller set of changes overall
I'm personally still not a fan of silently changing this for the user, but implementation wise it looks fine.
I'm personally still not a fan of silently changing this for the user, but implementation wise it looks fine.
As far as I can tell, this is only changed on the stream output and only if Multitrack Video is being used, which reduces the impact a bit, but I do see your point.
It looks like there is a logged message that the channels are being set. Would you prefer some indication in the UI that audio channels on the stream output may change? Or is your issue that you do not think the stream output audio channels should change from the user setting at all?
It looks like there is a logged message that the channels are being set. Would you prefer some indication in the UI that audio channels on the stream output may change? Or is your issue that you do not think the stream output audio channels should change from the user setting at all?
I think we should tell the user to fix their settings (i.e. what we do now), rather than silently changing them. This is also the case with other incompatible settings such as 120 FPS, even though similarly we could use the FPS divisor feature to automatically fix that.
There could be an argument for being able to record 5.1 while streaming in stereo, but I think that that should be implemented similarly to how output scaling works, where it is an explicit setting.
I agree that if there is existing behavior that can cause TEB to throw a start error on incompatible settings, that this should do that too, rather than "fixing" the setting behind the scenes. And I still stand behind the idea that an encoder audio conversion system backed by swresample would prove more versatile and useful now and down the line. A dialog could, in theory, give the option to "Use Recommended (Stereo)" and then turn on such a system.
It looks like there is a logged message that the channels are being set. Would you prefer some indication in the UI that audio channels on the stream output may change? Or is your issue that you do not think the stream output audio channels should change from the user setting at all?
I think we should tell the user to fix their settings (i.e. what we do now), rather than silently changing them. This is also the case with other incompatible settings such as 120 FPS, even though similarly we could use the FPS divisor feature to automatically fix that.
we'd like to make get client configuration aware of when users see the current message, will file a follow up/alternative PR if that's the consensus (rather than automatic resampling)
It looks like there is a logged message that the channels are being set. Would you prefer some indication in the UI that audio channels on the stream output may change? Or is your issue that you do not think the stream output audio channels should change from the user setting at all?
I think we should tell the user to fix their settings (i.e. what we do now), rather than silently changing them. This is also the case with other incompatible settings such as 120 FPS, even though similarly we could use the FPS divisor feature to automatically fix that.
There could be an argument for being able to record 5.1 while streaming in stereo, but I think that that should be implemented similarly to how output scaling works, where it is an explicit setting.
I agree with Rodney here on pretty much all points. I do not like silently changing settings and I do not like creating inconsistency in behaviour when it comes to invalid settings.
It looks like there is a logged message that the channels are being set. Would you prefer some indication in the UI that audio channels on the stream output may change? Or is your issue that you do not think the stream output audio channels should change from the user setting at all?
I think we should tell the user to fix their settings (i.e. what we do now), rather than silently changing them. This is also the case with other incompatible settings such as 120 FPS, even though similarly we could use the FPS divisor feature to automatically fix that. There could be an argument for being able to record 5.1 while streaming in stereo, but I think that that should be implemented similarly to how output scaling works, where it is an explicit setting.
I agree with Rodney here on pretty much all points. I do not like silently changing settings and I do not like creating inconsistency in behaviour when it comes to invalid settings.
@Warchamp7, @derrod, @RytoEX: I've filed https://github.com/obsproject/obs-studio/pull/11141 as a tweak to current behavior