Conversation
This class implements a real-time dynamic range compressor as an AudioProcessor, allowing for audio signal compression based on specified parameters. It includes methods for configuring audio format, processing input, and managing internal state.
Added support for dynamic range compression in audio sink.
Added a DynamicRangeCompressor instance to the audio sink for live parameter updates.
|
TESTING IS REMAINING = DRAFT FOR NOW |
|
It's almost done (Yeah it's W.I.P) |
Added constants for compressor settings in FullScreenPlayer.
Fix missing newline at the end of CS3IPlayer.kt
Added a new SwitchPreference for compressor settings.
Added new strings for compressor settings and updated existing ones.
Updated layout properties for compressor dialog and adjusted button configurations.
|
It's working now but needs little work |
|
done +tested+optional (player settings) defualt off |
Updated compressor settings restoration logic to always restore settings when opening the dialog and when the player UI loads.
Refactor compressor dialog functions for clarity and efficiency, updating variable names and UI interactions.
Updated compressor dialog layout to follow the speed_dialog.xml pattern, including adjustments to width, orientation, and component arrangement.
Updated default compressor settings for better audio control.
|
Fixed some minor bugs now it's fully ready I checked it on tv and phone it works perfectly now |
Removed LGPL attribution and simplified comment.
fire-light42
left a comment
There was a problem hiding this comment.
First review based on quick testing and reading. It is a very good and cool feature, but needs some small refinement.
|
I'll tweak stuff according to reviews u done thanks |
Updated documentation and comments for clarity on dynamic range compressor parameters and functionality.
|
fixing minor error |
Updated string formatting for compressor_on_format to use positional arguments.
|
Waiting for re review |
fire-light42
left a comment
There was a problem hiding this comment.
Many smaller changes, but they should be easy to fix.
The feature works well, but it is a bit confusing for users and the code contains a few warnings and a deprecation. It should be ready to merge after this review.
Do not worry about the new settings system, I will update the code before merging.
|
|
||
| // ── Threshold slider + FABs ──────────────────────────────────────── | ||
| binding.compressorThresholdBar.addOnChangeListener { _, value, fromUser -> | ||
| if (fromUser) { compressor.threshold = value; updateThresholdLabel(); updateCurrentLabel() } |
There was a problem hiding this comment.
Please use newlines instead of ";" to follow the rest of the code base.
| </LinearLayout> | ||
|
|
||
| <!-- Threshold row: FAB minus | slider | FAB plus --> | ||
| <TextView |
There was a problem hiding this comment.
Would it be possible to briefly explain how users should think about what this threshold, akin to Sync Subs "Use this if the subtitle are shown 100 ms too late".
Something to tell the user what changing the slider actually does, and preferably what + and - does.
It can be dynamic like the sync subs info, just focus on making it simple and understandable without much text.
|
|
||
| <!-- Makeup Gain row: FAB minus | slider | FAB plus --> | ||
| <TextView | ||
| android:id="@+id/compressor_makeup_label" |
There was a problem hiding this comment.
Same thing here, just a small text to tell the users what changing this does in a simple manner. It would do wonders for usability.
| protected var selectSubtitlesDialog: Dialog? = null | ||
| protected var selectCompressorDialog: Dialog? = null | ||
| protected var playBackCompressorEnabled = false | ||
| set(value) { |
There was a problem hiding this comment.
This setter no longer refers to the selectSubtitlesDialog!
| playerBinding?.apply { | ||
| playerSpeedBtt.isVisible = playBackSpeedEnabled | ||
| playerCompressorBtt.isVisible = playBackCompressorEnabled | ||
| if (playBackCompressorEnabled) restoreCompressorSettings() |
There was a problem hiding this comment.
Please use braces here to reduce risks of bugs when refactoring.
| if (compressor != null) arrayOf(compressor) else emptyArray() | ||
| return DefaultAudioSink.Builder(context) | ||
| .setEnableFloatOutput(enableFloatOutput) | ||
| .setEnableAudioTrackPlaybackParams(enableAudioTrackPlaybackParams) |
There was a problem hiding this comment.
This is deprecated, please use setEnableAudioOutputPlaybackParameters instead
| private var playBackSpeed: Float = 1.0f | ||
|
|
||
| /** Shared compressor — created once, injected into the audio sink, params updated live. */ | ||
| val compressor = DynamicRangeCompressor() |
There was a problem hiding this comment.
It looks like the compressor is always used even if the setting is turned off, it just affects how to player compressor logic works. I would prefer if the dynamic compressor object is not used at all when turned off in player settings.
This is because I cannot trust all android TV variants to behave as expected. Some TV variants will undoubtedly have unexpected issues with a custom audio processor even if the compressor is perfect (we already have many issues with FireOS). I therefore want to keep the default playback experience unmodified.
| syncPresetButtons(activeBtn) | ||
| } | ||
|
|
||
| syncPresetButtons(null) // none selected by default |
There was a problem hiding this comment.
Please automatically highlight the selected button based on the preset. Reopening the dialog removes the visual selection.
|
Great job on the pull request! |
Adds a real-time dynamic range compressor accessible via a new
"Compressor" button next to Tracks in the player controls.
Ported from VLC's compressor.c (LGPL, Steve Harris / Ronald Wright).
Implemented as a Media3 AudioProcessor injected into DefaultAudioSink
— sits directly in the audio pipeline, zero overhead when disabled.
Controls: threshold, ratio, attack, release, makeup gain.
All adjustable live via sliders with no player reload needed.
Settings are persisted across sessions.
Defaults: threshold -14dB, ratio 4:1, attack 10ms, release 50ms,
makeup +6dB.
Works with both nextlib and default decoder paths.
Full TV remote navigation. Correctly included in isDialogOpen().
This PR was developed with AI assistance (Claude by Anthropic) for implementation and bug hunting. All code has been tested on a real Android TV device and a phone.