Skip to content

Fix startup crash when recording audio encoder is not registered#1736

Open
summeroff wants to merge 3 commits into
stagingfrom
fix_recording_audio_encoder_null_codec
Open

Fix startup crash when recording audio encoder is not registered#1736
summeroff wants to merge 3 commits into
stagingfrom
fix_recording_audio_encoder_null_codec

Conversation

@summeroff

Copy link
Copy Markdown
Contributor

Description

Guards against a null codec in the recording audio encoder setup, which crashes the backend at startup when the AAC encoder type is not registered.

Crash

Sentry reports an EXCEPTION_ACCESS_VIOLATION_READ / 0x0 with this signature:

strlen
std::_Narrow_char_traits<char>::length        (__msvc_string_view.hpp:456)
std::operator<<(ostream&, const char*)         (__msvc_ostream.hpp:774)
OBS_service::setupRecordingAudioEncoder        (nodeobs_service.cpp:1199)
... OBS_API::OBS_API_initAPI                   (nodeobs_api.cpp:1027)

setupRecordingAudioEncoder() streams the codec from obs_get_encoder_codec(id) into an ostringstream:

const char *codec = obs_get_encoder_codec(id.c_str());   // id defaults to "ffmpeg_aac"
nameStream << "adv_audio_recording_" << codec << "_" << i;

When the encoder type is not registered, obs_get_encoder_codec() returns NULL (libobs obs-encoder.c: return info ? info->codec : NULL;), and operator<<(ostream&, const char*) calls strlen(NULL) → AV.

Root cause

The default recording audio encoder ffmpeg_aac is provided by obs-ffmpeg.dll. In the affected reports that module is not loaded (Failed to load module file '.../obs-ffmpeg.dll', file not found, and in one report all modules fail to register on re-init), so ffmpeg_aac is absent. The logs corroborate it: Failed to get properties for encoder '' (ffmpeg_aac) / Could not enumerate any AAC encoder bitrates.

Fix

Null-guard the codec in both recording audio paths and skip + log instead of dereferencing, so OBS_API_initAPI completes rather than crashing:

  • setupRecordingAudioEncoder() (advanced output)
  • updateAudioRecordingEncoder() (simple output) — also guards a null audioEncoder config value before std::string(codec)

This mirrors the existing null-check pattern in GetVideoEncoderName(). Surfacing the missing-plugin condition to the frontend is handled separately (on top of #1733).

How Has This Been Tested?

  • Incremental build of obs-studio-server-lib (Release, VS 2022): 0 errors.
  • clang-format 18.1.3 --dry-run -Werror on the changed lines: clean.

Types of changes

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

🤖 Generated with Claude Code

setupRecordingAudioEncoder() streamed the codec from obs_get_encoder_codec()
into an ostringstream with no null check. When the audio encoder type
(default ffmpeg_aac, provided by obs-ffmpeg.dll) is not registered — e.g. the
plugin failed to load — obs_get_encoder_codec() returns NULL and
operator<<(ostream&, const char*) calls strlen(NULL), crashing with
EXCEPTION_ACCESS_VIOLATION during OBS_API_initAPI.

Guard the null codec in both the advanced (setupRecordingAudioEncoder) and
simple (updateAudioRecordingEncoder) recording paths: log and skip audio
encoder setup so initialization completes instead of crashing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Guards against a startup crash in OBS_service when the configured recording audio encoder isn’t registered (e.g., obs-ffmpeg.dll not loaded), by null-checking the codec returned from obs_get_encoder_codec() and skipping encoder setup with an error log instead of streaming a null const char* into an ostringstream.

Changes:

  • Add a null-guard for codec in advanced recording audio encoder setup (setupRecordingAudioEncoder()).
  • Add a null-guard for both the configured encoder id and codec in simple recording audio encoder setup (updateAudioRecordingEncoder()), logging and returning early.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1195 to +1199
const char *codec = obs_get_encoder_codec(id.c_str());
if (!codec) {
blog(LOG_ERROR, "Recording audio encoder '%s' is not registered; skipping audio encoder setup", id.c_str());
return;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — addressed in dd24e88, but at the root rather than in the guard. clearRecordingAudioEncoder() released the encoders without nulling the pointers, so the dangling state existed independently of this guard (it was only masked because setupRecordingAudioEncoder() previously always overwrote the slots). It now nulls each pointer after release, so after clear()+early-return the tracks read as absent and updateRecordingAudioTracks()/obs_output_set_audio_encoder() are safe. This also fixes the latent release-without-null for all callers, not just the early-return path.

clearRecordingAudioEncoder() released the advanced recording audio encoders
but left the pointers in AdvancedRecordingAudioTracks[] dangling. Until now
the immediately-following setupRecordingAudioEncoder() always overwrote
them, but with the null-codec guard it can return early (when the encoder
type is unregistered), leaving released pointers that
updateRecordingAudioTracks() (obs_encoder_active) and
obs_output_set_audio_encoder() would dereference. Null each pointer after
release so the slots read as absent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +1555 to +1559
const char *codec = audioEncoder ? obs_get_encoder_codec(audioEncoder) : nullptr;
if (!codec) {
blog(LOG_ERROR, "Simple recording audio encoder '%s' not registered; skipping setup", audioEncoder ? audioEncoder : "(null)");
return;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 1618d95 by moving the guard above the audioSimpleRecEncID = "" reset, so the early-return path mutates nothing — the cached ID and audioSimpleRecordingEncoder stay in their prior consistent state instead of ID-cleared-but-encoder-kept. (After the guard, audioEncoder is guaranteed non-null, so the reset is now unconditional.) This bails before touching anything, so it can't disrupt an existing/active encoder.

Move the null-codec guard above the audioSimpleRecEncID reset so the
early-return path mutates nothing. Previously the ID was cleared before the
guard ran, leaving audioSimpleRecEncID = "" while audioSimpleRecordingEncoder
still pointed at a previously-created encoder. Returning before any mutation
keeps the cached ID and encoder pointer consistent (and avoids disrupting an
existing encoder).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

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.

2 participants