oot icon indicating copy to clipboard operation
oot copied to clipboard

Audio decomp, supersedes #1236

Open playerskel opened this issue 2 years ago • 12 comments

This PR supersedes #1236 by @MNGoldenEagle. I have tried to continue working on @MNGoldenEagle (and team)'s great work where they currently lack the time to finalize this PR for merging.

Some things covered here (in no particular order):

  • removes whitespaces and weird characters in (auto-generated) filenames
  • get rid of the ELF creation way of embedding data into the final ROM
  • automatically generate the audio_init_params.c constants upon music (.aifc) changes
  • apply some, but certainly not all, feedback from @Dragorn421 and @engineer124
  • merged with the latest master branch

Not sure if it's ideal, but instead of creating ELF files I have now opted to generate C files directly from within the audio-related Python scripts. These are then compiled into object files etc. This can be improved further, but it works fairly well for now.

I'd be happy to apply other feedback or necessary changes, but I tried to steer away from making bigger picture decisions such as naming schemes etc that were mentioned in #1236. Will reply in #1236 to clarify what has been applied in this PR and what has not. Looking forward to feedback.

playerskel avatar Mar 30 '23 22:03 playerskel

one issue that causes me lots of problems (using the old audio PR) is the python scripts don't clean up their temp files, and I ended up filling my WSL partition due to millions (literally) MQDebug_xxxxxxxx_aseq directories in my /tmp folder. sooo not sure if you explicitly addressed that with the new changes, but if not, i do recommend making the scripts clean up after themselves lol

krm01 avatar Mar 31 '23 01:03 krm01

@krm01 i did notice the same and removed some temporary files, but not all; will definitely have another look and address this, good point @engineer124 makes sense, agreed, will do

playerskel avatar Mar 31 '23 07:03 playerskel

https://user-images.githubusercontent.com/45049787/229369141-e25d6016-5f26-4422-bf37-714e0a0c0a03.mp4

after messing around i was able to replace voice clips

ariahiro64 avatar Apr 02 '23 17:04 ariahiro64

@engineer124 @fig02 @Dragorn421 hey team, was wondering if there are concrete things we/I can work on to get this merged or if it's not yet entirely clear to the rest of the team (which I could understand too). There's always things to improve further, but the audio system works rather well and it should be a fairly straightforward merge. So kind of wondering what can be done to get this going before the PR becomes stale and outdated etc. Thanks.

playerskel avatar May 18 '23 21:05 playerskel

Is there any update on this?

RevoSucks avatar Jun 11 '23 14:06 RevoSucks

A "todo list" for this PR (very high level and fragmentary)

I'm partly rewording comments from others, this isn't all from me. I hope I'm not including outdated information Corrections welcome!

Build system

  1. Samples can (and should) be decompressed on extraction, and matching compression exists. However this needs additional information on how to compress the file, which isn't provided by the build system at the moment

  2. It does not use parallel jobs where it could, making it needlessly slow

  3. The binaries are basically assembled from arrays of bytes the python scripts spit out, where we could instead make use of assembler syntax (gas (gnu as)) to produce legible output. It would also be easier to maintain and follow

Data

  1. It's unclear what we can/should commit to the repo and what should be extracted. As it stands, some files being extracted means modifying the audio assets even to simply add things is not git friendly. Committing a few files would help

  2. Some data is extracted too raw and would be a lot more useful with more processing (e.g. "drums and tuning values", I'm told)

Documentation

  1. The new tools written for this PR are very undocumented (both in workings and usage)

  2. The documentation provided by the PR is incomplete, one file simply stops in the middle of a sentence (the one on sequence language iirc)

  3. It would be better to document the audio code fully prior to handling the assets, to have a better (and provably correct) understanding of How Things Work, as well as agreed-upon names. In particular the sequence language interpreter

Dragorn421 avatar Jun 13 '23 08:06 Dragorn421

I've done some work already to address some of the things above, such as the "drums and tuning values" issues and reworking the build system. If anyone would like to collaborate on those feel free to reach out.

We also need feedback from anyone who has already tried to use this and what their experiences were. What worked as expected? What caused problems? If you have used this, please comment or bring it up in the discord. It would be very helpful for determining further improvements.

Thar0 avatar Jun 13 '23 16:06 Thar0

https://discord.com/channels/388361645073629187/882153990798848021/1128799336759836775

Notes: https://discord.com/channels/388361645073629187/388362111534759942/1128944052121907270

Make clean bug? https://discord.com/channels/388361645073629187/451783162859749386/1129545948842963026

(Hm discord)

Dragorn421 avatar Jul 12 '23 22:07 Dragorn421

Few things i noticed while testing this:

  • just compiling doesn't pick up any changes in .seq files, i have to run "setup" every time before compiling
  • because setup ran, it will compile all sequences instead of only the ones i changed
  • I also have a temp-file problem, every compile after a "setup" run creates exactly 4030 empty files in /tmp/
    • this also happens for non-audio changes, just with fewer files, the main repo does not have this problem

For the setup i made a smaller task just for the audio:

setup_audio:
	python3 tools/audio/assemble_sequences.py assets/sequences build/include build
	python3 tools/audio/assemble_sound.py assets/soundfonts build/assets build/include assets/samples --build-bank --match=ocarina -q

For the temp files, no idea where they are from, but i was able to see a few with lsof before the process stopped:

/tmp  lsof -r1 ctm*
COMMAND    PID    USER   FD   TYPE DEVICE SIZE/OFF   NODE NAME
cfe     153941 mbeboek    1w   REG   0,35        0 162047 ctmf8xxGLE
cfe     153941 mbeboek    3w   REG   0,35        0 162047 ctmf8xxGLE

But i did not find any reference to "cfe" in the repo.

I'm testing directly on linux (arch).

HailToDodongo avatar Jul 23 '23 10:07 HailToDodongo

Few things i noticed while testing this:

* I also have a temp-file problem, every compile after a "setup" run creates exactly 4030 empty files in /tmp/
  
  * this also happens for non-audio changes, just with fewer files, the main repo does not have this problem

...

For the temp files, no idea where they are from, but i was able to see a few with lsof before the process stopped:

/tmp  lsof -r1 ctm*
COMMAND    PID    USER   FD   TYPE DEVICE SIZE/OFF   NODE NAME
cfe     153941 mbeboek    1w   REG   0,35        0 162047 ctmf8xxGLE
cfe     153941 mbeboek    3w   REG   0,35        0 162047 ctmf8xxGLE

But i did not find any reference to "cfe" in the repo.

I'm testing directly on linux (arch).

This is due to outdated ido recomp binaries. The older version had a bug that would leave empty temp files around.

hensldm avatar Jul 23 '23 16:07 hensldm

Few things i noticed while testing this:

* I also have a temp-file problem, every compile after a "setup" run creates exactly 4030 empty files in /tmp/
  
  * this also happens for non-audio changes, just with fewer files, the main repo does not have this problem

... For the temp files, no idea where they are from, but i was able to see a few with lsof before the process stopped:

/tmp  lsof -r1 ctm*
COMMAND    PID    USER   FD   TYPE DEVICE SIZE/OFF   NODE NAME
cfe     153941 mbeboek    1w   REG   0,35        0 162047 ctmf8xxGLE
cfe     153941 mbeboek    3w   REG   0,35        0 162047 ctmf8xxGLE

But i did not find any reference to "cfe" in the repo. I'm testing directly on linux (arch).

This is due to outdated ido recomp binaries. The older version had a bug that would leave empty temp files around.

Ah thanks! just merged the latest master in, and the temp files are no longer created.

HailToDodongo avatar Jul 23 '23 16:07 HailToDodongo

This is going to be superseded by a newer audio extraction tooling effort cf the discord for details

Dragorn421 avatar Apr 19 '24 12:04 Dragorn421

Hello, We've officially begun merging Tharo's audio infrastructure to the codebase with https://github.com/zeldaret/oot/pull/2008

Thank you for your work on this, and @MNGoldenEagle for work on the original iteration of this. This will be closed now, but everyone's work does not go unappreciated :)

fig02 avatar Aug 08 '24 04:08 fig02