bass-sys icon indicating copy to clipboard operation
bass-sys copied to clipboard

Signatures of TagBext, TagCart, TagCue and TagSample are incorrect

Open thatbakamono opened this issue 2 years ago • 0 comments

As described in the title, the signatures of TagBext, TagCart, TagCue and TagSample are incorrect. These four structures use a mechanism called flexible array member, as you can see below:

typedef struct {
	char Description[256];			// description
	char Originator[32];			// name of the originator
	char OriginatorReference[32];	// reference of the originator
	char OriginationDate[10];		// date of creation (yyyy-mm-dd)
	char OriginationTime[8];		// time of creation (hh-mm-ss)
	QWORD TimeReference;			// first sample count since midnight (little-endian)
	WORD Version;					// BWF version (little-endian)
	BYTE UMID[64];					// SMPTE UMID
	BYTE Reserved[190];
#if defined(__GNUC__) && __GNUC__<3
	char CodingHistory[0];			// history
#elif 1 // change to 0 if compiler fails the following line
	char CodingHistory[];			// history
#else
	char CodingHistory[1];			// history
#endif
} TAG_BEXT;
typedef struct
{
	char Version[4];				// version of the data structure
	char Title[64];					// title of cart audio sequence
	char Artist[64];				// artist or creator name
	char CutID[64];					// cut number identification
	char ClientID[64];				// client identification
	char Category[64];				// category ID, PSA, NEWS, etc
	char Classification[64];		// classification or auxiliary key
	char OutCue[64];				// out cue text
	char StartDate[10];				// yyyy-mm-dd
	char StartTime[8];				// hh:mm:ss
	char EndDate[10];				// yyyy-mm-dd
	char EndTime[8];				// hh:mm:ss
	char ProducerAppID[64];			// name of vendor or application
	char ProducerAppVersion[64];	// version of producer application
	char UserDef[64];				// user defined text
	DWORD dwLevelReference;			// sample value for 0 dB reference
	TAG_CART_TIMER PostTimer[8];	// 8 time markers after head
	char Reserved[276];
	char URL[1024];					// uniform resource locator
#if defined(__GNUC__) && __GNUC__<3
	char TagText[0];				// free form text for scripts or tags
#elif 1 // change to 0 if compiler fails the following line
	char TagText[];					// free form text for scripts or tags
#else
	char TagText[1];				// free form text for scripts or tags
#endif
typedef struct
{
	DWORD dwCuePoints;
#if defined(__GNUC__) && __GNUC__<3
	TAG_CUE_POINT CuePoints[0];
#elif 1 // change to 0 if compiler fails the following line
	TAG_CUE_POINT CuePoints[];
#else
	TAG_CUE_POINT CuePoints[1];
#endif
} TAG_CUE;
typedef struct
{
	DWORD dwManufacturer;
	DWORD dwProduct;
	DWORD dwSamplePeriod;
	DWORD dwMIDIUnityNote;
	DWORD dwMIDIPitchFraction;
	DWORD dwSMPTEFormat;
	DWORD dwSMPTEOffset;
	DWORD cSampleLoops;
	DWORD cbSamplerData;
#if defined(__GNUC__) && __GNUC__<3
	TAG_SMPL_LOOP SampleLoops[0];
#elif 1 // change to 0 if compiler fails the following line
	TAG_SMPL_LOOP SampleLoops[];
#else
	TAG_SMPL_LOOP SampleLoops[1];
#endif
} TAG_SMPL;

It was implemented in bass-sys code like that: https://github.com/KernelErr0r/bass-sys/blob/be2537c57b771b6baddf323d33bf7f5582d908da/src/types.rs#L353-L365 https://github.com/KernelErr0r/bass-sys/blob/be2537c57b771b6baddf323d33bf7f5582d908da/src/types.rs#L411-L434 https://github.com/KernelErr0r/bass-sys/blob/be2537c57b771b6baddf323d33bf7f5582d908da/src/types.rs#L515-L520 https://github.com/KernelErr0r/bass-sys/blob/be2537c57b771b6baddf323d33bf7f5582d908da/src/types.rs#L562-L575 and that's plain wrong. FAM doesn't provide length and even if it did, I wouldn't bet my life on rustc and Rust's inner implementation of Vec.

There is an RFC for dealing with these kinds of situations easily but unfortunately, it's just an RFC, it isn't a part of Rust.

I found a working way of doing that, it makes use of ZST to represent FAM and encapsulates everything in a Box-like wrapper struct to provide a nice API to deal with FAM, unfortunately, it comes with its own issues, you can see it described in details here.

// As a side note: I could also use ZST to represent FAM and leave everything else to the user, it kinda makes sense because these are pure FFI bindings and I shouldn't put any abstraction here, but I don't want to make the user deal with this stupid shit.

thatbakamono avatar Nov 19 '21 23:11 thatbakamono