Skip to content

Make an API that can extract tracked data from the relevant replayers#131

Open
mywave82 wants to merge 1 commit into
adplug:masterfrom
mywave82:track-export
Open

Make an API that can extract tracked data from the relevant replayers#131
mywave82 wants to merge 1 commit into
adplug:masterfrom
mywave82:track-export

Conversation

@mywave82

@mywave82 mywave82 commented Feb 6, 2022

Copy link
Copy Markdown
Contributor

Base class and relevant players implements the follow new API calls

  • getpattern(int order) resolves order to pattern-index
  • getrows() how long are each pattern (normally 64)
  • getnchans() how many channels/tracks are each pattern
  • gettrackdata() return track data for a given pattern/row/channel combinations (some replayers could benefit from unfolding tracked data from byte-streams into actually pattern-arrays to speed this up)

Bump the .so file version, since the core base-class has changed slightly

CxadhybridPlayer

  • Added some buffer-overflow protections in loader/player

This makes it possible to expose visual tracker data from some of the replayers:
Screenshot at 2022-02-06 13-15-17

@Malvineous

Copy link
Copy Markdown
Member

Is there any way this could be done without doubling the amount of code that has to be maintained going forward? It looks like this adds a duplicate parser to each supported format - one for the player, and one to return the tracked data. It means both those parsers now have to be maintained even though they both interpret the same file data.

It would be nicer if there was only one parser, which was used for both playback and for returning the tracked data. As it stands now, because it's an entirely independent piece of code, it could be merged into OCP instead of AdPlug and it would work just the same.

I have nothing against the idea of AdPlug returning tracked data, but I'm not overly keen on having two implementations side by side that are different but do the same thing, especially when this new code doesn't have any unit tests to confirm whether it's producing the right output or not.

I am wondering whether you might get a better result by extracting the OPL data generated by AdPlug and parsing that into the OCP event stream. While this won't let you see the tracker effects in the underlying file, it will mean that it will work for all supported file formats (now and in the future, with no special support from AdPlug) and there won't be any redundant code in any of the file format handlers.

@mywave82

mywave82 commented Feb 6, 2022

Copy link
Copy Markdown
Contributor Author

If keeping the adplug library as pure as possible and let OCP extract information.

Most data needed are currently in private areas. Moving them into protected would make it possible make inherited classes for most of them that would live in OCP code-base instead

@Malvineous

Copy link
Copy Markdown
Member

Could you do that with friend classes to avoid needing changes to AdPlug? Otherwise changing them from private to protected should be ok. But it does mean you end up with a lot of Adlib-specific code in OCP.

@mywave82

Copy link
Copy Markdown
Contributor Author

Using protected + friend should be possible (I am not a C++ guru, but I usually reach my goals with some trial and errors), but that leaves you with responsibility to bump the .so version every time a class-layout changes which is easy to forget.

Using protected alone is not convenient as the Cmodplayer has many subclasses in order to override load(), and each of them would need a new child unless using "illegal" typecasting.

Third option is that I maintain a fork. I already do this for for another library for similar reasons - that I am the solo use-case that wants to extract verbose information for visualization.

@binarymaster

Copy link
Copy Markdown
Member

Very interesting feature. Since I use XMPlay as default player for everything, it could allow using MOD Pattern Display visuals when using an applicable plugin.

Please rebase your commits on top of master branch, some build warnings/errors may be fixed.

Comment thread src/lds.cpp Outdated
high = (comhi + transp) << 4;
}

if(chandelay[chan]) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src\lds.cpp(271): error C4700: uninitialized local variable 'chan' used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I have two options here.

a) easy way - remove the entire if() and not fix chandelay

b) add chandelay effect if a note is played and there are no other effects. (The chandelay seem to be a global feature per channel on this fileformat)

@ciros88

ciros88 commented Aug 13, 2025

Copy link
Copy Markdown

would be lovely having it!

@mywave82

Copy link
Copy Markdown
Contributor Author

I rebased in the latest patch from my ocp (Open Cubic Player) branch

 * getpattern(int order) resolves order to pattern-index
 * getrows() how long are each pattern (normally 64)
 * getnchans() how many channels/tracks are each pattern
 * gettrackdata() return track data for a given pattern/row/channel combinations (some replayers could benefit from unfolding tracked data from byte-streams into actually pattern-arrays to speed this up)

Bump the .so file version, since the core base-class has changed slightly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants