numandev1 / react-native-compressor

🗜️Compress Image, Video, and Audio same like Whatsapp 🚀✨
MIT License
852 stars 85 forks source link

Crash of video compression on Samsung J5 #56

Closed daschaa closed 2 years ago

daschaa commented 2 years ago

We are currently facing an error with the compression on a Samsung J5 device. It runs into an IllegalStateException in the VideoSlimmer.convertVideo() call.

We get the following stacktrace:

    android.media.MediaCodec.native_stop MediaCodec.java
    android.media.MediaCodec.stop MediaCodec.java:2251
    com.zolad.videoslimmer.VideoSlimEncoder.releaseCoder VideoSlimEncoder.java:613
    com.zolad.videoslimmer.VideoSlimEncoder.convertVideo VideoSlimEncoder.java:391
    com.zolad.videoslimmer.VideoSlimTask.doInBackground VideoSlimTask.java:30
    com.zolad.videoslimmer.VideoSlimTask.doInBackground VideoSlimTask.java:11
    android.os.AsyncTask$3.call AsyncTask.java:394
    java.util.concurrent.FutureTask.run FutureTask.java:266
    android.os.AsyncTask$SerialExecutor$1.run AsyncTask.java:305
    java.util.concurrent.ThreadPoolExecutor.runWorker ThreadPoolExecutor.java:1167
    java.util.concurrent.ThreadPoolExecutor$Worker.run ThreadPoolExecutor.java:641
    java.lang.Thread.run Thread.java:923

We found out, that in the com.zolad.videoslimmer package the releaseCoder() method has no proper exception handling if an encoder is already released. This results in the exception bubbling up into the react-native-compressor package.

Is there any chance that we can fix this in this library?

numandev1 commented 2 years ago

@joxw1 nowadays, my office routine is too much busy. I can fix it on weekend

numandev1 commented 2 years ago

@daschaa I have not Samsung j5 device. can you tell me the reproducible step of this error so I can fix it

daschaa commented 2 years ago

I can not tell you exactly the reproducible step because i just saw this issue often in our crash reports. I think the problem lays in the third-party-dependency rather than in this library. I can only reference to the stacktrace given above.

daschaa commented 2 years ago

Ok i have fixed the issue and created my own VideoSlimmer package. As long as the VideoSlimmer package is not maintained, I would propose to use my package here: https://github.com/daschaa/VideoSlimmer/packages/1163123

I can add you as maintainer if you want to. Then you don't have to add the code to your native codebase. Would be cool if we could collaborate there. My plan is to refactor the package completely and add proper testing to it. What do you think @nomi9995 ? 🙌🏼

numandev1 commented 2 years ago

@daschaa Thanks a lot. I already have removed the VideoSlimmer package and maintaining my own. I have a plan to replace this in the future because it is slow.

https://github.com/Shobbak/react-native-compressor/commit/443cefa634d030129de586c3590ddfef9f1af956

numandev1 commented 2 years ago

@daschaa https://github.com/AbedElazizShe/LightCompressor My plan is to integrate this package what do you think?

NiklasLehnfeld commented 2 years ago

@nomi9995 Did you check the license of the mentioned library? That is GPL 3. I am not a lawyer, but I think that would mean that you also have to change your license to GPL 3. And unfortunately I am currently not 100% sure, but I think that would mean that you would lose us as users.

Correct me if I am wrong.

numandev1 commented 2 years ago

@NiklasLehnfeld sorry I did not read. i will revert it soon

numandev1 commented 2 years ago

@NiklasLehnfeld https://github.com/Shobbak/react-native-compressor/commit/671d83dc43aab5d66d8096c5e860165a8ecfc5ac

NiklasLehnfeld commented 2 years ago

Sorry, I am a bit confused. Were you actually using the library? I don't see the dependency in the build.gradle in the last few commits. 🤔

And it was also not used anywhere in the commit you reverted, was it?

numandev1 commented 2 years ago

@NiklasLehnfeld I was copied code directly in this package but then I reverted. now i am using this package https://github.com/nomi9995/VideoCompressor

NiklasLehnfeld commented 2 years ago

@nomi9995 ah okay. Is that now already included in one of the latest releases?

numandev1 commented 2 years ago

@nomi9995 ah okay. Is that now already included in one of the latest releases?

yes, included in new release.

numandev1 commented 2 years ago

I think it is already fixed. feel free to reopen if you see this problem again.

numandev1 commented 11 months ago

fixed in 1.8.2