search

ammarahm-ed / react-native-ijkvideo

A Cross Platform (Android & iOS) video & audio component with react-native-video like API & all formats support based on IJKPlayer FFMpeg 3.4
MIT License
25 stars 9 forks source link

Playback does not stop when refreshing react in dev mode #7

Closed maximilize closed 4 years ago

maximilize commented 4 years ago

When refreshing the react app in dev mode the video keeps playing in background. Seems like there is some cleanup code missing.

ammarahm-ed commented 4 years ago

On android or iOS?

maximilize commented 4 years ago

Android. You can reproduce it easily when a video is playing and you hit "r" in the dev console to refresh the app.

ammarahm-ed commented 4 years ago

@maximilize I will look into this

ammarahm-ed commented 4 years ago

@maximilize I looked into this, and its not happening here. Maybe you are doing something wrong. When I press r the player ejects and the video stops too. send me your code for how you are using the video player so I can see.

maximilize commented 4 years ago

I just tested your code. Sadly it's still not working. Also there is a new bug. When I change the video source, the playback of the other video does not stop. It plays in background and I had to force stop the app.

maximilize commented 4 years ago

I was able to fix this issue, also the other with the non-existing cleanup code on app refresh.

You can find my changes here: https://github.com/maximilize/react-native-ijkplayer/commit/aa543aca9f8af7f5bd97b7ca9a59194217829d78

ammarahm-ed commented 4 years ago

@maximilize I tested it but I was unable to reproduce the refresh issue. Should I wait for your pull-request or make the changes here myself? 

ammarahm-ed commented 4 years ago

@maximilize also i see that you have disabled subtitle-render? any reason for that. However the cleanup seems good. But you have renamed the package back to react-native-ijkplayer so i might not be able to merge your changes since that name is not available on npm if I publish it, which is why the name is changed.

ammarahm-ed commented 4 years ago

However most of the video-player api is exposed on android I think. Almost 90% moreover the code related to equalizer etc. needs to be removed because it is unnecessary and shouldn't be there. Instead a seperate module can handle that more nicely. I am using a simple module in my app for equalizer which i will make public I think. What do you think?

maximilize commented 4 years ago

Separating the equalizer module is a good idea I guess.

Setting "subtitle" to 0 was a wrong decision. In our project we don't need this setting, but of course it's useful for other projects.

I reset my project to your current master and did some small changes on top of it and created a pull request.