[TIMOB-13704] Android: Added setTime() method to android/modules/media/src/java/ti/modules/tita...

[sheran]

This is my contribution to adding a setTime() method to AudioPlayer. Corresponding JIRA ticket is https://jira.appcelerator.org/browse/TIMOB-13704

[hieupham007]

Hi Sheran,
1. setTime in this PR isn't integrated with sound, so it doesn't address https://jira.appcelerator.org/browse/TIMOB-13704. I.e, setTime(55000) would not play the sound at 55s.
2. Putting the property in propertyAccessor will automatically generate getter and setter for the property, so you don't need another @Kroll.setProperty. Also, I think this would crash in scons. You can use propertyChanged in TiUIView instead.

[sheran]

Hey there,

Ok, I understand what you mean. The thing is, I compiled and ran the SDK and this worked for me; no crashes. It may be likely that because I added the propertyAccessor it worked. My modified SDK is working fine for my app and I will continue to use it. I submitted the pull request in the hopes of having it tested and possibly giving back to the community. Thanks for following up with this and getting back to me.

[hieupham007]

He Sheran,
Perhaps you forgot to commit the integration with sound? I don't see how this pull request fixes https://jira.appcelerator.org/browse/TIMOB-13704. Am I missing something here? Generally we don't merge a pull request unless it fixes a JIRA issue. Thanks for the quick reply :)

[ayeung]

Adding the Kroll.setProperty does not crash the build. It would just overwrite the setter. However, it probably isn't necessary like @hieupham007 said since it's handled inside propertyChanged of TiSound.

Anyways, I tested the PR, and the setTime() method isn't working as expected. It doesn't seem to skip to a particular time in the stream. For me, it just always starts off from the beginning.

[sheran]

Hmm, ok. Not sure why this happens to you guys. Just before writing this, I used my modified SDK and tried again and it works fine. I have not tested playback from a local source, but I've tried streaming and it works. Here's the code I use:

function doPlay(e) {
    audioPlayer.addEventListener('change', handleChange);
    audioPlayer.url = "http://tenminutepodcast.com/Audio/BitKillerJones.mp3";
    audioPlayer.setTime(55000);
    audioPlayer.start();
}

This is in my alloy.js

var audioPlayer = Ti.Media.createAudioPlayer({ 
    allowBackground: true
});

As I said before, I'm happy with the results I have so far. I didn't know that adding the property into the propertyAccessor automagically created the getter and setter, so I will remove my explicit setter and test it as well. Thanks for letting me know about that.

[ayeung]

Ok, just tested it with the new test case and it works great. If you could remove the setTime() method, I would be happy to merge in this PR.

[sheran]

@ayeung I've removed the setTime() method now. Sorry, didn't see your message earlier.

[hieupham007]

Code reviewed and functionally tested. Request accepted