View Issue Details

IDProjectCategoryView StatusLast Update
0000925OpenMPT[All Projects] libopenmptpublic2018-08-02 15:07
ReporterSaga MusixAssigned Tomanx 
PrioritynormalSeverityfeatureReproducibilityN/A
Status newResolutionopen 
Product Version 
Target Versionlibopenmpt 0.5 (goals)Fixed in Version 
Summary0000925: Support for external samples in libopenmpt
Description

libopenmpt does currently not support sample data (or any other kind of data) outside the user-provided module file. There are several multi-file formats supported by OpenMPT:

  • MPTM (external samples)
  • MT2 (external samples)
  • ITP (external instruments)
  • Startrekker FLT/EXO modules (external synthesized instruments)

In the future, we may also support:

  • Graoumf Tracker 2 (external samples, I started working on a loader but it's far from finished)
  • TFMX (external samples, no plans to support this format at the moment)
  • Other formats

Apart from the fact that someone may simply want to play such modules using a libopenmpt-based player, there is also the possibility of someone wanting to use external samples e.g. for saving space when composing music for a game.

There are also security implications when accessing external files, and the feature should probably be opt-in (and possibly fully backwards-compatible with previous library versions, e.g. by using a ctl value for providing the module file path).

TagsNo tags attached.
Has the bug occurred in previous versions?
Tested code revision (in case you know it)

Relationships

parent of 0001135 assignedmanx mpt::fstream is broken on MinGW for unicode filenames 
has duplicate 0001078 closed Silent audio output (no sound) on foobar2000 
Not all the children of this issue are yet resolved or closed.

Activities

manx

manx

2017-03-14 15:45

administrator   ~0002911

When adding support for file I/O inside libopenmpt anyway, we can also extend the API to to specify a filename directly instead of having the API user do the file loading. The single reason for not supporting that right now, is the lack of all direct file I/O inside libopenmpt.

A ctl for the existing functions might also be useful though. Not sure about that yet.

manx

manx

2017-03-14 15:47

administrator   ~0002912

Another possibility might be to have libopenmpt provide the list of external filenames to the application and have the application issue an explicit load_instrument or load_sample call after loading the module had already been loaded by libopenmpt.

A variant of that approach would involve callbacks to the application while loading. However, that approach will be very difficult to implement in foreign language bindings.

Saga Musix

Saga Musix

2017-03-14 15:51

administrator   ~0002913

Another possibility might be to have libopenmpt provide the list of external filenames to the application and have the application issue an explicit load_instrument or load_sample call after loading the module had already been loaded by libopenmpt.

This might not be feasible with all supported module formats, because sometimes patch-ups are done after loading the external file, but before giving control back to the application. For example in the ITP format, we first load an instrument file and then may override some of the instrument properties if "Embed instruments' header" is used. We would have to be able to do these patch-ups after loading the file, which would complicate the code in many ways. I do not like that idea at all.

manx

manx

2017-03-14 15:52

administrator   ~0002914

From an implementation point-of-view, being able to use C++17 std::filesystem would simplify path traversal and related security issues a lot.

manx

manx

2017-03-14 15:54

administrator   ~0002915

Another possibility might be to have libopenmpt provide the list of external filenames to the application and have the application issue an explicit load_instrument or load_sample call after loading the module had already been loaded by libopenmpt.

This might not be feasible with all supported module formats, because sometimes patch-ups are done after loading the external file, but before giving control back to the application. For example in the ITP format, we first load an instrument file and then may override some of the instrument properties if "Embed instruments' header" is used. We would have to be able to do these patch-ups after loading the file, which would complicate the code in many ways. I do not like that idea at all.

OK, I had kind of expected such complications, thus this possibility is not on the table any more.

manx

manx

2017-03-14 16:31

administrator   ~0002916

We probably want at least 2 levels of external file loading:

  1. files in the same directory as the module or in subdirectories below it
  2. files in directories above the module or absolute paths

With option 1 being the safer subset of option 2.

Saga Musix

Saga Musix

2017-03-14 16:39

administrator   ~0002917

Option 1 is also the more often seen and more useful variant. Option 2 is more likely to occur with unfinished / unreleased modules on an artist's computer.

Saga Musix

Saga Musix

2017-03-20 12:07

administrator   ~0002921

For platforms like Emscripten, Startrekker (and possibly TFMX in the future) support might be the most interesting case (for example for listening to Startrekker files on ModLand online).

It's also the most peculiar case because we won't know in advance if the external instrument file will be present or not, and which file extension it will have. For a Startrekker module called foo.mod, the external instrument file could be named foo.mod.nt, foo.mod.NT, foo.mod.as or foo.mod.AS (ignoring mixed-case file extensions). It is also perfectly valid for this external instrument file to be missing in case no synthesized instruments are used. Emscripten can provide a virtual file system but as far as I understand, it won't help us when playing modules from an external source. Hence, a callback might be the most convenient way to handle this situation for Emscripten, even though it might send up to four bogus HTTP requests when loading a Startrekker module. Like in most other languages, Emscripten can use C callback functions.

Saga Musix

Saga Musix

2018-01-29 21:04

administrator   ~0003400

Last edited: 2018-01-29 21:06

View 3 revisions

This topic has been brought up again because emoon's new HippoPlayer would have to support this in its own plugin API, and one if its default plugins uses libopenmpt.
The callback function approach would have two benefits for us:

  • It works with virtual file systems (e.g. when playing modules in zip files, or over the internet as in the emscripten example)
  • Any path traversal logic would have to be done by the library user. Security would be their business, not ours.

In addition, any programming language that can interface in C should be able to use C callbacks (otherwise e.g. using the WinAPI would be more or less impossible).

The C callback function signature could look like this: bool LoadFile(const char *filename, void **data, size_t *size)

manx

manx

2018-01-30 16:19

administrator   ~0003401

The callback function approach would have two benefits for us:

We alone would have to duplicate the security-conscious directory handling code at least twice in this case: once for openmpt123 and once for xmp-openmpt. We would not gain any ease of implementation either. Also, in the common use case of loading the files from a local file-system, this would unnecessarily complicate client code. APIs should be easy to use correctly and difficult to use incorrectly. Providing only a callback interface and offloading the responsibility to client code in this aspect would violate this API design principle.
However, I do think such a callback function could be useful in addition to libopenmpt's internal handling, in particular for players which can load from some kind of virtual internal file system. I do not think it should be the only option, and I do not think it should be the default option to handle this though.

Issue History

Date Modified Username Field Change
2017-03-14 15:36 Saga Musix New Issue
2017-03-14 15:39 manx Target Version => OpenMPT 1.?? (libopenmpt 1.0) (goals)
2017-03-14 15:45 manx Note Added: 0002911
2017-03-14 15:47 manx Note Added: 0002912
2017-03-14 15:48 manx Assigned To => manx
2017-03-14 15:48 manx Status new => assigned
2017-03-14 15:48 manx Status assigned => new
2017-03-14 15:51 Saga Musix Note Added: 0002913
2017-03-14 15:52 manx Note Added: 0002914
2017-03-14 15:54 manx Note Added: 0002915
2017-03-14 16:31 manx Note Added: 0002916
2017-03-14 16:39 Saga Musix Note Added: 0002917
2017-03-14 22:26 Saga Musix Description Updated View Revisions
2017-03-20 12:07 Saga Musix Note Added: 0002921
2017-08-27 09:27 manx Target Version OpenMPT 1.?? (libopenmpt 1.0) (goals) => libopenmpt 0.4 (goals)
2018-01-17 08:06 manx Relationship added has duplicate 0001078
2018-01-29 21:04 Saga Musix Note Added: 0003400
2018-01-29 21:06 Saga Musix Note Edited: 0003400 View Revisions
2018-01-29 21:06 Saga Musix Note Edited: 0003400 View Revisions
2018-01-30 16:19 manx Note Added: 0003401
2018-02-14 20:09 manx Target Version libopenmpt 0.4 (goals) => libopenmpt 0.5 (goals)
2018-08-02 15:07 manx Relationship added parent of 0001135