• Status Unconfirmed
  • Percent Complete
  • Task Type Patches
  • Category Codecs
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by wtachi - 2011-07-11

FS#12189 - Simplify the codec API and don't always loop MODs.

This patch makes some changes to codec_api to help remove dependencies for my project.

The unused strcasestr callback is removed.

The global_settings pointer was being used by several codecs to check whether REPEAT_ONE mode is on; this has been replaced with a callback "should_loop()".

The MOD codec has been modified not to restart the file when it reaches the end. This does not affect MOD files that use "position jump" to loop part of the song. Ideally the MOD codec would only follow such loops if should_loop() returns 1, but detecting loops is nontrivial.

Updated for libgme and to make the MOD fix actually work.

nls commented on 2011-08-30 13:33

I don't see why the mod change is relevant here and i think it should be separated out and submitted as a separate patch otherwise this looks good to me

I also think the mod-changes should be handled separately. Removing global_settings from the codec interface is a good thing imho, but the naming "should_loop()" is a bit strange… "repeat_track()" or "loop_track()" better fits to my personal taste.

The changes to codec API have been submitted with r30391. The mod changes are left (see attached patch).

The codec API change (remove global_settings pointer from codec interface) breaks FS#11891 (Add mp3 gap skipping support to improve gapless playback).

I understand the motivation for removing the global_settings member. However, that change also removes a generic mechanism for configuring codecs. What would now be a good way to configure a codec – either in a generic way, or if that's impractical, in this specific case (boolean flag to turn codec-specific gap elision on or off)?

We might discuss this general topic via ML or on irc. If you want to update FS#11891 you might just add a dedicated function similar to "loop_track()".

MikeS commented on 2011-08-31 14:21

One idea: Use a codec configuration struct that isn't global_settings, passed from the codec to a function in the codec api, that returns config parameters. In a core build, they can copy from global_settings or anywhere else, outside the core in some app, they may be filled-in however the program determines is best.


Available keyboard shortcuts


Task Details

Task Editing