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

Fixed sampleRate settings to be adapted to Ios #61

Merged
merged 7 commits into from
Apr 1, 2023

Conversation

yama-yeah
Copy link
Contributor

@yama-yeah yama-yeah commented Mar 14, 2023

#62
Modified code so that SampleRate can be changed.
Since the ChangeLog has not been touched, please edit the ChangeLog if you want to incorporate this modification as a new version.

@anarchuser
Copy link
Owner

Hey, thank you for your work. I need a bit more time to look over the changes; I'll get back to you sometime this week. In the meantime, I'd like to ask you to please open an issue describing why you suggest this change. This helps me keep track of changes more easily.
Lastly, feel free to add an entry to the Changelog for 0.6.5, that's what I will publish it as once I've had a look through the code.

@anarchuser anarchuser added the bug Something isn't working label Mar 19, 2023
@yama-yeah
Copy link
Contributor Author

Thanks for the reply.
After lunch I'll get to work!

Copy link
Owner

@anarchuser anarchuser left a comment

Choose a reason for hiding this comment

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

Alright, I've had a look through the changes. A general note upfront: If you leave out all the style changes to files unrelated to your actual issue, you significantly raise the chances of your changes being reviewed quickly.
That said, as also noted below, I will let those style changes pass because it doesn't really matter to me; there is one though I'd like you to change.

Afterwards, I can merge your changes. I cannot evaluate the actual iOS changes, and thus I will trust they work.

example/lib/main.dart Show resolved Hide resolved
Option 1: Add new feature to the app
Option 2: Fix issue with mic_stream not working properly
Option 3: Improve performance of main.dart file
Option 4: Update dependencies in pubspec.yaml
Option 5: Reformat code in main.dart to adhere to style guide.
This commit adds .vscode to the .gitignore file, preventing it from being committed to the repository.
This commit removes the `.vscode/settings.json` file. The file was deleted and there is no longer a need for it in the project.
… conditions

The commit message describes the changes made in the commit. In this case, the code has been simplified by removing unnecessary line breaks and if conditions. Therefore, the appropriate commit message would be "refactor: simplify code by removing unnecessary line breaks and if conditions".
@@ -113,8 +113,7 @@ class _MicStreamExampleAppState extends State<MicStreamExampleApp>
}

void _calculateSamples(samples) {
if (page == 0)
_calculateWaveSamples(samples);
if (page == 0) _calculateWaveSamples(samples);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to one line!

@yama-yeah
Copy link
Contributor Author

Thanks for the review.
I have improved the style.
It appears that if was set to two lines and else if was set to one line as a dart style setting in vscode.
In the future, maybe there will be people like me who are pulled into the vscode settings and the if statement style will not be stable.
I didn't have much time this time, so I dealt with it by turning off automatic formatting on save, but it might be better to let the repository define the formatting settings for you.

@anarchuser
Copy link
Owner

anarchuser commented Mar 30, 2023

Sadly, I don't have the time to set things like autoformattung up. At some point in the future I want to clean up the whole project, but until then, I can only give basic support

@anarchuser
Copy link
Owner

anarchuser commented Mar 30, 2023

I'll merge and publish it some time this week

@anarchuser anarchuser linked an issue Apr 1, 2023 that may be closed by this pull request
@anarchuser anarchuser merged commit 66efe47 into anarchuser:main Apr 1, 2023
@anarchuser
Copy link
Owner

anarchuser commented Apr 1, 2023

Published as mic_stream: 0.6.5.
Thanks for your contribution

@rootux
Copy link

rootux commented Jul 10, 2023

Hey @yama-yeah - sorry for the late response - but I just tested this on the latest ios and it does not seems to work

@yama-yeah
Copy link
Contributor Author

As shown here IOS does not guarantee a change in sampling rate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sample Rate changes were not applied in IOS.
3 participants