bugfix(audio): Reapply Bink movie volume on first frame - #3083
Open
CryoTheRenegade wants to merge 7 commits into
Open
bugfix(audio): Reapply Bink movie volume on first frame#3083CryoTheRenegade wants to merge 7 commits into
CryoTheRenegade wants to merge 7 commits into
Conversation
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.
xezon
reviewed
Aug 9, 2026
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.
xezon
added
Audio
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
reviewed
Aug 10, 2026
Skyaero42
reviewed
Aug 10, 2026
Skyaero42
reviewed
Aug 10, 2026
|
I don't think the title is correct, as the volume is not applied every frame, but only once. |
CryoTheRenegade
changed the title
- 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.
Author
fixed |
xezon
reviewed
Aug 10, 2026
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
reviewed
Aug 10, 2026
|
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. |
xezon
reviewed
Aug 10, 2026
Skyaero42
reviewed
Aug 11, 2026
| virtual VideoStreamInterface* load( AsciiString movieTitle ) override; ///< Load video file in to memory for playback | ||
|
|
||
| // TheSuperHackers @bugfix Compute the Bink volume for a given speech volume. | ||
| static Int calculateMovieAudioVolume( Real speechVolume ); |
Skyaero42
reviewed
Aug 11, 2026
| void BinkVideoPlayer::update() | ||
| { | ||
| VideoPlayer::update(); | ||
|
|
There was a problem hiding this comment.
Maybe it is not a good idea to prompt the LLM to revert whatever it did, and use a diff tool instead.
Author
There was a problem hiding this comment.
should i revert the whitespace?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2850.
Movie audio (Bink) bypasses the Miles mixer and plays straight through DirectSound via
BinkSoundUseDirectSound, so the only volume control isBinkSetVolume(). It was called exactly once inBinkVideoPlayer::createStream(), before Bink's audio output had actually started, so it never took effect. Movies played at full volume regardless of the Options sliders.The fix extracts the volume calc into
calculateMovieAudioVolume()and reapplies it on every decoded frame inBinkVideoStream::frameDecompress(), so the setting takes effect once audio is running and live slider changes during playback are picked up.The volume formula itself is unchanged from retail: floor of 327 out of 32768, never literal 0, because Bink interprets 0 as "play at full volume". That quirk is why the original defensive comment exists; the floor isn't the bug.