Recording: add true mono recording support [DRAFT] - #15947
Conversation
daschuer
left a comment
There was a problem hiding this comment.
Cool, this is already a nice Draft. I left some comments for polishing.
|
As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future. |
|
Oh also the checks that have failed I think are due to a warning that was also thrown in my code but I ignored as it was a warning i didnt know it would cause these checks to fail but it's basic reordering of lines to change order of implementation i guess so i'll do that too. And in the meanwhile I have filled out the contributor agreement. |
|
The GitHub workflow fails because of code style issues. You can completely get around the hassle with code style by installing pre-commit locally. It will fix these issues on the go: |
|
There are still code style issues. Do you have pre commit installed now? If yes, you can fix the code style for all commits by (untested) Git will stop if there is an formating issue and fix it in the working copy. You can apply it by Finally |
…ror on building after changing base initEncoder() fucntion to accept number of channels
…ne audio output and recording scenario
2fa28b2 to
b446d68
Compare
|
Did you try, the suggestion? Has it failed? I wonder why there is a final code style fix commit. That's not an issue, I am just curious. |
Yeah just tried the rebase and force pushed, the last fix code styles issues commit is a previous one i tried but it is done again due to the rebase. |
Is it possible that the code style check is failing in the UI file which i changed but wasnt able to get my VSC to match the spacing already present in the file and used normal tabs? Because I dont see precommit fixing that. I am really sorry for not taking of pre-commit earlier which could have avoided this entire fuss. |
daschuer
left a comment
There was a problem hiding this comment.
I have tested Wave recording and it works good. Thank you.
We have a EncoderBroadcastSettings::getChannelMode(); function that return Mono or stereo. I think this need to be used to avoid redundant channel info.
f22062e to
1667a4a
Compare
1667a4a to
0d615b3
Compare
|
Hi! I've attempted to fix the code style issues and rebased on main. Let's see if the CI Check passes? If this still fails, I'm happy to create a fresh PR with a clean commit history. Let me know which approach you'd prefer. |
|
Hi Daniel! I have opened a new draft PR #15977 for cleaner commit history and easier to pass CI code style check due to no non pre-commit commits. Please allow workflow approval on that and once that passes CI Checks, I will close this PR |
|
Hi Daniel! I am closing this PR and opening a new one to avoid commit history before pre-commit and make the passing of the CI Check easier. Thank you :) |
This PR implements True Mono Recording Support as detailed in Issue #15180 . I have, after discussions with the community and maintainers, implemented an Independent Recording output option. So if the user wants the audio output in stereo but the recording output in Mono, that is possible (or any of the other three permutations).
The PR is opened as Draft because encoder-specific validation is still in
Progress and community feedback is requested, especially for compressed formats.
Also, This is the GUI Change that will take place due to the recording output preference added:

Before:
After:

Also I havent still setup precommit, I'm happy to set it up and publish the PR if needed.