Ticket #5490 (closed Patches: Committed)

Opened 6 years ago

Last modified 6 years ago

Switching to Timidity 2.13.2

Reported by: oldnemesis Owned by: spiff
Priority: 4 - Normal Milestone: 9.04 "Camelot"
Component: Other (un-categorized if does not fit anywhere else) Version: GIT
Severity: Normal Keywords:
Cc: spiff Blocked By:
Blocking: Platform: All
Revision:

Description

Upgraded Timidity to version 2.13.2. Timidity 2.13.2 provides the following advantages:

  • Better sound quality due to better (but more CPU-hungry in terms of 2004) implementation of resampling, effects, and reverb;
  • Support for soundfonts;
  • Handles modern MIDI files as it properly supports more events.
  • (there is actually much more, I just listed what I know)

The following changes are made to Timidity core:

  • Timidity converted to library with similar API (added to timidity.c and playmidi.c)
  • Since sound output is now going through default audio queue (aq_), we're now getting wave data through added buffer output interface.
  • Fixed numerous memory leaks on Timididy shutdown. We're a shared library now :)
  • Removed support for non-MIDI files (500K of code). Could be added back if anyone really want to use it to handle MOD/XM/S3M and other Really Old Stuff from tracker scene. Saved almost 600K though, so makes sense.
  • Removed URL and archives support as we're supposed to handle only .mid/.kar files using wrapped fopen() which will take care of everything.
  • Added support for working without configuration file if there is a soundfont file at path q:\system\players\paplayer\timidity\soundfont.sf2, in which case no configuration is needed.

The following changes are made to the Timidity integration:

  • Added ErrorMsg() function which returns error message to make the API less ugly;
  • Added Cleanup() function which should be called before unloading the shared library;
  • Integrated bugfixes from patch #5461 (which could be now discarded as the code changed);
  • The patch is integrated into XBMC build system (less work for ya :)

Known problems:

  • MIDI files play a little faster using current SVN on ALSA output. I backported the changes down to 16447, and it looks like this issue is only affected revisions 16538 and up. Music plays fine on 16537 and below. I see there were some changes in player. Please someone more experienced take a look on that :)

Attachments

timidity.patch.gz Download (49.3 KB) - added by oldnemesis 6 years ago.
patch to branches/timidityupdate - apply from the root

Change History

Changed 6 years ago by oldnemesis

patch to branches/timidityupdate - apply from the root

comment:1 Changed 6 years ago by spiff

hi,

i have applied the diff to the branch. since i do not have much time to play can you please co and confirm everything is as intended?

also; i could not reproduce the pitch issues consistently. i had a glitch but i think that was just a bug in my alsa driver. now everything seems to work fine. watch your log for 'requested samplerate (%d) not supported by hardware, using %d instead'. if so only thing i can suggest is changing to 44k1. i would be fine with that as most soundfonts arent really 48k worthy anyways.

comment:2 Changed 6 years ago by spiff

  • Owner set to spiff
  • Status changed from new to assigned

comment:3 Changed 6 years ago by oldnemesis

Hi,

The result code compiles on Linux and works as supposed to. Didn't test on Windows or OS X.

The pitch issue is still there. The pitch is higher, and the melody plays faster. No errors in log, and the only warning is "WARNING: CALSADirectSound::CALSADirectSound - device is not able to pause playback, will flush instead."

comment:4 Changed 6 years ago by spiff

:

dunno what to do when i cannot reproduce it consistently. we plan to remove the internal resampling anyways. does switching to 44k1 fix it at your end?

also i only tested linux as well - the others will have to take care of those platforms, not my side of things:)

comment:5 Changed 6 years ago by oldnemesis

Sorry, forgot to add. Sampling at 44100 seems to work fine, the pitch is correct.

comment:6 Changed 6 years ago by spiff

rite. then i suggest we go for 44k1 for now as that should allow you to get further with the other diffs. i have opened a ticket for the resampling issue as it really needs to be solved (alot of other stuff output 48k as well).

if you agree i will do the 48k -> 44k1 change, no need for a diff

comment:7 Changed 6 years ago by oldnemesis

I agree (and I won't hear the difference between 48 and 44k1 anyway)

comment:8 Changed 6 years ago by spiff

  • Status changed from assigned to closed
  • Resolution set to Commited to SVN

it's in! cheers

Note: See TracTickets for help on using tickets.