thebigg73 / OpenSongTablet

Android port of OpenSong. Use your mobile device as a portable song book. Gareth Evans
GNU General Public License v3.0
32 stars 23 forks source link

[v6 beta 9] Bad start of Mediaplayer for pads #196

Closed iv-gha closed 1 year ago

iv-gha commented 1 year ago

The media player does not start every so often with error -38 in logcat when it tries to do getDuration().

It seems this is a symptom of the player not being fully prepared even though doPlay, with the start() in, is called by setOnPreparedListener in loadAndStart.

v6 seems to create new mediaplayers all the time, did v5 do things differently?

I cannot get pads and pad cross fade stable until there is reliability of starting a pad using loadAndStart. Please take a look at getting it reliable.

If media player start is fixed then I have the code, using v5 approaches, almost there to make pads and pad cross fade work reliably in v6.

For v6 beta 9 pads and pad info display are very unstable.

I am feeling a bit of deja-vu, pad fixes were one of the first pulls I offered to the project - with it reliable since 5.1.8. We can sort it!

Cheers Ian.

iv-gha commented 1 year ago

I have add https://github.com/thebigg73/OpenSongTablet/pull/197 which handles this by an 'on error' handler. Please see if the fail of Mediaplayer error can be avoided, it remains worrying!

It seems to be the async prepare that is failing and so doPlay is not called. So I guess it is making sure there are no problems with how the prepare is called.

After research I see that 'detect the fail and do something about it' is used by others!

iv-gha commented 1 year ago

I have adjusted to use the v5 approach of reuse of persistent MediaPlayer instances. I have not seen the fail so far! We shall see.

thebigg73 commented 1 year ago

Good work Ian! I'll pull this and test. I was planning on adding in a boolean for 'onPrepared' to act as an additional check before trying to get the duration. I might leave this in. Any bad call to the mediaPlayer puts it's state as an error. Your onError handler should also catch this attempted change of state.

thebigg73 commented 1 year ago

So far so good. I haven't come across any issues.

iv-gha commented 1 year ago

I have spotted a need to gracefully handle a stop when a pad is paused - I have a fix. I also have applied the v5 fix to pad settings - which applies settings changes to a running pad. I will offer in due course.

I think this can be closed. Thanks.