solve the memory leak on Android and avoid the crash on kitkat #1328
Conversation
…ll mp.selectTrack(0);
| @@ -565,7 +572,10 @@ public void run() { | |||
| } | |||
|
|
|||
| // Select track (so we can use it to listen to timed meta data updates) | |||
| mp.selectTrack(0); | |||
| try{ | |||
There was a problem hiding this comment.
Maybe you should add a comment why this try catch is there for future contributors
There was a problem hiding this comment.
for avoiding crash when drag the video progress on some old android phone. It will throw error code -1010, this error code was define in MediaPlayer class as below:
/** Bitstream is conforming to the related coding standard or file spec, but
* the media framework does not support the feature. */
public static final int MEDIA_ERROR_UNSUPPORTED = -1010;
There was a problem hiding this comment.
@cobarx What do you think? IMHO this should be added as a code comment to not confuse contributors in the future!
There was a problem hiding this comment.
@n1ru4l Absolutely, any special conditions like this should always be commented. Otherwise, it's impossible to refactor them later since you can't know if they're still relevant.
I will get this tested and merged tomorrow. Sorry about the delay, I've had very little time to work on the project lately.
|
Please test, review, and merge. I'm getting this as well - 10% of my users are getting this crash |
|
@evanjmg Sorry to hear about your issues! In such an urgent case you still have to option to either use https://www.npmjs.com/package/patch-package or fork! |
|
already forked |
|
Ok, I looked through this code and it seems like blindly selecting track 0 isn't useful unless track 0 happens to contain timed metadata. So I've made it look to see if there is even any timed metadata and only select those tracks. I can't find a sample stream to test this, so I'm just going to cross my fingers and hope this works properly. @RWOverdijk any chance you can test the updated code? I'm going ahead and merging this. @evanjmg and @linguokun1 would you mind testing the updated code to make sure it fixes the issue for you? |
|
Nice refactor. So far so good; it’s an improvement for me. I’ll let you know if I have any more issues. |
|
@cobarx I have used the code I modifing in my project, It's ok and no problem till now. |
solve the memory leak on Android and avoid the crash on kitkat (rebased from commit 52334f0)
Solve the memory leak on Android and avoid the crash on Xiaomi Phone M4's kitkat rom when call mp.selectTrack(0);