Skip to content

Conversation

@norihiro
Copy link
Contributor

@norihiro norihiro commented Sep 26, 2024

Description

The variable input.conversion.allow_clipping was left uninitialized. This could result in randomly sending unclamped audio to the output handlers that did not request the conversion.

To avoid the same initialized variables again when adding a new variable to the structure, I initialized audio at the declaration so that all other variables are initialized to zero.

Motivation and Context

The member variable allow_clipping was introduced by the commit dcee4fc in #8289.

In the function audio_output_connect, which allows the parameter conversion to be null to indicate no conversion is requested,
the variable allow_clipping was not initialized when the conversion was not requested.

I'm writing a plugin that calls audio_output_connect through obs_add_raw_audio_callback like this.
When following the code to see what is the default value of allow_clipping, I realized it was never initialized.

How Has This Been Tested?

Had a modification below to log allow_clipping just before it was used.

diff --git a/libobs/media-io/audio-io.c b/libobs/media-io/audio-io.c
index 18911ea58..06b922a13 100644
--- a/libobs/media-io/audio-io.c
+++ b/libobs/media-io/audio-io.c
@@ -117,6 +117,8 @@ static inline void do_audio_output(struct audio_output *audio, size_t mix_idx,
 	for (size_t i = mix->inputs.num; i > 0; i--) {
 		struct audio_input *input = mix->inputs.array + (i - 1);
 
+		blog(LOG_INFO, "%s: i=%zu: allow_clipping=%d", __func__, i, (int)input->conversion.allow_clipping);
+
 		float(*buf)[AUDIO_OUTPUT_FRAMES] =
 			input->conversion.allow_clipping ? mix->buffer_unclamped
 							 : mix->buffer;

Then, called obs_add_raw_audio_callback twice with conversion = NULL.

Checked the log and saw allow_clipping is uninitialized.

Without this commit, allow_clipping sometimes get non-zero value.

info: do_audio_output: i=1: allow_clipping=0
info: do_audio_output: i=2: allow_clipping=255

With this commit, I confirmed allow_clipping is always 0.

do_audio_output: i=1: allow_clipping=0
do_audio_output: i=2: allow_clipping=0

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

The variable `input.conversion.allow_clipping` was left uninitialized.
This could result in randomly sending unclamped audio to the output
handlers that did not request the conversion.
@norihiro norihiro force-pushed the uninitialized-allow_clipping branch from 6e70ec5 to 3720f3d Compare September 26, 2024 13:35
@norihiro norihiro changed the title libobs: Fix sending unclamped audio libobs: Fix sending unclamped audio to output handler Sep 26, 2024
@WizardCM WizardCM added the Bug Fix Non-breaking change which fixes an issue label Sep 26, 2024
Copy link
Member

@derrod derrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An oversight on my part when I originally introduced this feature.

Hopefully we've not blown anybody's ears out by allowing audio on Twitch to clip 😅

@RytoEX RytoEX added this to the OBS Studio 31 milestone Sep 26, 2024
@RytoEX RytoEX merged commit 1dc87ca into obsproject:master Sep 26, 2024
@norihiro norihiro deleted the uninitialized-allow_clipping branch September 27, 2024 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Fix Non-breaking change which fixes an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants