-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
[Feature] Ability to extract files simultaneously #12
Conversation
android/src/main/java/com/ryanheise/just_waveform/JustWaveformPlugin.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/ryanheise/just_waveform/JustWaveformPlugin.java
Outdated
Show resolved
Hide resolved
Going forward, I wonder whether it would be better to generate a unique ID for each instance rather than use the file name. The plan is to eventually include partial results of the waveform data in the progress so that eventually you might not need the file. |
Ok, will change to ID |
Before you do that, I haven't actually thought it through completely, just thinking out aloud. This may be like a job ID. |
android/src/main/java/com/ryanheise/just_waveform/JustWaveformPlugin.java
Outdated
Show resolved
Hide resolved
@@ -121,7 +121,7 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { | |||
UInt32 scaledSampleIdx = 0; | |||
int progress = 0; | |||
|
|||
while (frameCount > 0) { | |||
while (frameCount > 0 && progress < 100) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, what is the scenario where the second condition is required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes the progress never become 100, kind of rare times. may be frame count cause the problem. Its difficult to find
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your condition wouldn't help if the progress never became 100. Maybe you meant that frameCount never becomes 0?
This looks great! thank you guys for the great work. |
That is something that can/should be implemented by the app. (I like to keep things simple)
At this stage my plan is to eventually merge this PR. Before I do that, I'm just thinking about whether we want to have a unique ID for each instance generated by uuid or just use the file path as the unique ID. (Again just thinking of simplicity.) |
I think we should avoid the path because of duplication. If the same widget is rendered twice this would cause an error. Where using uuid even if there are duplicates the waveform would be extracted. |
I would consider it a misuse of the plugin to simultaneously write to the same path, so the path should be unique among the simultaneous requests. |
It is a misuse, but I still think UUID would be the better approach since it's not prone to as many problems as the path approach down the line. And it would stop people from opening issues when they misuse the plugin, instead, it would just make their apps a little bit slower. |
This problem is not a a real problem since it can be documented and an exception can be thrown if the same output file is being used in two simultaneous requests. You mentioned there are more problems, though? |
I'll add to the above that using a UUID does not actually solve the problem of using the same output file in simultaneous requests - an exception would be needed. |
This PR has now been merged and released, and I have done some minor cleanup and removed the uuid key in favour of the output filename. Thanks, @AlexSmirnov9107 for the contribution. |
Fix or Feature to #10