• 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 Sean Bartell - 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.

Sean Bartell commented on 2011-08-17 16:53

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

Nils Wallménius 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

Andree Buschmann commented on 2011-08-30 14:03

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.

Andree Buschmann commented on 2011-08-30 19:58

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

sideral commented on 2011-08-31 08:02

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)?

Andree Buschmann commented on 2011-08-31 09:49

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()".

Michael Sevakis 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