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

Wrong values for DT (Detune) when importing TFI/VGI instruments #88

Closed
bryc opened this issue Mar 26, 2019 · 30 comments
Closed

Wrong values for DT (Detune) when importing TFI/VGI instruments #88

bryc opened this issue Mar 26, 2019 · 30 comments

Comments

@bryc
Copy link

bryc commented Mar 26, 2019

These values are being loaded as raw into registers, but TFI/VGI defined custom values so they end up being wrong.

raw value TFI/VGI real OPN
0 -3 0
1 -2 1
2 -1 2
3 0 3
4 +1 0
5 +2 -1
6 +3 -2
7 -3

Real OPN uses sign-magnitude numbers, so 0 and 4 are both "no detune applied".
There's no straight-forward way to convert, but this is the direct translation of values:

TFI/VGI raw OPN raw
0 7
1 6
2 5
3 0 or 4
4 1
5 2
6 3
@rerrahkr
Copy link
Member

I fixed it at 596b38d. Thanks for your report!

@freq-mod
Copy link
Contributor

Hmm, now TFI's -3 value (raw 0) translates to 5, which is wrong if the table is correct...

@OPNA2608
Copy link
Member

@Papiezak It appears to work fine on my end with 596b38d? weird.
grafik

@freq-mod
Copy link
Contributor

@OPNA2608 Interesting. I will check once again when I return from work, maybe it's a problem with @Wohlstand's OPN Bank Editor that confused me...

@Wohlstand
Copy link

maybe it's a problem with @Wohlstand's OPN Bank Editor that confused me...

@Papiezak , I have mapped value to the raw OPN value, therefore combobox is indicating the same as raw OPN value. Anyway, because of TFI's different detune format, I have made this to convert it into raw OPN2 that is used by OPN-BE internally: https://github.com/Wohlstand/OPN2BankEditor/blob/master/src/FileFormats/format_tfi.cpp#L42-L80

@freq-mod
Copy link
Contributor

dtn

On operator 4, "-3" detune value from bank editor got translated to "5" at Bamboo Tracker.

@Wohlstand
Copy link

Wohlstand commented Mar 27, 2019

Looking on those tables:

static uint8_t opn1_to_opn2[7]
{
    5, 6, 7, 0, 1, 2, 3
};

static uint8_t opn2_to_opn1[8]
{
    3, 4, 5, 6, 3, 0, 1, 2
};

the -3 of OPN2BE is same as raw 0, so, it will save into TFI the value of opn2_to_opn1[0], I.e. raw 3. The question, where in BambooTracker the code that works with TFI? Also, does it loads same or differently when you'll import via OPNI or WOPN? Note that BambooTracker now does support that 😉

@freq-mod
Copy link
Contributor

Well, with OPNI -3 is actually "7" in Bamboo Tracker 🤔

@Wohlstand
Copy link

Wohlstand commented Mar 27, 2019

Well, with OPNI -3 is actually "7" in Bamboo Tracker 🤔

So, it's the question how the detune dealing is done here 🤔
OPNI keeps OPN2 register native format of detune without any sort of re-mapping 🤔
TFI keeps something different that needs additional re-mapping to make it be correct with OPN2...

@OPNA2608
Copy link
Member

@Wohlstand

The question, where in BambooTracker the code that works with TFI?

This is all the TFI-specific code I can spot at quick glance.
https://github.com/rerrahkr/BambooTracker/blob/596b38d12c1cae5cd0dd81fc9504720afab9943c/BambooTracker/io/instrument_io.cpp#L2148-L2229

@Wohlstand
Copy link

Wohlstand commented Mar 27, 2019

Well, with OPNI -3 is actually "7" in Bamboo Tracker thinking

@OPNA2608 , lol, why OPNI's detune was treated as TFI's? 🤣 I already said, OPNI keeps native OPN2 value that doesn't needs any conversion! 🦊
But, looks like this is not a correct mapping, and better to compare with original software where TFI is native format, I have used that as referrence while worked on TFI originally...

Also, OPN2's detune format does use of 8 values, two of them are meaning +0 and -0.

@OPNA2608
Copy link
Member

OPNA2608 commented Mar 27, 2019

@OPNA2608 , lol, why OPNI's detune was treated as TFI's? 🤣 I already said, OPNI keeps native OPN2 value that doesn't needs any conversion! 🦊

I don't get what you're saying, sorry. What is OPNI? We're talking about the TFI format here.

Testing the .TFI export with your very own tool appears to prove this mapping as correct.

@freq-mod
Copy link
Contributor

freq-mod commented Mar 27, 2019

@Wohlstand
Copy link

Wohlstand commented Mar 27, 2019

I don't get what you're saying, sorry. What is OPNI? We're talking about the TFI format here.

I have asked to test import with OPNI to verify the situation with TFI to guess where is the issue as in the case of OPNI the detune field doesn't need to convert into anything as OPNI provides OPN2 native values as-is. Anyway, if my TFI detune map tables are incorrect, I'll fix them. I did that very long time ago, and I agree if some mistake was made.

@OPNA2608
Copy link
Member

I just had someone export an example .tfi file from TFM Music Maker.

https://cdn.discordapp.com/attachments/519164281518948372/560488921943769092/01n10.tfi

The Detune setting of the 4 operators are: 0 +1 -1 0
Their representation in the .tfi file is: 03 04 02 03, which should import correctly into BT after the above-linked commit.

It appears your .tfi Detune mapping is wrong.

@Wohlstand
Copy link

Wohlstand commented Mar 27, 2019

Just now I have found this issue: Wohlstand/OPN2BankEditor#5
I have made my map by using of this: https://vgmrips.net/wiki/TFI_File_Format#Detune_values

@OPNA2608
Copy link
Member

When I exported a patch with the same 0 +1 -1 0 Detune values from your editor, i got different values. (3 0 4 3 iirc)

@Wohlstand
Copy link

Okay, I see what's wrong: I have mistook with the order of negative values: 5, 6, 7, but must be 7, 6, 5, so, I'll send the fix now

Wohlstand added a commit to Wohlstand/OPN2BankEditor that referenced this issue Mar 27, 2019
By mistake, the order of negative values was inverted.

The discussion: BambooTracker/BambooTracker#88
@Wohlstand
Copy link

Made just now: Wohlstand/OPN2BankEditor@496a615
Once it will be built by CI, you'll be able to verify the stuff. Otherwise, you can pull that and build from source by yourself.

@Wohlstand
Copy link

Well, with OPNI -3 is actually "7" in Bamboo Tracker thinking

Anyway damn, I have miss-looked, it's correct value, just didn't noticed it's here shown in unsigned raw form here, my fault 😋

@OPNA2608
Copy link
Member

So the import of .tfi instruments into BT appears to work correctly now. Can this issue be closed @bryc?

@jpcima
Copy link
Contributor

jpcima commented Mar 27, 2019

@OPNA2608 , lol, why OPNI's detune was treated as TFI's? rofl I already said, OPNI keeps native OPN2 value that doesn't needs any conversion! fox_face

Where? Did I make an implementation mistake in Bamboo Tracker's OPNI?
I took a direct value from registers. From my point of view, this seemed to be fine.

https://github.com/rerrahkr/BambooTracker/blob/596b38d12c1cae5cd0dd81fc9504720afab9943c/BambooTracker/io/instrument_io.cpp#L2529

@Wohlstand
Copy link

Where? Did I make an implementation mistake in Bamboo Tracker's OPNI?
I took a direct value from registers. From my point of view, this seemed to be fine.

No, everything is fine, it's my misslook where I though it was incorrect 😋

@bryc
Copy link
Author

bryc commented Mar 28, 2019

Sure it can be closed, that can be left to the project owners. But this discussion got me thinking that maybe the UI can be improved..

DT

Low 2 bits are the magnitude, a value of 0 to 3, and third bit is the sign, so if enabled, magnitude is negative. I think that would be a more intuitive way of editing.

It's kind of confusing to look at it 0-7, because the detune goes 0, +1, +2, +3... 0, -1, -2, -3. I suppose the intention of TFI/VGI was to make this more understandable, but it apparently made things worse now that two bugs were found in two different projects 😆.

The alternative would be to relabel the notches on the slider, so that it does not go from 0-7, but are ordered "0:7, 1:6, 2:5, 3:4, 4:3, 5:2, 6:1", and the display could be -3 to +3. Because its just UI, it can be done without changing the register data, but this would mean that a value of 0 needs to be switched to 4, and 0 is no longer selectable (Or the inverse: 4 is converted to 0, and 4 is not selectable).

@freq-mod
Copy link
Contributor

Sure, but there still are tons of software (notably, hoot voice ripper) that use 0-7 scale, so there would have to be a hint what 1..3 and 5...7 coresspond to.

Or even better - an option to choose between detune slider type 😉

After all, there are a lot of people that think or thought that "-3" means "0", "0" meant "3" and "+3" meant "7"...

@rerrahkr
Copy link
Member

an option to choose between detune slider type 😉

That looks very useful to know the real DT behavior. I will try to implement it later.

@Wohlstand
Copy link

That looks very useful to know the real DT behavior. I will try to implement it later.

At me in OPN2-BE I have shown detune values from OPN2 documentation with ε formula:

0, ×(1+ε), ×(1+2ε), ×(1+3ε), 0, ×(1-ε), ×(1-2ε), ×(1-3ε)

@OPNA2608
Copy link
Member

The OPNA documentation depicts the mapping of DT-value -> meaning like this:
grafik

I think a simple +3 to -3 enumeration looks alot cleaner and intelligible at first glance than the ε formula the OPN2 documentation uses, while also following this example the manual sets.

@rerrahkr
Copy link
Member

rerrahkr commented Mar 31, 2019

At fc1f48d I added detune slider type selection whether 0-7 or -3-+3. (I fixed slider label to display "+" in "signed type" at bdc97d7.)

@rerrahkr
Copy link
Member

rerrahkr commented May 3, 2019

I released v0.2.0, the fixed version. Thanks everyone! Close.

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

No branches or pull requests

6 participants