Media onStatusUpdate maybe not running in Angular zone

[rapropos]

[x] bug report

Current behavior:
Changes made to controller properties from within a MediaOnStatusChange callback are not reflected in templates.

Expected behavior:
onStatusChange handlers should be able to have their activity automatically reflected in templates.

Other information:
See this forum thread for sample code illustrating the bug. Triggering change detection manually seems to work, and I suspect zone.run() would as well. I don't know if this is something that ionic-native shims can do for us or not.

[ghenry22]

Yes I have to wrap code in the obstatusbupdate in zone.run otherwise no change detection as you say. Same in the onSuccess callback (and I have not tested but presume on error will have the same issue)

[ihadeed]

This is happening because we're just passing the callback as-is to the plugin, we're not converting it to a promise/observable. It's possible to fix it, but I think it's better to have less Angular specific code in Ionic Native. I recommend wrapping it with ngZone.run(() => { ... }) manually.

[ihadeed]

My response from the forum:

On a second thought...

We can solve this issue without adding Angular specific code in Ionic Native (ngZone.run() ... etc).

We can change the definitions of the Media plugin to work as follows:

let file: MediaObject = this.media.create('file.mp3');
file.onSuccess().subscribe(() => { ... }); // replaces onSuccessCallback
file.onError().subscribe(() => { ... }); // replaces the onErrorCallback
file.onStatusUpdate().subscribe(() => { ... }); // replaces onStatusUpdate

This means that the .create() function will no longer take the callbacks as parameters. Ionic Native will create those callbacks and converts them into Observables since they emit data more than once. I can probably do this in a way that doesn't break old apps... (the backwards compatibility will be removed in Ionic Native 4.x)

[ghenry22]

onSuccess and onError only trigger once and would be better suited as promises rather than observables.

onStatus could be implemented as an observable.

But to be honest, the media plugin as it is now works just fine, it's simple to use just needs a note in the docs to wrap the return functions in ngzone.run() and everything is good.

This might be a better long term solution but the benefits are minimal.

[rapropos]

just needs a note in the docs to wrap the return functions in ngzone.run()

I will defer to the eventual decision here, but cannot disagree more strongly with this. I see ionic-native as a way to ensure that app code never needs to explicitly deal with Angular zones. Especially if the eventual goal is to allow replacing Angular with other frameworks, it makes it all the more crucial that we do not encourage app code to ever manually interact with zones.

[ghenry22]

Sure but I also think that stability is important here. The media plugin has had several changes and some breakages in the last few versions and I feel that keeping things stable for a while would be a good think for users. This change chould certainly be planned for the next major version and delivered with proper migration instructions and details on the breaking changes though. This would give the short term stability as well as serving the long term goal and providing a clearly documented upgrade path.

[ihadeed]

According to this https://cordova.apache.org/docs/en/latest/reference/cordova-plugin-media/#parameters

The success callback gets called when play, record or stop action has completed. I assume the error callback is called whenever an error has occurred in any of these functions. It's kinda tricky. Not sure why don't they just have callbacks for each method instead of using a shared one.

I haven't used the plugin enough to figure out how these callbacks work. I'm just working with what I understand from their docs. I'll have to dig into their source code or test the plugin more to understand how things are supposed to work.

[ihadeed]

Just looked at their source code.. Success and error callbacks are used multiple times.

[ghenry22]

Success is only used once, it is called on completion of playback or recording.

Error I know it is called if there are specific errors creating the media object or starting playback (ie network, unsupported file etc). I can't see any scenario where error would be called more than once as it's generally pretty terminal, but I could be wrong.

Status is obviously called multiple times for any changes in playback or recording state.

Anyway I am happy to test and give input on an updated implementation, I just feel that it's important to keep things stable and well documented. People seem to struggle a lot with file and media operations already with Cordova and they are some of the more common plugins so stability is an important thing for a happy user base :)

[ihadeed]

@ghenry22

From what I see in the source code, they are calling it it every time the media stops: https://github.com/apache/cordova-plugin-media/blob/master/www/Media.js#L209

I guess that's when .play() finishes, or when .stop() or .stopRecording() is called. Error callback is passed as the error callback for almost every method.

[ghenry22]

@ihadeed ah you're right, I was thinking of a one time use of a media file but if someone is going to play the same media file multiple times then onSuccess will be called each time that playback/recording completes and onError could potentially be called on any play request if something goes wrong.

I guess in that case an observable may make more sense.

[ihadeed]

@ghenry22

Please try @ionic-native/media@dev and let me know if it works well.

let file = this.media.file('file.mp3');

file.onSuccess.subscribe(..);
file.onError.subscribe(...);
file.onStatusChange.subscribe(...);

[ghenry22]

should be able to test it out tomorrow, will update you.

[ihadeed]

@ghenry22 did you get a chance to test it? I'm releasing 4.0 soon and I would like to include this.

If you haven't tested it that's fine, I'll run a test when I get a chance.

[teamidevia]

I tried the following code but nothing is in my console. Did I do anything wrong?

 this.audio = this.media.create(this.currentTrack.path)
    
 this.audio.onSuccess.subscribe((data) => console.log(data))
 this.audio.onError.subscribe(data => console.log(data));
 this.audio.onStatusUpdate.subscribe((data) => console.log(data));

[ghenry22]

did you call this.audio.play() at the end? You will need to call play before anything happens.

Also this would be better asked in the ionic forums, this github issue is already closed so you are not going to get much response here.

[mortuz]

Seems like this the problem was in 4.0, in 4.1 it's working. Thanks!