Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

media plugin: success and error callbacks removed in v3+ #1333

Closed
ghenry22 opened this issue Apr 6, 2017 · 17 comments
Closed

media plugin: success and error callbacks removed in v3+ #1333

ghenry22 opened this issue Apr 6, 2017 · 17 comments
Labels

Comments

@ghenry22
Copy link
Contributor

ghenry22 commented Apr 6, 2017

While the Status update callback can still be specified the success and error callbacks cannot with version 3+ of ionic native.

This means we cannot catch errors during media creation nor can we catch the playback complete event which calls the success callback once a file has played to completion.

create method should be updated to take an (optional) function for onSuccess and onError as well as the current onStatus. This keeps it in line with the underlying plugin as well.

If I am just missing something here let me know, I am just working on updating to ionic-native 3.0

@gmapis
Copy link

gmapis commented Apr 6, 2017

I'm experiencing the same issue! I'm is the process of rolling back because if it!

@ihadeed
Copy link
Collaborator

ihadeed commented Apr 14, 2017

Should be fixed in the next release.

constructor(private media: MediaPlugin) {}

...

this.media.create('file.mp3')
  .then((file: MediaObject) => { ... })
  .catch(e => console.log('Error opening file', e));

@ghenry22
Copy link
Contributor Author

The problem is in the then method you are returning the media object that is needed to control playback and this returns as soon as create is successful, but with the bare plugin the success callback is only called on completion of playback of the current file so it's quite a different behaviour.

You can use the status updates to track media status sure but it means that anyone who was relying on the success callback to indicated that playback has finished will have to adjust their app.

@ghenry22
Copy link
Contributor Author

It's neither right nor wrong i guess, just changes the implementation.

@ihadeed ihadeed reopened this Apr 15, 2017
@ihadeed ihadeed added the bug label Apr 15, 2017
@ihadeed
Copy link
Collaborator

ihadeed commented Apr 15, 2017

@ghenry22 yes, you're right. I thought the success callback was called when the media object is successfully created. This needs to be fixed.

Thanks for pointing it out.

@ghenry22
Copy link
Contributor Author

ghenry22 commented Apr 17, 2017

@ihadeed I actually think that the way it is implemented here is a lot more sensible. Maybe the output of that call back could be made available in a "playback complete" event attached to the mediaObject that users could subscribe to?

Then all the original functionality of the plugin is available but the way that you use it makes a lot more sense in that create returns when create is successful rather than when playback has completed.

That would help to accommodate legacy apps with minimal changes and if you want more detailed playback status you use onStatus callback.

@baksagimm
Copy link

Seems the update of mediaplugin 3.5.0 don't fire the promise. Which it was a problem in previous release.

@ghenry22
Copy link
Contributor Author

@ihadeed - I'm not sure what has happened but some changes have been made that have now resulting the media plugin being quite broken in 3.6.0.

Looking at the docs on the ionic site for media, if I follow the documentation with version 3.4.4 of the plugin everything works as expected.

If I then just update to 3.6.0 in package.json I immediately get TS throwing errors and aborting builds.

I would suggest that you revert the media plugin code to be as it was in v3.4.4 as this at least makes the media plugin work and the documentation consistent with the plugin. More thought needs to be put into this before chopping and changing the way something essential like the create function works.

@ihadeed
Copy link
Collaborator

ihadeed commented Apr 27, 2017

@ghenry22 sorry I forgot to update the documentation. The usage now is as follows:

const file: MediaObject = this.media.create('file.mp3');
file.record(); 

.create() method takes 3 optional callbacks, onProgress, onSuccess and onError. Full example would be:

const onProgress = (...args) => console.log('media progress', args);
const onSuccess = (...args) => console.log('Media success', args);
const onError = (...args) => console.log('Media error', args);

const file: MediaObject = this.media.create('file.mp3', onProgress, onSuccess, onError);
file.record(); 

@ghenry22
Copy link
Contributor Author

I saw that when I checked the current source code to see what was up. Whenever trying to specify the extra callbacks for onSuccess and onError though I get a TS error that this doesn't match the signature of the create method, it seems to still be expecting only src and OnStatusUpdate as arguments.

Perhaps typings need to be updated as well to resolve that? If you can check I'll give updating another try.

@ihadeed
Copy link
Collaborator

ihadeed commented Apr 27, 2017

Is the error emitted by your IDE or the CLI?

@ghenry22
Copy link
Contributor Author

that's in the IDE it's just VSCODE, all other ionic and ionic-native stuff works fine. I'll give it another try tomorrow if you think it's just the IDE freaking out over nothing.

@ihadeed
Copy link
Collaborator

ihadeed commented Apr 27, 2017

I just had a look at the new definitions. It's possible that the callbacks you provided do not match the callbacks expected by function. They're not that flexible now.

Can you share your code so I can have a look?

@ghost
Copy link

ghost commented Apr 28, 2017

    const onProgress = (args) => console.log('media progress', args);
    const onSuccess = (args) => console.log('Media success', args);
    const onError = (args) => console.log('Media error', args);

    const file: MediaObject = this.media.create('assets/media/朋友的酒.mp3', onProgress, onSuccess, 
    onError);
    file.setVolume(1);
    file.play();
    file.startRecord();

I have a problem!
image
any idea?
thank you so much for your plugin.

@danielehrhardt
Copy link
Contributor

Same Problem as @litsun here

@ronaldmak
Copy link
Contributor

@litsun The 1st error occurs because method "setVolume" is called before method "play" is called. It is not allowed. The 2nd error occurs because method "startRecord" is called while the audio is playing.

@ghenry22
Copy link
Contributor Author

this is resolved in 3.8.0 plugin is back to working as normal with all expected call backs and the docs are updated to be accurate as well.

Tested and working on android and iOS with my apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants