Bug in OJN Parser

[hirandom]

There is a bug with the notes on certain charts like Xepher and Fantazindy(o2ma269400.ojn), in which exist a really large number of notes. After a certain point in time certain lanes just keep displaying a long note till the end of the song when the long note should have ended a long time ago. Basically, the long note doesn't end when its supposed to. It simply doesn't end.

I guess this bug's not so important cause no one really plays such charts, but you never know if this bug will surface in any human playable notecharts. I could send pictures if you give me a way to do so.

FYI: I am on a mac, with java 1.6.0_29 (I believe this is the latest available for mac at this time).

[hirandom]

BTW, I meant Xepher SHD lv 1155 (Xepher.ojn) and Fantazindy SHD lv 378 (o2ma269400.ojn)

[mrcdk]

Hi, yes, we know there are some problems with that, it's because the long note end is before the long note start :S Not sure if it's because the chart being like that or because us doing something wrong XD (It'll be our fault for sure xD)
I will try to find a fix to this but I don't have that much time right now :(

[keigen-shu]

It's definitely the notecharter's fault. I tested Fantazindy[SHD] on LostWave. It crashed when trying to link long notes (a combination of badly made chart and badly crafted code). Apparently 7 long note ends are missing on the chart. On such an occasion, we're supposed to use any available normal note right after the long note starting point as its ending point as defined in BMS's default LN_TYPE.

[mrcdk]

Ummm, I'm trying to fix this but I'm not sure how o2jam manages to fix these charts because I can't play them in the emu :/

I've just made that, if the next event in the same channel isn't a longnote end event, convert it to longnote end. Not sure if it's the correct fix. I'll keep this issue open just in case xD

[hirandom]

I believe after a start of a longnote, you need to ignore all notes in that lane until you reach the end of the hold note. At least, doing that gives the same result as o2mania. Not really sure how I inform you of what changes I made to my local copy, so here is the edited version of the parser... All the lines I added ( I didn't delete any ) are marked with //added this. I edited my copy of the ojnparser to as such:

package org.open2jam.parser;

import java.io.RandomAccessFile;
import java.io.UnsupportedEncodingException;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import org.open2jam.util.Logger;
import org.open2jam.util.CharsetDetector;

class OJNParser
{
private static final String genre_map[] = {"Ballad","Rock","Dance","Techno","Hip-hop",
"Soul/R&B","Jazz","Funk","Classical","Traditional","Etc"};

/** the signature that appears at offset 4, "ojn\0" in little endian */
private static final int OJN_SIGNATURE = 0x006E6A6F;

public static boolean canRead(File file)
{
    return file.getName().toLowerCase().endsWith(".ojn");
}

public static ChartList parseFile(File file)
{
    ByteBuffer buffer;
    RandomAccessFile f;
    try{
        f = new RandomAccessFile(file.getAbsolutePath(),"r");
        buffer = f.getChannel().map(FileChannel.MapMode.READ_ONLY, 0, 300);
    }catch(IOException e){
        Logger.global.log(Level.WARNING, "IO exception on reading OJN file {0}", file.getName());
        return null;
    }

    buffer.order(java.nio.ByteOrder.LITTLE_ENDIAN);

    OJNChart easy = new OJNChart();
    OJNChart normal = new OJNChart();
    OJNChart hard = new OJNChart();

    int songid = buffer.getInt();
    int signature = buffer.getInt();
    if(signature != OJN_SIGNATURE){
        Logger.global.log(Level.WARNING, "File [{0}] isn't a OJN file !", file);
        return null;
    }

    float encode_version = buffer.getFloat();

    int genre = buffer.getInt();
    String str_genre = genre_map[(genre<0||genre>10)?10:genre];
    easy.genre = str_genre;
    normal.genre = str_genre;
    hard.genre = str_genre;

    float bpm = buffer.getFloat();
    easy.bpm = bpm;
    normal.bpm = bpm;
    hard.bpm = bpm;

    easy.level = buffer.getShort();
    normal.level = buffer.getShort();
    hard.level = buffer.getShort();
    buffer.getShort(); // 0, always

    int event_count[] = new int[3];
    event_count[0] = buffer.getInt();
    event_count[1] = buffer.getInt();
    event_count[2] = buffer.getInt();

    easy.note_count = buffer.getInt();
    normal.note_count = buffer.getInt();
    hard.note_count = buffer.getInt();

    int measure_count[] = new int[3];
    measure_count[0] = buffer.getInt();
    measure_count[1] = buffer.getInt();
    measure_count[2] = buffer.getInt();
    int package_count[] = new int[3];
    package_count[0] = buffer.getInt();
    package_count[1] = buffer.getInt();
    package_count[2] = buffer.getInt();
    short old_encode_version = buffer.getShort();
    short old_songid = buffer.getShort();
    byte old_genre[] = new byte[20];
    buffer.get(old_genre);
    int bmp_size = buffer.getInt();
    int file_version = buffer.getInt();

    byte title[] = new byte[64];
    buffer.get(title);
    String str_title = bytes2string(title);
    easy.title = str_title;
    normal.title = str_title;
    hard.title = str_title;

    byte artist[] = new byte[32];
    buffer.get(artist);
    String str_artist = bytes2string(artist);
    easy.artist = str_artist;
    normal.artist = str_artist;
    hard.artist = str_artist;

    byte noter[] = new byte[32];
    buffer.get(noter);
    String str_noter = bytes2string(noter);
    easy.noter = str_noter;
    normal.noter = str_noter;
    hard.noter = str_noter;

    byte ojm_file[] = new byte[32];
    buffer.get(ojm_file);
    File sample_file = new File(file.getParent(), bytes2string(ojm_file));
    easy.sample_file = sample_file;
    normal.sample_file = sample_file;
    hard.sample_file = sample_file;

    int cover_size = buffer.getInt();
    easy.cover_size = cover_size;
    normal.cover_size = cover_size;
    hard.cover_size = cover_size;

    easy.duration = buffer.getInt();
    normal.duration = buffer.getInt();
    hard.duration = buffer.getInt();

    easy.note_offset = buffer.getInt();
    normal.note_offset = buffer.getInt();
    hard.note_offset = buffer.getInt();
    int cover_offset = buffer.getInt();

    easy.note_offset_end = normal.note_offset;
    normal.note_offset_end = hard.note_offset;
    hard.note_offset_end = cover_offset;

    easy.cover_offset = cover_offset;
    normal.cover_offset = cover_offset;
    hard.cover_offset = cover_offset;

    easy.source = file;
    normal.source = file;
    hard.source = file;

    ChartList list = new ChartList();
    list.add(easy);
    list.add(normal);
    list.add(hard);

    list.source_file = file;
    buffer = null;

    try {
        f.close();
    } catch (IOException ex) {
        Logger.global.log(Level.WARNING, "Error closing the file (lol?) {0}", ex);
    }
    return list;
}

public static List<Event> parseChart(OJNChart chart)
{
    ArrayList<Event> event_list = new ArrayList<Event>();
    try{
            RandomAccessFile f = new RandomAccessFile(chart.getSource().getAbsolutePath(), "r");

            int start = chart.note_offset;
            int end = chart.note_offset_end;

            ByteBuffer buffer = f.getChannel().map(FileChannel.MapMode.READ_ONLY, start, end - start);
            buffer.order(java.nio.ByteOrder.LITTLE_ENDIAN);
            readNoteBlock(event_list, buffer);
            f.close();
    }catch(java.io.FileNotFoundException e){
        Logger.global.log(Level.WARNING, "File {0} not found !!", chart.getSource().getName());
    } catch (IOException e){
        Logger.global.log(Level.WARNING, "IO exception on reading OJN file {0}", chart.getSource().getName());
    }
    return event_list;
}

private static void readNoteBlock(List<Event> event_list, ByteBuffer buffer) {
    boolean[] startedHN = new boolean[7]; //added this
    for ( int tmpc = 0; tmpc < 7; tmpc++ ){ startedHN[tmpc] = false; } //added this
    while(buffer.hasRemaining())
    {
        int measure = buffer.getInt();
        short channel_number = buffer.getShort();
        short events_count = buffer.getShort();

        Event.Channel channel;
        switch(channel_number)
        {
            case 0:channel = Event.Channel.TIME_SIGNATURE;break;
            case 1:channel = Event.Channel.BPM_CHANGE;break;
            case 2:channel = Event.Channel.NOTE_1;break;
            case 3:channel = Event.Channel.NOTE_2;break;
            case 4:channel = Event.Channel.NOTE_3;break;
            case 5:channel = Event.Channel.NOTE_4;break;
            case 6:channel = Event.Channel.NOTE_5;break;
            case 7:channel = Event.Channel.NOTE_6;break;
            case 8:channel = Event.Channel.NOTE_7;break;
            default:
            channel = Event.Channel.AUTO_PLAY;
        }

        for(double i=0;i<events_count;i++)
        {
            double position = i / events_count;
            if(channel == Event.Channel.BPM_CHANGE || channel == Event.Channel.TIME_SIGNATURE) // fractional measure or BPM event
            {
                float v = buffer.getFloat();
                System.out.println("Here c: "+ channel + " evC: " + events_count + " i: " + i + " v: " + v );
                if(v == 0)continue;

                event_list.add(new Event(channel,measure,position,v,Event.Flag.NONE));

            }else{ // note event
                short value = buffer.getShort();
                int volume_pan = buffer.get();
                int type = buffer.get();
                if(value == 0)continue; // ignore value=0 events

                // MIN 1 ~ 15 MAX, special 0 = MAX
                float volume = ((volume_pan >> 4) & 0x0F) / 16f;
                if(volume == 0)volume = 1;

                // LEFT 1 ~ 8 CENTER 8 ~ 15 RIGHT, special: 0 = 8
                float pan = (volume_pan & 0x0F);
                if(pan == 0)pan = 8;
                pan -= 8;
                pan /= 8f; //TODO or maybe 7f? (15-8) / 8 = 7 / 8 = 0.875 and it should be 1, right?

                value--; // make zero-based ( zero was the "ignore" value )

        // A lot of fixes here are done thanks to keigen shu. He's stealing my protagonism D:
        Event.Flag f;
        type %= 8;
        switch(type)
        {
        case 0:
                        if ( channel != Event.Channel.AUTO_PLAY && startedHN[channel_number-2] ){ break; } //added this
            event_list.add(new Event(channel,measure,position,value,Event.Flag.NONE,volume, pan)); //added this
        break;
        case 1:
            //Unused (#W Normal displayed in NoteTool)
        break;
        case 2:
            //fix for autoplay longnotes, convert them to normal notes (it doesn't matter but... still xD)
                        if ( channel != Event.Channel.AUTO_PLAY && startedHN[channel_number-2] ){ break; } //added this
            f = channel == Event.Channel.AUTO_PLAY ? Event.Flag.NONE : Event.Flag.HOLD;
            event_list.add(new Event(channel,measure,position,value,f,volume, pan));
                        startedHN[channel_number-2] = true; //added this
        break;
        case 3:
            //Skip if autoplay
                        if ( channel != Event.Channel.AUTO_PLAY && !startedHN[channel_number-2] ){ break; } //added this
            if(channel == Event.Channel.AUTO_PLAY) break;
            event_list.add(new Event(channel,measure,position,value,Event.Flag.RELEASE,volume, pan));
                        startedHN[channel_number-2] = false; //added this
        break;
        case 4:
            event_list.add(new Event(channel,measure,position,1000+value,Event.Flag.NONE,volume, pan));
        break;
        case 5:
            //Unused (#M Hold displayed in NoteTool. Does not link with 0x06.)
        break;
        case 6:
            //fix for autoplay longnotes, convert them to normal notes (it doesn't matter but... still xD)
            f = channel == Event.Channel.AUTO_PLAY ? Event.Flag.NONE : Event.Flag.HOLD;
            event_list.add(new Event(channel,measure,position,1000+value,f,volume, pan));
        break;
        case 7:
            //Skip if autoplay
            if(channel == Event.Channel.AUTO_PLAY) break;
            event_list.add(new Event(channel,measure,position,1000+value,Event.Flag.RELEASE,volume, pan));
        break;
        }
            }
        }
    }
    Collections.sort(event_list);
}

private static String bytes2string(byte[] ch)
{
    int i = 0;
    while(i<ch.length && ch[i]!=0)i++; // find \0 terminator
    String charset = CharsetDetector.analyze(ch);
    try {
        return new String(ch,0,i,charset);
    } catch (UnsupportedEncodingException ex) {
        Logger.global.log(Level.WARNING, "Encoding [{0}] not supported !", charset);
        return new String(ch,0,i);
    }
}

}

[mrcdk]

Yeah, I saw that behavior on o2mania but I'm not sure about it... what if there is no long note end? That's why I've used the solution from keigen-shu...

Btw, you can use Gist for the code

Oh yeah, all the work I'm doing is in the separated_parsers branch

[keigen-shu]

Hmm... ~hirandom's solution works, but it would just keep deleting notes until it finds a release note. Like CDK mentioned, it is still broken if there is no release note after the hold note. This is also true for my solution (i.e. what if there is no NORMAL note after the RELEASE note?).

If I remember correctly, both NoteTool and O2Jam would just display these notes as is without fixing them. So this bug is something that you can choose not to fix because it's not your problem.

What if you wanted to fix it? Well, I did some analysis on the possible points of errors.

+--------------------------------+----------+
| prevNote.Type >> thisNote.Type | Problem? |
+--------------------------------+----------+
| NORMAL        >> NORMAL        | Nope.    |
| NORMAL        >> LONG_START    | Nope.    |
| NORMAL        >> LONG_END      | Yes.     |
+--------------------------------+----------+
| LONG_START    >> NORMAL        | Yes.     |
| LONG_START    >> LONG_START    | Yes.     |
| LONG_START    >> LONG_END      | Nope.    |
+--------------------------------+----------+
| LONG_END      >> NORMAL        | Nope.    |
| LONG_END      >> LONG_START    | Nope.    |
| LONG_END      >> LONG_END      | Yes.     |
+--------------------------------+----------+

All you need to do some specific action when one of these problems pop up.

P/S: I just spent an entire evening figuring out these problems. However, the method I used to process these notes is different than the one on this project. Therefore, I won't be posting my solutions here unless someone is interested.

[mrcdk]

So, there is no ideal fix for this, right? If o2jam displays them without fix we should show them as is, right?

keigen-shu, are you converting the normal > long_end to a longnote or just skipping the long_end event?

[keigen-shu]

~mrcdk
There is no ideal fix for all cases, but you can keep the common errors away. Doing that is not as simple as just looking at the types of the two notes; you have to look fore more data and assume some rules.

For my solutions I assumed the following:

1. When iterating through notes looking for errors, a LONG_START note must be located before a LONG_END note. This means, if a stray LONG_END note is encountered, you have to look backwards first.
2. A long note should be valid if both ends of the long notes have identical SoundIDs.
3. Switching between LONG_START notes and NORMAL notes are allowed because they both make sounds.
4. Deletion of LONG_END notes are allowed because they do not make any sound.
5. If attempts fail, assume BMS's LN_TYPE 1 mode.
6. Always attempt to keep NORMAL notes in the game by moving them to the Autoplay lane.

Here's the pseudocode for the fixes:

• On a LONG_START >> NORMAL case, assume Rules 2 and 5. If it fails, look ahead until a non-NORMAL note is reached. If the non-NORMAL note is of type LONG_START or of different SoundID, convert LONG_START into NORMAL. Otherwise (meaning LONG_END with matching SoundID), follow Rule 6 and merge the LONG_START with the LONG_END.
• On a NORMAL >> LONG_END case (rule 1 will get you here) where both notes have the same SoundID, follow rules 2 and 3 and replace notes to LONG_START >> LONG_END. If both notes have different SoundID, follow rule 4.
• On a duplicate LONG_START or LONG_END case, create an appropriate note in between the duplicates so that you get two or more long notes.
• If all attempts to fix the problem fail or end up creating more problems, the chart is considered as unrecoverable.

Edit: Forgot to mention that you'll probably need sort the note list by time first before doing this.

[mrcdk]

Wow, thanks for this. I will work on this after i've finished the bmswriter (if i can because damn :() XD

[mrcdk]

Well, I've just committed a possible fix for the broken longnotes, but I've only tested with Fantazindy(o2ma269400.ojn) Would be nice if anyone can send me more broken songs to test it. If it's a keysounded one the better xD

[mrcdk]

Ok, I've tested more broken chart (there are a lot of them! seriously what were they thinking? xD) and it seems to work...

But because almost every broken chart is just a spamming of 972374073 different notes it's hard to say if it makes a good work or not...

[chaosfox]

can anyone send me these songs with the problem ?

I got the fix for one winged angel(o2ma74.ojn normal and hard is full of these problems), but by what you guys are saying the problem is bigger.

Also, like kegen-shu said, im not interested in solving the general problem, if it doesnt work on o2jam there is not much sense in trying to make it work here, because these songs were made to work on o2jam.

[mrcdk]

It should be fixed in the separated_parsers branch... The "fix" is in
EventList.java. I'm still doing some changes to it and want to finish them
before merging it.

EDIT: So, what are going to do then? We can't have broken charts being played, can we?

[chaosfox]

I know, but your fix is huge and mine is 10 lines, thats why I want to test on the other songs.

Thats the thing, there shouldn't be broken charts at all right ? if these songs were created to play on o2jam I imagine the author tested on o2jam until it worked.

[chaosfox]

So, I found the files on google(quite easy actually).
fantazindy (o2ma269400.ojn) have 10203 notes in total
o2mania on autoplay makes 9860 cools + 273 miss to a total of 10133 notes ! It is dropping 70 notes !

so I did the same test with my fix and open2jam on autoplay makes:
8558 cools + 1545 miss = 10133, same as o2mania

I tested xepher too but I couldn't get the numbers, o2mania note counter loops over when it reaches 9999 lol

I looked over your code on EventList, my fix is basically the same but I don't try to guess anything,
see here: https://gist.github.com/2847198

I compared some cases with o2mania and it seems to drop the same notes, I can't compare all of them though ..

[mrcdk]

dunno, mine makes 10132 notes xD My method tries to keep it conservative (keep all the playable notes) i think... Yours just deletes everything xD Also, take note that you have to move the none and hold events to the autoplay channel.

Now, yes, they should work in o2jam but we can't be sure about it, o2jam can have hacks to broken notetool versions, the chart can be a converted chart from another game that can have errors, our parser can have some errors too etc.

[chaosfox]

being conservative is good, but my fear is that the song will end up being different from o2jam with your method

I think I have o2jam emu here, Im gonna do some tests.

Edit: wait, it makes 1 less than my method ? Is that number right ?

[chaosfox]

So, i tested, I used one winged angel(o2ma74.ojn) on normal. This song have 2086 notes on total.

1. o2jam philippines ( this is me playing)
   http://dl.dropbox.com/u/371106/open2jam_16/o2jam_ph.png
   notice the score: 1797 + 196 + 10 = 2003, that means 83 notes dropped.
2. o2mania 1.4.2
   http://dl.dropbox.com/u/371106/open2jam_16/o2mania_1.4.png
   2003 notes, 83 dropped.
3. open2jam - cdk fix
   http://dl.dropbox.com/u/371106/open2jam_16/open2jam_fixsp.png
   2069 notes, 17 dropped.
4. open2jam - my fix
   http://dl.dropbox.com/u/371106/open2jam_16/open2jam_fixcf.png
   2003 notes, 83 dropped.

Your fix is able to "save" more notes, and maybe that was the author intention, but the problem is that if we do that it will be different from o2jam, I don't want it to be different from o2jam, even if it is "wrong", you know what I mean ?

[mrcdk]

I see... well, why don't keep both methods and let the user choose it?

And btw, that isn't my fix, is keigen-shu one xD

[chaosfox]

I guess so, you mean like a option on the settings ?

[keigen-shu]

It's better to keep the fix lying around, now that O2Jam is officially dead. O2Mania was the reason we had thousands of broken and improperly (SC as Note 1) written charts all over the net. There are songs that actually abuse this behaviour for reasons I do not want to know.

Maybe we make a pop-up when it detects note leaks to let players "Attempt to fix broken notechart" and prompt at the end of the song whether or not to backup the broken version and replace it with the fixed one.