bugfix(audio): Apply configured volume to Bink movie audio by CryoTheRenegade · Pull Request #3083 · TheSuperHackers/GeneralsGameCode

CryoTheRenegade · 2026-08-09T01:58:37Z

Problem

Bink movie audio bypasses the Miles mixer. The initial BinkSetVolume() call ran before Bink's audio output was ready, so movies ignored the configured Speech volume and could play at full volume.

Changes

Call BinkWait() before applying the initial volume in BinkVideoPlayer::createStream().
Add VideoPlayerInterface::setVolume() and implement it in BinkVideoPlayer so Speech-volume changes update every open Bink stream.
Keep the retail volume conversion, including its nonzero minimum because Bink treats a literal volume of 0 as full volume.
Use explicit no-op implementations for FFmpeg and tool-only video players, whose audio volume is not handled by this Bink path.

Testing

Built all Win32 Release targets for Generals and Zero Hour with MSVC, including MapCacheBuilder and WorldBuilder.
Started Zero Hour through the rebuilt executable and verified Bink intro playback, the Options Voice Volume control, menu flow, and clean exit.
Started a Challenge at Speech 0 and Speech 100. The loading interval measured -28.8 dB mean / -12.0 dB peak at Speech 0 and -18.3 dB mean / -0.1 dB peak at Speech 100. The capture also contained unaffected music and sound effects.
Replaced EALogoMovie synchronously with Sizzle in a test harness. The replacement rendered normally and received the configured volume. The same 35-second interval measured -83.3 dB mean / -66.2 dB peak at Speech 0 and -19.7 dB mean / -2.5 dB peak at Speech 100.

Fixes TheSuperHackers#2850. Movie audio ignored the Options volume sliders because BinkSetVolume was called once in createStream(), before Bink's audio output had started, so it never took effect. Extract the volume calc into calculateMovieAudioVolume() and reapply it on every decoded frame in frameDecompress() so it takes effect and tracks live slider changes.

Fix the floor comment (formula is unchanged from retail, floor is 327 not 1) and drop the live-slider-change claim from the per-frame comment; the Options menu isn't reachable during movie playback. The reason to reapply every frame is that the one-shot createStream() call ran before Bink's audio output started, so it never took effect.

Per review: don't reapply the volume every frame. The original defect was that BinkSetVolume ran in createStream(), before Bink's audio output existed, so it was discarded. Apply it once on the first BinkDoFrame(), the earliest point it can actually take effect. Drops the createStream() call and the per-frame poll in favor of a one-shot latch.

Skyaero42 · 2026-08-10T08:31:43Z

I don't think the title is correct, as the volume is not applied every frame, but only once.

- Return the 327 floor instead of 0 on the null-audio path so it does not trip Bink's 'play at full volume' quirk. - Trim the redundant formula comment. - Restore retail tab alignment on m_handle/m_memFile and align m_volumeSet.

CryoTheRenegade · 2026-08-10T13:50:24Z

I don't think the title is correct, as the volume is not applied every frame, but only once.

fixed

Per xezon's review. Replace the per-frame latch with a push model: - VideoPlayerInterface gains a setVolume(Real) virtual (no-op default in VideoPlayer); BinkVideoPlayer overrides it to push the volume to every open stream's Bink audio output. - MilesAudioManager::processPlayingList() pushes the speech volume to TheVideoPlayer whenever the volume changes, so movies that bypass the Miles mixer follow the sliders. - BinkVideoPlayer::update() applies the volume once on the first frame a stream exists, fixing the original too-early set in createStream() (Bink's audio output is not running yet at creation, so it was lost). - calculateMovieAudioVolume() now takes the speech volume as a parameter.

Skyaero42 · 2026-08-10T19:43:21Z

You are clearly using AI. While there is in principle nothing against it, I would strongly recommend you review the changes it makes - per our contribution guide.
The sloppy improvements currently take up unnecessary and valuable review time.

bobtista · 2026-08-19T19:29:35Z


 		virtual void notifyVideoPlayerOfNewProvider( Bool nowHasValid ) override { }

+		virtual void setVolume( Real volume ) override { }


FFmpegVideoPlayer derives from VideoPlayer, so it picks up this no-op and the volume plumbing is Bink-only. Override it there too?

FFmpegVideoPlayer now overrides setVolume and pushes the speech volume to the OpenAL movie stream. Initial volume is also applied when the stream starts, since that backend is already live by then.

Wait do we have OpenAL in upstream yet? This might not work until we get that landed :(

Yeah, OpenAL is not in this repo yet. The FFmpeg override is behind RTS_USE_OPENAL so Miles/Bink builds keep a no-op, and the OpenAL gain path is just plumbing until that backend lands.

Co-authored-by: Cursor <cursoragent@cursor.com>

bobtista · 2026-08-22T16:45:47Z

This is looking good :) Can you do some testing and add the results to the PR description? eg

Starting a Challenge/movie with Speech volume already at 0 or a low value.
Replacing one Bink movie with another without an intervening empty update.
Confirming the replacement movie also receives the configured volume.

CryoTheRenegade · 2026-08-24T19:43:34Z

Done

greptile-apps · 2026-08-30T20:44:01Z

Greptile Summary

The PR updates Bink movie audio to apply the configured speech volume after polling the newly opened stream and propagates subsequent speech-volume changes to active movies.

Adds a shared video-player volume-control contract with Bink, FFmpeg, and null-provider implementations.
Applies Bink volume after BinkWait during stream creation and updates every open Bink stream when the setting changes.
Uses NullVideoPlayer in non-video tool targets so they continue satisfying the expanded interface.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp	Applies initial volume after polling the Bink stream and propagates later volume changes across open streams.
Core/GameEngineDevice/Source/MilesAudioDevice/MilesAudioManager.cpp	Forwards changed speech volume to the active video-player provider during audio-list processing.
Core/GameEngine/Include/GameClient/VideoPlayer.h	Expands the shared video-player contract with volume control and supplies a no-op null implementation.
Core/GameEngineDevice/Include/VideoDevice/FFmpeg/FFmpegVideoPlayer.h	Keeps the optional FFmpeg provider compatible with the expanded interface through a no-op override.

Sequence Diagram

sequenceDiagram
    participant Audio as MilesAudioManager
    participant Player as BinkVideoPlayer
    participant Stream as BinkVideoStream
    participant Bink as Bink API
    Player->>Stream: createStream(handle)
    Stream->>Bink: BinkWait(handle)
    Player->>Bink: BinkSetVolume(initial volume)
    Audio->>Player: setVolume(speech volume)
    loop Every open stream
        Player->>Bink: BinkSetVolume(updated volume)
    end

_{Reviews (3): Last reviewed commit: "refactor(video): make null players expli..." | Re-trigger Greptile}

xezon

Good to see this got rid of the update loop hack.

OmarAglan · 2026-09-01T14:52:22Z

@codex

chatgpt-codex-connector · 2026-09-01T14:58:01Z

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 6af80fb5a5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

xezon · 2026-09-02T07:20:33Z

The change title and description are outdated.

CryoTheRenegade · 2026-09-02T15:46:20Z

The change title and description are outdated.

updated

xezon reviewed Aug 9, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

xezon added Audio Is audio related Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker labels Aug 9, 2026

Skyaero42 reviewed Aug 10, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

Skyaero42 reviewed Aug 10, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

Skyaero42 reviewed Aug 10, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Include/VideoDevice/Bink/BinkVideoPlayer.h Outdated

CryoTheRenegade changed the title ~~bugfix(audio): Reapply Bink movie volume every frame~~ bugfix(audio): Reapply Bink movie volume on first frame Aug 10, 2026

xezon reviewed Aug 10, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Include/VideoDevice/Bink/BinkVideoPlayer.h Outdated

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

Skyaero42 reviewed Aug 10, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Include/VideoDevice/Bink/BinkVideoPlayer.h Outdated

xezon reviewed Aug 10, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

fixup(audio): Remove stale Bink volume state

87a335d

Skyaero42 reviewed Aug 11, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Include/VideoDevice/Bink/BinkVideoPlayer.h Outdated

Skyaero42 reviewed Aug 11, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp

fixup(audio): Make Bink volume helper private

7ed80ac

xezon reviewed Aug 15, 2026

View reviewed changes

fixup(audio): Address xezon review comments

1649fcf

bobtista reviewed Aug 19, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

bobtista reviewed Aug 19, 2026

View reviewed changes

CryoTheRenegade and others added 2 commits August 21, 2026 08:42

fixup(audio): Restore delayed Bink volume apply and add FFmpeg setVolume

7387918

Co-authored-by: Cursor <cursoragent@cursor.com>

fixup(audio): Reset Bink volume latch when a new stream is created

8d4a427

Co-authored-by: Cursor <cursoragent@cursor.com>

xezon reviewed Aug 30, 2026

View reviewed changes

Comment thread Core/GameEngine/Include/GameClient/VideoPlayer.h Outdated

Comment thread Core/GameEngineDevice/Include/VideoDevice/Bink/BinkVideoPlayer.h Outdated

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

fix(audio): apply Bink volume after servicing stream

1e7ddfc

xezon reviewed Aug 31, 2026

View reviewed changes

Comment thread Core/GameEngine/Include/GameClient/VideoPlayer.h Outdated

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

Comment thread Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp Outdated

fix(audio): refine video volume initialization

809d30f

xezon reviewed Aug 31, 2026

View reviewed changes

Comment thread Core/GameEngine/Include/GameClient/VideoPlayer.h Outdated

refactor(video): make null players explicit

6af80fb

xezon approved these changes Sep 1, 2026

View reviewed changes

Comment thread Core/GameEngineDevice/Include/VideoDevice/FFmpeg/FFmpegVideoPlayer.h

CryoTheRenegade changed the title ~~bugfix(audio): Reapply Bink movie volume on first frame~~ bugfix(audio): Apply configured volume to Bink movie audio Sep 2, 2026

xezon approved these changes Sep 2, 2026

View reviewed changes

xezon merged commit e2ad13b into TheSuperHackers:main Sep 2, 2026
23 checks passed

CryoTheRenegade deleted the bugfix/2850-video-audio-volume branch September 4, 2026 01:47


		virtual void notifyVideoPlayerOfNewProvider( Bool nowHasValid ) override { }

		virtual void setVolume( Real volume ) override { }

Uh oh!

Conversation

CryoTheRenegade commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Skyaero42 commented Aug 10, 2026

Uh oh!

CryoTheRenegade commented Aug 10, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Skyaero42 commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobtista Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

CryoTheRenegade Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

bobtista Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

CryoTheRenegade Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

bobtista commented Aug 22, 2026

Uh oh!

CryoTheRenegade commented Aug 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OmarAglan commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

xezon commented Sep 2, 2026

Uh oh!

CryoTheRenegade commented Sep 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

CryoTheRenegade commented Aug 9, 2026 •

edited

Loading

Skyaero42 commented Aug 10, 2026 •

edited

Loading

greptile-apps Bot commented Aug 30, 2026 •

edited

Loading