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 |
|
fixing minor error |
Updated string formatting for compressor_on_format to use positional arguments.
|
Waiting for re review |
fire-light42
left a comment
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! |
Added dynamic hints for compressor threshold and makeup gain sliders.
Refactor buildAudioSink to only use custom sink when compressor is provided.
Refactor compressor dialog handling and update UI elements for better readability and maintainability.
|
Fixed all points from the review:
Also fixed a build-breaking XML tag mismatch and a string formatting warning from CI. |
|
fixed typo |
Suppress deprecation warning for flush() method.
|
done! |
|
I will try to review tomorrow 👍 |
|
Whoops note for firelight I tired to make it competible with the new ui and the features option is gone I'm outside right now I cannot fix it can u do it for me while porting to new version |
|
Compatibile* |
Removed the 'compressor_enabled_key' string from resources.
Added newline characters for better formatting in legal notice text.
|
Trying to fix it i cannot figure out it |
This branch has not been deployed
2 participants
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.