Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Issues to be addressed with NBS format version 6 #307

Closed
12 tasks
Bentroen opened this issue Jan 28, 2022 · 3 comments
Closed
12 tasks

Issues to be addressed with NBS format version 6 #307

Bentroen opened this issue Jan 28, 2022 · 3 comments
Labels
C: Info/Discussion An informative post or extended discussion about future proposed improvements. C: NBS format change Suggestions or issues that require changes to the NBS format

Comments

@Bentroen
Copy link
Member

Bentroen commented Jan 28, 2022

Overview

Since the transition from MCNBS to ONBS, many updates to the .nbs format have been released. While those updates were necessary to support new features being added to the program, new NBS versions have been, so far, released on an "as necessary" basis, without any specific criteria or public notice prior to the official release.

Ideally, releasing a new version of a widely adopted format should be preceded by public discussions with its users. In the case of the NBS format, these would include plugin developers, as well as anyone creating software based on the format. The release schedule should also include an adaptation period, so developers have enough time to update plugins to comply with the new version.

In practice, this was never the case with the format versions released to this date. This impairs the widespread adoption of the format and causes plugins to be automatically outdated as a new version is released, to the detriment of their users and the NBS ecosystem as a whole.

Starting in NBS version 6, the project should strive to be as clear and transparent as possible about the upcoming changes, as well as take community input into consideration as much as possible. This way, future changes to the format can be bundled up in fewer version updates released as sparingly as possible.

This issue is an attempt to gather all the current issues with the NBS format as of version 5, as well as suggestions for new features that could potentially be included in version 6. Keep in mind this is only an overview of the current issues -- where necessary, more detailed write-ups on each of these issues may be written, including their possible workarounds.

If you're a developer creating something based on the .nbs format and would like to suggest a change, feel free to send it in the comments below!

Format issues

  • Instruments have no property to link them to Minecraft sound events. Currently, what many plugins (and even the built-in data pack export) do is to use the 'Instrument name' property as a sound event. This usage is not documented in the NBS specification.
  • No way to specify the loop end, which may be at the end of the bar or at the last note block according to the user's preferences. This makes the behavior of a particular NBS file non-deterministic/ambiguous across different installations.
  • As the song tempo is stored in ticks per second, there's not enough granularity to store precise values in beats per minute (BPM). Currently, only multiples of 0.15 BPM are stored properly, so values such as 110 get rounded to 109.95 on song load. (0.01 t/s = 0.15 BPM) (see BPM changes when project loaded #306)
  • It's not possible to pick which Minecraft version a song is aimed at, so the number of default instruments is always reliant on the Note Block Studio version that the song was saved in. This can be worked around by implementing "Future instruments" (to support opening newer songs in an older version), and allowing the Minecraft save version to be picked per song (unlocking access to 5, 10, or 16 custom instruments).
  • Note Block Studio currently allows 240 custom instruments. With the 16 default instruments, this occupies the full range of the unsigned byte data type (0-255). When new vanilla instruments are added to Minecraft, the instrument ID of songs nearing the maximum custom instrument size will overflow, preventing them from being saved on newer versions unless the numeric range for instrument IDs is increased.
  • The current way strings are encoded can only store ASCII characters. Non-ASCII characters only have the first byte of their UTF-16 representation written to the file, causing mojibake to appear when the song is reloaded. When the byte written to the file lies within the ASCII charset (0x00-0x7F), the equivalent ASCII character is displayed; otherwise, the displayed character is whatever ANSI character Windows maps that codepoint to. This can be fixed by encoding strings in UTF-8. Once this issue is fixed, the documentation should indicate how invalid ASCII characters should be treated in versions 0-5.
  • The binary nature of NBS files causes any change in the file structure to render the file incompatible with existing parsers. As .nbs files were never intended to be versioned, they don't conform to any particular standard, such as JSON or NBT; as such, it's not possible to "know" what's ahead without tightly following the most recent NBS specification. Ideally, a way should exist to "skip" unknown fields so any future .nbs files can be read by existing implementations. It's a technical challenge to make the format future-proof while avoiding major changes to the file structure.

Documentation issues

  • The 'Layer lock' field, originally intended to be a boolean, may actually assume values 0-2 (0= unlocked, 1=locked, 2=solo). This is currently undocumented in the NBS specification, and is caused by an incorrect variable (one that takes solo into account) being saved to the file.
  • Formally, there's no restriction put in place to the number of instruments, layers or ticks a song can have. Each version of the program, however, has limits regarding that count (e.g. songs with >200 layers, >18 custom instruments or >32,000 ticks can possibly crash certain versions of NBS). A survey must be made regarding the soft and hard limits of each Note Block Studio version, and, if deemed necessary, these limits should be enforced in the NBS specification.
  • The documentation marks all fields as signed, even though some of them don't fit in signed data types (e.g. layer panning = 0-200, instrument ID = 0-255). The signedness should be specified in every field to avoid ambiguities.
  • There are some features of the NBS format that are implemented in a particular manner by Note Block Studio, but aren't officially part of the specification. Some of these should be mentioned there, in order to standardize the way NBS and other implementations calculate some attributes of the song. These include:
    • How layer/note panning and volume should be weighted into the actual volume and panning values (implemented here)
    • How pitch is applied to the key in order to determine the final pitch of a note (and how to convert it to Minecraft's pitch values)
    • How the pitch value of custom instruments affects the final pitch of a note
  • Some fields (auto-save and auto-save duration) are deprecated, but there's no official recommendation in what to set those values to on newly created songs.

Feature suggestions

@Bentroen Bentroen added the C: NBS format change Suggestions or issues that require changes to the NBS format label Jan 28, 2022
@koca2000
Copy link

First of all, I definitely support the idea of the format discussion and I think it may bring some really good things to the .nbs format.

Instruments have no property to link them to Minecraft sound events. Currently, what many plugins (and even the built-in data pack export) do is to use the 'Instrument name' property as a sound event. This usage is not documented in the NBS specification.

This is definitely interresting issue and as far as I remember it is also inconsistently used. NoteBlockAPI use the 'Sound file' property for custom instrument's since there is lower probabilty that it will be in incorrect format (OpenNoteBlockStudio's default custom instrument name is not allowed in Minecraft).

The current way strings are encoded can only store ASCII characters. Non-ASCII characters only have the first byte of their UTF-16 representation written to the file, causing mojibake to appear when the song is reloaded. When the byte written to the file lies within the ASCII charset (0x00-0x7F), the equivalent ASCII character is displayed; otherwise, the displayed character is whatever ANSI character Windows maps that codepoint to.

Most of the songs I came across didn't even have their metadata field filled but I agree that on the specification level this may be a problem. The first obvious idea is to switch to UTF-8 which is completely universal when it comes to letters, symbols and whatever may user enter. However, it's decoding is definitely more complicated than ASCII since it's not only multibyte but also with different count of bytes per symbol. For Java parsers like NoteBlockAPI this shouldn't be a problem.

The binary nature of NBS files causes any change in the file structure to render the file incompatible with existing parsers. As .nbs files were never intended to be versioned, they don't conform to any particular standard, such as JSON or NBT; as such, it's not possible to "know" what's ahead without tightly following the most recent NBS specification. Ideally, a way should exist to "skip" unknown fields so any future .nbs files can be read by existing implementations. It's a technical challenge to make the format future-proof while avoiding major changes to the file structure.

I think that the fact that NBS files are binary is a good thing and I would definitely stick to it because any textual format would make the files a lot bigger. The solution to the issue of future proof implementation also is not as complicated and have been used in many file formats over the years. The NBS file is already splited into blocks (metadata block, note blocks, layer blocks, custom instrument blocks) if you add a length information in bytes to the beginning of each block, you can later add any field you want to the end of the block and properly implemented parsers should simply seek to the end of the block and ignore these new fields.

Formally, there's no restriction put in place to the number of instruments, layers or ticks a song can have. Each version of the program, however, has limits regarding that count (e.g. songs with >200 layers, >18 custom instruments or >32,000 ticks can possibly crash certain versions of NBS). A survey must be made regarding the soft and hard limits of each Note Block Studio version, and, if deemed necessary, these limits should be enforced in the NBS specification.

Actually, this is not really true. There are hard limits in the NBS specification. Since e.g. song's length is signed short in the specification you simply can't have a song longer than 32 767 ticks even if your parser would be able to handle it. And the same holds for every numeric field. I think that especially numbers like song's length (short), instrument's identifier (byte), custom instrument's count and identifier (both byte) should be changed to at least int or short.

From the point of view of the NoteBlockAPI developer, the rest of the issues is not very important for me. Layer lock was ignored so far because it doesn't make much sense to lock some layer in the exported song (and there is also no reason why would you export song with multiple layers but only want to play only one layer). Since the tempo is stored as ticks per second in file the issue with BPM is also not relevant for me. NoteBlockAPI already supports future instruments so that is also not an issue for me.

@encode42
Copy link
Member

encode42 commented Feb 3, 2022

The 'Layer lock' field, originally intended to be a boolean, may actually assume values 0-2 (0= unlocked, 1=locked, 2=solo). This is currently undocumented in the NBS specification, and is caused by an incorrect variable (one that takes solo into account) being saved to the file.

I believe this would be a great feature to keep and document, as I and many others mute incomplete sections of the song or single out certain layers. The lack of documentation creates inconsistencies between ONBS, NoteBlockAPI, NBS.js, and whatever other .nbs parsers are out there. For example, NBAPI ignores layer solo and lock status as stated above. NBS.js, however, does not.

Instruments have no property to link them to Minecraft sound events. Currently, what many plugins (and even the built-in data pack export) do is to use the 'Instrument name' property as a sound event. This usage is not documented in the NBS specification.

I believe this is up to the library developers and users to deal with. Every implementation of the format would be different either way. NBAPI would have to map to Minecraft's sound events, NBS.js has to load instruments from an ArrayBuffer somehow, etc. The only solution I'd have in mind is to include the instrument files directly in the file, but that'd add an extreme amount of bloat fast. Potentially a way to export the song and its instruments separately from saving the song?

The binary nature of NBS files causes any change in the file structure to render the file incompatible with existing parsers. As .nbs files were never intended to be versioned, they don't conform to any particular standard, such as JSON or NBT; as such, it's not possible to "know" what's ahead without tightly following the most recent NBS specification. Ideally, a way should exist to "skip" unknown fields so any future .nbs files can be read by existing implementations. It's a technical challenge to make the format future-proof while avoiding major changes to the file structure.

Saving with a JSON-like format would be much easier to read, manipulate, and write. However, as @koca2000 said, the file size would explode. While I was working on the playlist feature of NBSPlayer, I attempted to save the loaded songs as their loaded JSON form. The resulting file was megabytes larger than all the loaded .nbs files combined. Keeping the format binary would be the best course of action for the sake of storage.

There's most likely a better way to keep the format future-proof while sticking with the binary format. For example, containing "version" blocks at the end of the file, such as a "v1" block, "v2", and so on. The parser can stop when it's finished reading all the version data it needs, and won't run into issues with any unimplemented versions.

As the song tempo is stored in ticks per second, there's not enough granularity to store precise values in beats per minute (BPM). Currently, only multiples of 0.15 BPM are stored properly, so values such as 110 get rounded to 109.95 on song load. (0.01 t/s = 0.15 BPM) (see BPM changes when project loaded #306)

This could be easily solved by making BPM the primary tempo format. Load as BPM, then calculate and save the TPS from said BPM.


I greatly appreciate the new approach on spec changes. As the community expands and more parsers are developed, this will ensure the future stability of the format and its parsers!

@Bentroen
Copy link
Member Author

Bentroen commented Mar 16, 2022

Thank you @koca2000 and @encode42 for your input! It's great to have different perspectives on the problem.

NoteBlockAPI use the 'Sound file' property for custom instrument's since there is lower probabilty that it will be in incorrect format (OpenNoteBlockStudio's default custom instrument name is not allowed in Minecraft).

For example, NBAPI ignores layer solo and lock status as stated above. NBS.js, however, does not.

This goes to show how important proper standardization is. We've had two different implementations having to come up with workarounds for issues with the format in their own way!

I think that the fact that NBS files are binary is a good thing and I would definitely stick to it because any textual format would make the files a lot bigger.

I can confirm we won't be moving to a different format. Not only the increased file size would be undesirable, but this would mean an added difficulty for plugin makers to maintain two different formats. That being said, JSON could be used in the future as an intermediate format for saving project files. (I'll get back to this!)

The first obvious idea is to switch to UTF-8 which is completely universal when it comes to letters, symbols and whatever may user enter.

This is probably the solution we're gonna go with. Though, we must also clearly determine how existing songs have to be interpreted, as for example pynbs crashes when reading a field containing non-ASCII characters due to not specifying an encoding (see OpenNBS/pynbs#3).

Potentially a way to export the song and its instruments separately from saving the song?

This is planned as a separate file format! See #239.

if you add a length information in bytes to the beginning of each block, you can later add any field you want to the end of the block and properly implemented parsers should simply seek to the end of the block and ignore these new fields.

For example, containing "version" blocks at the end of the file, such as a "v1" block, "v2", and so on.

These are both great ideas, but I'm wondering if they can be implemented in a successful way, given the kinds of changes we might have to make.

For example, if you look at the kinds of fields that have been added over the past few NBS versions, not all of them are simply part of a block, but they are compound fields that appear once for every instance of an element (e.g. layer lock, note block pitch). So to make this work while allowing new fields to be added anywhere -- even in note blocks or layers --, we would probably have to repeat this procedure on multiple levels.

Not only this, but some of the changes could even modify the way some fields are interpreted: see, for example, @encode42's suggestion:

This could be easily solved by making BPM the primary tempo format.

Considering changes of this nature could be needed even after the future-proof update, no change in the file structure would account for this in a way that would avoid the need for parser updates, unless we marked the existing field as deprecated and re-added it later in the file.

Accounting for all of this would increase the format's complexity in such a way that it makes me wonder whether it's worth to make the format future-proof at all. On the long term it would be a wise idea, but on the other hand, the number of fields we could potentially still add is so small that it possibly doesn't justify such a large structural change.

Layer lock was ignored so far because it doesn't make much sense to lock some layer in the exported song

And this brings me to the third point: NBS files are already kinda saturated with features. Anything we could add further on from this point would probably be irrelevant for NBS players -- as mentioned by @koca2000, features such as layer lock already are.

There are two solutions for this: one is to delay NBS version 6 so we have enough time to think of all features that could potentially cause breaking changes, and add them to the format in that version.

The other, and more ideal, one, would be to split responsibilities: create a new type of file called .nbsproj to hold all data that's relevant to project files (this could be a JSON file since it's much more likely to suffer changes over time), and keep .nbs as a distribution format. This is a long-term goal that would likely only be pursued on the Python rewrite.


So, here are the key takeaways of this:

  • Some of these issues are complex and require further discussion so we can come up with an ideal solution, raising the need for separate tickets to be opened.
  • After these changes are completed, it would be wise to mark NBS files as feature-complete, and this would avoid the need for further changes.
  • Any further changes, if necessary, should belong in a new format that will solve the issue of needing an NBS format change for every tiny feature added to the program. Then, the way the program interprets those features may change, but the way it's written on NBS files will always be the same.
  • Since we can't change files that are already saved and support for them must be kept, these caveats resulting from the current issues could go in a new section of the NBS spec page, "Issues", mentioning all current issues and the suggested way(s) of fixing them.

NB: in the past, I've done a detailed write-up on both character encoding and layer lock, but on both occasions I've lost them due to typing directly on the GitHub form. :(

Bentroen added a commit to OpenNBS/pynbs that referenced this issue Apr 7, 2022
Prevents `UnicodeDecodeError: invalid continuation byte` when decoding non-ASCII characters, due to a bug in Note Block Studio where only the first byte of the character's UTF-16 codepoint is written to the file, resulting in an invalid UTF-8 codepoint. This will be patched in a future version of the NBS format (see OpenNBS/NoteBlockStudio#307).

`cp1252` was chosen to closely match the current (unintended) interpretation of those characters by Note Block Studio, given the fact that it has always been a Windows-only application. Also, unlike ISO-8859-1/Latin-1, all characters in the `0x8A-0xFF` range are printable characters (whereas `0x80-0x9F` are control characters in Latin-1).

By no means this is an ideal solution -- it's nothing short of a workaround to prevent that exception, until the issue is properly fixed in Note Block Studio. As the information in those characters is already lost the moment they are saved to the file, it's preferable to have the read process yield "garbage" characters than incur in an exception.

Fixes #3
@Bentroen Bentroen pinned this issue Feb 19, 2023
@Bentroen Bentroen added the C: Info/Discussion An informative post or extended discussion about future proposed improvements. label Feb 19, 2023
@koca2000 koca2000 mentioned this issue Mar 1, 2023
@OpenNBS OpenNBS locked and limited conversation to collaborators Jun 2, 2023
@Bentroen Bentroen converted this issue into discussion #402 Jun 2, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
C: Info/Discussion An informative post or extended discussion about future proposed improvements. C: NBS format change Suggestions or issues that require changes to the NBS format
Projects
None yet
Development

No branches or pull requests

3 participants