-
Notifications
You must be signed in to change notification settings - Fork 159
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 waveform extraction natively part-1 #126
✨ Add waveform extraction natively part-1 #126
Conversation
8e7f1e9
to
44ced5b
Compare
} else { | ||
throw "Failed to start player"; | ||
} | ||
} | ||
setRefresh(forceRefresh); |
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.
Here, setRefresh method calls notifyListeners. So, no need to notify it again. Or you can add conditions on when to notify.
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.
Or you can create 2 different Methods, setRefresh
and _setRefresh
. and implementation of setRefresh will be
void setRefresh() {
_setRefresh();
notifyListeners();
}
and _setRefresh will update the data. You can use _setRefresh inside of this player controller and other users can use setRefresh for manually updating value. Here, _setRefresh will not notify listeners.
extractWaveformData( | ||
path: path, | ||
noOfSamples: noOfSamples, | ||
).then( |
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.
Why are we using then instead of await? remove it if not necessary.
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.
extractWaveformData
is an resource expensive function so we don't want to block the UI until this completes so I'm not using await here.
return null | ||
} | ||
|
||
@RequiresApi(Build.VERSION_CODES.LOLLIPOP) |
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.
Remove lolipop version
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.
And update minSDKVersion to 21 in gradle
private var waveformExtractor: WaveformExtractor? = null | ||
private var noOfSamples = 100 | ||
|
||
@RequiresApi(Build.VERSION_CODES.LOLLIPOP) |
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.
^^
android/src/main/kotlin/com/simform/audio_waveforms/WaveformExtractor.kt
Show resolved
Hide resolved
|
||
func startListening(){ | ||
|
||
func startListening() { | ||
if #available(iOS 10.0, *) { |
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.
Remove this condition if possible.
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.
This condition is required because Timer.schedules timer is available ios 10 or above
44ced5b
to
a8ad979
Compare
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.
LGTM!
CHANGELOG.md
Outdated
@@ -1,3 +1,9 @@ | |||
## 0.2.0 | |||
|
|||
- Fully reworked waveforms from audio file |
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.
change this description with appropriate sentence.
CHANGELOG.md
Outdated
## 0.2.0 | ||
|
||
- Fully reworked waveforms from audio file | ||
- Breaking removed `readingComplete` PlayerState and `visualizerHeight`. With this, added `extractWaveforms` function to extract waveforms. |
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.
format issue
a8ad979
to
a3d2cbe
Compare
No description provided.