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

Remove bitmask for getInfo() in jni #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LossyDragon
Copy link

@LossyDragon LossyDragon commented Nov 29, 2020

There are some modules that actually contain 256 rows, such as xylan orb and libxmp reports it as such. The masks limit the values at 255, so anything higher will result in 0. XM modules can have 256 (1...256) rows based on the documentation. I guess XM starts with 1 instead of 0 for the beginning row?

I don't see a reason why these values should be masked as its just passing long integer values provided by xmp_frame_info.

Fixes issue at libxmp/libxmp#63

@AliceLR
Copy link

AliceLR commented Nov 29, 2020

FWIW X-Tracker modules can have up to 512 rows and OctaMED modules can have up to 3200 rows, so the row/num_row bounds definitely would have been problematic even if the length value was (length - 1). I also think some formats allow >=256 positions and patterns (also XM?). The bounding here is arbitrary and I agree it should be removed.

Out of curiosity, does this MED with a 3200-line pattern cause UI breakage or crashes? xmp-cli can't display the higher row/length values and exits before it finishes playing (but it doesn't seem to be a crash)... long.med.zip

@LossyDragon
Copy link
Author

LossyDragon commented Nov 29, 2020

FWIW X-Tracker modules can have up to 512 rows and OctaMED modules can have up to 3200 rows, so the row/num_row bounds definitely would have been problematic even if the length value was (length - 1). I also think some formats allow >=256 positions and patterns (also XM?). The bounding here is arbitrary and I agree it should be removed.

Out of curiosity, does this MED with a 3200-line pattern cause UI breakage or crashes? xmp-cli can't display the higher row/length values and exits before it finishes playing (but it doesn't seem to be a crash)... long.med.zip

It seems to play it fine, but silently breaks the pattern row with an error of E/Xmp: [PatternViewer] Canvas error: java.lang.ArrayIndexOutOfBoundsException: length=256; index=276

No crashes of any kind, just the canvas is unable to deal with that large of a row size.

But at a quick glance, it seems to be caused by this along with the decimal to hex conversion at line 83

@LossyDragon
Copy link
Author

Though I'm not sure currently how deep the issue would persist Specifically how the canvas will react with a 3 character long row number next to channel 1

@LossyDragon
Copy link
Author

LossyDragon commented Nov 29, 2020

xmp-cli can't display the higher row/length values and exits before it finishes playing (but it doesn't seem to be a crash

I have to hold the player on loop in order to complete all the rows. The time states it about 5 seconds long for playtime which when its not looping will also exit prematurely.

@LossyDragon
Copy link
Author

LossyDragon commented Nov 29, 2020

I managed to get the pattern viewer to display values over 256. Though there are a couple of issues to address to port it over to java and this PR.

1. The row numbers get considerably close to the first channel. As shown in the screenshot.
2. Some refactoring would need to take place to dynamically get the row numbers automatically. Instead of assuming 256 (or in my example 3200)

Edit: Fixed. With a bonus of re-centering the line bar so its not so high up.

hiWLTin

… 3 digit row numbers, recenter current line bar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants