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

Added fp.exists() false negative shield. #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agamemnus
Copy link

The Media API doesn't account for cases where fp.exists() may provide a false negative. (For example, APK expansions...)

Cosmetic: I also did a short-circuit on the first condition, converted to "} else {"s instead of hanging brackets, and fixed the incorrect spacing in the last few lines. (caused probably by the hanging brackets!)

The Media API doesn't account for cases where fp.exists() may provide a false negative. (For example, APK expansions...)

Cosmetic: I also did a short-circuit on the first condition, converted to "} else {"s instead of hanging brackets, and fixed the incorrect spacing in the last few lines. (caused probably by the hanging brackets!)
@agamemnus
Copy link
Author

Actually, I am not entirely sure why FileInputStream fileInputStream = new FileInputStream(file) isn't used for the second case of Environment.getExternalStorageDirectory().getPath() + "/" + file... was that a bug, also?

@clelland
Copy link
Contributor

It may have been a bug; I like the way that you've done it, though, regardless.

Do you have an ICLA on file with Apache? I can't tell by your Github username. Also, I'd like to have an issue on our JIRA tracker (https://issues.apache.org/jira) for it for future reference.

Thanks!

@agamemnus
Copy link
Author

Gee, that ICLA thing is pretty formal. I'll email that ASAP...

@agamemnus
Copy link
Author

I sent in a signed and scanned ICLA to Apache's secretary email.

@agamemnus
Copy link
Author

I got the email back saying it was received.

https://issues.apache.org/jira/secure/Dashboard.jspa doesn't want to load.

@vispo
Copy link

vispo commented Mar 16, 2015

I am still in shock reading that you can't use Android expansion files with the Cordova Media plugin to play sounds. I have a 150mb project that uses a lot of sound and I've developed it using the Media plugin. Is it true that you can't use the Media plugin with Android expansion files??? What to do???

@pniraula
Copy link

@vispo and there seems to be some problem with android 5.0 and media plugin. I am working in a project which uses media plugin to play audio. The application works well in all versions of android but 5.0.

@agamemnus
Copy link
Author

vispo, you can use Crosswalk and do xwalk --disable-gesture-requirement-for-media-playback in [root]\platforms\android\assets\xwalk-command-line.

@vispo
Copy link

vispo commented Mar 16, 2015

Have never heard of Crosswalk before. I see it at https://crosswalk-project.org . Is it a Phonegap plugin or something entirely separate from Phonegap?

@vispo
Copy link

vispo commented Mar 16, 2015

I hear that Amazon allows Android apps up to 150mb in size. I could probably get away without having to use an expansion file if we distribute it through Amazon rather than the Google Play Store. Have you distributed an Android app through Amazon? Is it OK? Limitations?

@agamemnus
Copy link
Author

(To clarify, you don't need the Media plugin if you use the --disable-gesture-requirement-for-media-playback switch.)

I have not tried using Amazon to publish my game.

Crosswalk is a system that adds a standalone version of Chromium to your Android Cordova app or game. It's only a few steps to use it via the "migrate" option. Crosswalk is well-supported by Intel and a growing base of users.

P.S.: if you do use Crosswalk and expansion files, I made an Android expansion file plugin here: https://github.com/agamemnus/cordova-plugin-xapkreader.

@cordova-qa
Copy link

Cordova CI Build has one or more failures.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 8.1 Store Link Link Link
Windows 10 Store Link Link Link
Windows 8.1 Phone Link Link Link
iOS Link Link Link
Android Mac Link Link Link

@agamemnus
Copy link
Author

Hard to see.... too many links....

@janpio
Copy link
Member

janpio commented Aug 18, 2018

It's been a loooong time @agamemnus , but can you maybe still explain what

cases where fp.exists() may provide a false negative. (For example, APK expansions...)

actually meant? Can you provide some context on how to test this? Thanks.

@agamemnus
Copy link
Author

You'd have to install both my plugin and that one and you will see that the test identifies the file as not existing when it actually does, IIRC. Problem is, Google has been very bad in supporting APK expansions and they keep making changes that makes it very difficult to maintain.

@janpio janpio changed the title Added fp.exists () false negative shield. Added fp.exists() false negative shield. Aug 18, 2018
@janpio
Copy link
Member

janpio commented Aug 18, 2018

Ok, so this is kind of an edge case?
Anyway, I think your code itself is pretty reasonable. Can you describe exactly what is happening with the files in plain English though? From top to bottom of your change...

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

Successfully merging this pull request may close these issues.

6 participants