Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add No_Hardware as HardwareType to disable radio audio effects #161

Merged
merged 13 commits into from
Sep 10, 2024

Conversation

GeheimagentNr1
Copy link

This pull requests add the hardwareTypeIndex to the "No_Hardware" type of the HardwareType enum and the "No Hardware (Real Radio Affects disabled)" option with value 3 to the hardware select of the settings-modal.tsx, to make it possible to disable radio affects in TrackAudio.

This pull request shall implement the issue: #156
This pull request is dependent on the afv-native pull request: pierr3/afv-native#13

@pierr3
Copy link
Owner

pierr3 commented Aug 26, 2024

Thank you! It's a good idea to put this option in the hardware type, unfortunately this is not how you disable realistic audio effects, so your PR on afv-native is also not correct and won't be merged, there are already functions in afv-native that allow you to disable the effects, namely "SetEnableInputFilters" and "SetEnableOutputEffects" which should both be set to false. If you want to tweak your PR I'll be happy to review it, otherwise I plan to implement this in the next releases.

@GeheimagentNr1
Copy link
Author

I changed the implementation to use the "SetEnableInputFilters" and "SetEnableOutputEffects", but I was unable to test them, because I was unable to build and check the changes because of the following error.

Maybe you have an advice what is going on, or can test it yourself.

LINK : fatal error LNK1104: cannot open file 'PocoFoundationmt.lib' [S:\Develop_Projects\TrackAudio\backend\build\extern\afv-native\afv_native.vcxproj]

@pierr3
Copy link
Owner

pierr3 commented Sep 7, 2024

Yes that's an issue on main on windows at the moment, sorry about that, we will fix it soon. Thank you for the update to the PR, I will review your code.

@pierr3
Copy link
Owner

pierr3 commented Sep 7, 2024

You should be able to build now.

@GeheimagentNr1
Copy link
Author

The build is now possible, and the setting is working, but the flags do not prevent the radio effects.
Do I need to modify another place in the code?

backend/src/main.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@neilenns neilenns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to my specific line comments, you also need to update the default configuration so the default value for the new setting is specified.

src/main/config.ts Outdated Show resolved Hide resolved
@pierr3
Copy link
Owner

pierr3 commented Sep 8, 2024

Did not fully read in but you need to make sure you set those flags on connection and not only when updating the settings. AFV has no concept of a settings file, so you need to set everything before connecting

@neilenns
Copy link
Contributor

neilenns commented Sep 8, 2024

Agree with @pierr3's comment. You probably want to set the settings in this method. It seems to be the shared function that gets called in various places when there's a need to ensure the audio settings are correct.

And add radioEffects to defaultConfiguration and SetRadioEffects to the setAudioSettings
backend/src/main.cpp Outdated Show resolved Hide resolved
backend/src/main.cpp Show resolved Hide resolved
@GeheimagentNr1
Copy link
Author

Change requests should know all be done.
If I forgot something or if something else shall be changed, tell me.

backend/src/main.cpp Outdated Show resolved Hide resolved
src/main/config.ts Show resolved Hide resolved
@neilenns
Copy link
Contributor

neilenns commented Sep 9, 2024

Looks like the latest change has a build failure as well:

/Users/runner/work/TrackAudio/TrackAudio/backend/src/main.cpp:264:5: error: no matching function for call to 'transform'
    std::transform(radioEffects.begin(), radioEffects.end(), radioEffects.begin(), std::tolower);
    ^~~~~~~~~~~~~~
/Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__algorithm/transform.h:23:1: note: candidate template ignored: couldn't infer template argument '_UnaryOperation'
transform(_InputIterator __first, _InputIterator __last, _OutputIterator __result, _UnaryOperation __op)
^
/Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.5.sdk/usr/include/c++/v1/__algorithm/transform.h:33:1: note: candidate function template not viable: requires 5 arguments, but 4 were provided
transform(_InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2,
^

@GeheimagentNr1
Copy link
Author

The lower case build issue is now fixed. If there is a better way, tell me and I will use it.

backend/src/main.cpp Outdated Show resolved Hide resolved
backend/src/main.cpp Outdated Show resolved Hide resolved
@pierr3
Copy link
Owner

pierr3 commented Sep 10, 2024

Well done! Thank you for your help

@pierr3 pierr3 merged commit fe98a69 into pierr3:main Sep 10, 2024
3 checks passed
@GeheimagentNr1 GeheimagentNr1 deleted the disabled_hardware branch September 14, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants