Permalink
Browse files

front: Test header when searching for ROMS, rather than file extension.

Much more reliable, allows arbitrary filenames.
  • Loading branch information...
littleguy77 committed Jan 13, 2015
1 parent 7fc4354 commit 9eb5796979ed41e04c76e7c08137b8be356116c6
Showing with 5 additions and 2 deletions.
  1. +5 −2 src/paulscode/android/mupen64plusae/task/CacheRomInfoTask.java
@@ -33,6 +33,7 @@
import paulscode.android.mupen64plusae.persistent.ConfigFile;
import paulscode.android.mupen64plusae.persistent.ConfigFile.ConfigSection;
import paulscode.android.mupen64plusae.util.RomDatabase;
import paulscode.android.mupen64plusae.util.RomHeader;
import paulscode.android.mupen64plusae.util.RomDatabase.RomDetail;
import android.os.AsyncTask;
import android.text.TextUtils;
@@ -139,8 +140,10 @@ protected void onCancelled( ConfigFile result )
}
else
{
String name = searchPath.getName().toLowerCase( Locale.US );
if( name.matches( ".*\\.(n64|v64|z64|zip)$" ) )
// TODO: if name ends in zip, extract, then search contents

This comment has been minimized.

Show comment
Hide comment
@paulscode

paulscode Jan 13, 2015

Member

This could happen in RomHeader around line 179. If file has the zip extension, create a ZipInputStream and read the header directly (versus extracting the whole file). Would be good to have the core support something like that as well, so no more need to unzip ROMs.

@paulscode

paulscode Jan 13, 2015

Member

This could happen in RomHeader around line 179. If file has the zip extension, create a ZipInputStream and read the header directly (versus extracting the whole file). Would be good to have the core support something like that as well, so no more need to unzip ROMs.

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 Jan 13, 2015

Member

Yep, working on the zipinputstream stuff now actually. Getting some out of memory crashes though, so tracking that down.

Core support might be nice but I don't want to write that code :P

@littleguy77

littleguy77 Jan 13, 2015

Member

Yep, working on the zipinputstream stuff now actually. Getting some out of memory crashes though, so tracking that down.

Core support might be nice but I don't want to write that code :P

This comment has been minimized.

Show comment
Hide comment
@paulscode

paulscode Jan 13, 2015

Member

If we wanted to get really crazy, we could even avoid looking at the file extension entirely, and check if the first int in the stream is 0x504b0304 (the ZIP signature)

@paulscode

paulscode Jan 13, 2015

Member

If we wanted to get really crazy, we could even avoid looking at the file extension entirely, and check if the first int in the stream is 0x504b0304 (the ZIP signature)

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 Jan 13, 2015

Member

Awesome, I was wondering what that was. I much prefer that to the file extension.

@littleguy77

littleguy77 Jan 13, 2015

Member

Awesome, I was wondering what that was. I much prefer that to the file extension.

This comment has been minimized.

Show comment
Hide comment
@paulscode

paulscode Jan 13, 2015

Member

I'll take a look at adding direct zip reading to the core, and we can submit a pull request if it works well. I was thinking one way to do that would be to read the entries until you come to the first recognized ROM in the archive (based on the header). On the other head, it might be faster to just check the first file in the archive, but I think some ROMs tend to have a text file as the first entry. How were you thinking of handling potential multi-file archives in the RomHeader class?

@paulscode

paulscode Jan 13, 2015

Member

I'll take a look at adding direct zip reading to the core, and we can submit a pull request if it works well. I was thinking one way to do that would be to read the entries until you come to the first recognized ROM in the archive (based on the header). On the other head, it might be faster to just check the first file in the archive, but I think some ROMs tend to have a text file as the first entry. How were you thinking of handling potential multi-file archives in the RomHeader class?

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 Jan 13, 2015

Member

I was simply going to extract any ROM found in the zip file, at any depth, to the appropriate directory under GameData. A bit of chicken and egg since that folder is based on MD5. So I extract to temporary location, compute MD5, then move the file to the GameData subdirectory. Alternatively I could just have an "ExtractedRoms" folder or something and not bother co-locating it with the game-specific data.

@littleguy77

littleguy77 Jan 13, 2015

Member

I was simply going to extract any ROM found in the zip file, at any depth, to the appropriate directory under GameData. A bit of chicken and egg since that folder is based on MD5. So I extract to temporary location, compute MD5, then move the file to the GameData subdirectory. Alternatively I could just have an "ExtractedRoms" folder or something and not bother co-locating it with the game-specific data.

RomHeader header = new RomHeader( searchPath );
if( header.isValid )
result.add( searchPath );
}
return result;

4 comments on commit 9eb5796

@xperia64

This comment has been minimized.

Show comment
Hide comment
@xperia64

xperia64 Jan 14, 2015

Contributor

Main issue I could see with reading the zip header is falsely picking up APKs

Contributor

xperia64 replied Jan 14, 2015

Main issue I could see with reading the zip header is falsely picking up APKs

@paulscode

This comment has been minimized.

Show comment
Hide comment
@paulscode

paulscode Jan 14, 2015

Member

That was my main concern WRT whether or not to traverse the entire ZIP file or not. Scanning every APK could potentially be very slow if you need to check the header of every file inside it. Probably need to do some benchmarks to see if it is an issue IRL or not, though.

Member

paulscode replied Jan 14, 2015

That was my main concern WRT whether or not to traverse the entire ZIP file or not. Scanning every APK could potentially be very slow if you need to check the header of every file inside it. Probably need to do some benchmarks to see if it is an issue IRL or not, though.

@littleguy77

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 Jan 14, 2015

Member

Good point. If checking zip files is really slow, we could add a "Search zipped files" checkbox or something before it scans. The built-in java unzip methods actually seems pretty fast. I only extract an entry if its first 4 bytes match the ROM signature. So it will only "peek" into APKs. Will try to benchmark it.

Member

littleguy77 replied Jan 14, 2015

Good point. If checking zip files is really slow, we could add a "Search zipped files" checkbox or something before it scans. The built-in java unzip methods actually seems pretty fast. I only extract an entry if its first 4 bytes match the ROM signature. So it will only "peek" into APKs. Will try to benchmark it.

@littleguy77

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 Jan 14, 2015

Member

BTW unzipping this way is implemented. I just want to touch it up a bit before pushing (lots of try/catch/finally statements... hard to make them pretty).

Member

littleguy77 replied Jan 14, 2015

BTW unzipping this way is implemented. I just want to touch it up a bit before pushing (lots of try/catch/finally statements... hard to make them pretty).

Please sign in to comment.