-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[Bugfix] Add missing early return in AudioSubsectionReader::readSamples() #1248
Closed
danra
wants to merge
2,307
commits into
juce-framework:develop
from
soundradix:fix_subsection_reader
Closed
[Bugfix] Add missing early return in AudioSubsectionReader::readSamples() #1248
danra
wants to merge
2,307
commits into
juce-framework:develop
from
soundradix:fix_subsection_reader
Conversation
This file contains 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
…files) - all systems that supports C++11 are relying on AudioUnit v2 APIs instead" Didn't get why this change was needed for ARA. Probably will find out when also building AUs. This reverts commit 8eb74fd.
…juce) Also got rid of changes related to RTAS which is deprecated! Also got rid of StickyMidiKeyboard, can return if we want but better implement internally, to reduce conflicts.
We may now choose to merge celemony and JUCE branches there and then merge to it. This way we can more easily keep track of our changes that did not yet make it into JUCE.
# Conflicts: # README.md # extras/Projucer/Builds/MacOSX/Projucer.xcodeproj/project.pbxproj # extras/Build/CMake/JUCEUtils.cmake # extras/Projucer/Builds/MacOSX/Projucer.xcodeproj/project.pbxproj # extras/Projucer/JuceLibraryCode/BinaryData.cpp # extras/Projucer/JuceLibraryCode/BinaryData.h # extras/Projucer/Source/ProjectSaving/jucer_ProjectExport_CLion.h # extras/Projucer/Source/ProjectSaving/jucer_ProjectExport_Xcode.h # modules/juce_audio_plugin_client/AU/juce_AU_Wrapper.mm # modules/juce_audio_plugin_client/AUResources.r # modules/juce_audio_plugin_client/VST3/juce_VST3_Wrapper.cpp # modules/juce_audio_plugin_client/juce_audio_plugin_client.h
All changes there should already be merged here so made this a do-nothing rather than resolving the nested conflicts.
This makes sure the commits in that branch won't be lost even after we delete the branch, which will allow the intermediate steps migrating AAP from juce6 to juce7 would still be able to be reviewed and built.
On top of the automatic merge, needed to maintain the rename of getHostType in changes in our branch in juce_AU_Wrapper
…move to them as proper rename
Merge conflicts were resolved using helper branch where renames became proper renames. Not merging further at the moment due to https://forum.juce.com/t/macos-build-broken-if-enabling-push-notifications/55389
…icolons Before this tweak, a juce plugin target which had a target_compile_definitions entry with an escaped semicolon would end up generating format-specific targets which not only had a preprocessor definition matching the entry, but also extraneous definitions of the escaped-semicolon-split parts of that entry. For example, an entry of "MY_MACRO=f()\;g();" would end up with the extraneous preprocessor definitions MY_MACRO=f() and g(), in addition to the desired MY_MACRO=f();g(); This is because of how CMake handles escaped semicolons in lists, see e.g. https://gitlab.kitware.com/cmake/cmake/-/issues/20317
…es() Looks like a very old regression since 27895cb In case clearSamplesBeyondAvailableLength() returns a negative number of samples, we need to avoid calling readSamples() on the source reader. One reason is that it's possible the source reader readSamples() will also call clearSamplesBeyondAvailableLength(), which can end up calling zeromem() with that negative number of samples, causing a crash. (that would happen in case samplesAvailable's value would be an even lower negative number, causing the condition `(samplesAvailable < numSamples)` to evaluate to true).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Looks like a very old regression since 27895cb
In case
clearSamplesBeyondAvailableLength()
returns a negative number of samples, we need to avoid callingreadSamples()
on the source reader. One reason is that it's possible the source readerreadSamples()
will also callclearSamplesBeyondAvailableLength()
, which can end up callingzeromem()
with that negative number of samples, causing a crash. (that would happen in casesamplesAvailable
's value would be an even lower negative number, causing the condition(samplesAvailable < numSamples)
to evaluate to true).