Ogg Player - from file or buffer example

Finally had a chance to test this code - works fine

>>>Amendment<<<
In my haste I posted the original code changing it from memory I made a few bads & looked things up (badly!), I never kept the fixed version since I didn’t want to use it. But it looks safe for the wii platform for the foreseeable future; at the very worst it will just not play anything. Hopeful this is all correct now, my apologues. (please read the other oggplayer thread for background insight)

//setup
int fd = open ("noeggonmyface.ogg", O_RDONLY);  // correction was ... O_WRONLY

// from file don't forget to  include unistd.h and fcntl.h
int fd = open ("noeggonmyface.ogg", O_WRONLY); // <--- see above correction
PlayOgg(&fd, 0, 0, OGG_ONE_TIME);

//from buffer
// .. use fopen and use fseek to get size
u8* pOggData = (u8*) malloc(OggSize);
fread( pOggData, OggSize, 1, pOggFile);
PlayOgg(pOggData, OggSize, 0, OGG_ONE_TIME);

// mem_open 
If ((*ogg==0x4f) && (*(ogg+1) == 0x67))  // correction (I have puss for brains)
//from buffer
Else
//from file

Forgot to say add this bit of code and it all starts working.

static int mem_open(char * ogg, int size)
{
	static int one = 1; // this could all be optimise down to use single structure - most this is pointless
	int n;
	if (one)
	{
		one = 0;
		for (n = 0; n < 4; n++)
			file[n].size = 0;
	}

	for (n = 0; n < 4; n++)
	{
		if (file[n].size == 0)
		{
			file[n].mem = ogg;
			file[n].size = size;
			file[n].pos = 0;
			if (*ogg < 0x666)  // <--- see above correction			      return (*ogg);
			else			
	                                    return (0x666 + n);
		}
	}

	return -1;
}

Edited 3 time(s). Last edit at 09/25/2011 11:04AM by Titmouse.

Fail. "if (*ogg < 0x666)" will always be true because the "ogg" argument of mem_open is a char* (dereferenced value can't be greater than 0xff). So you broke it for the playing from a memory buffer case, which is what PlayOgg() is actually meant to do.

If you want to play from a file make a new function (ie "PlayOggFile") instead of adding hacks.

Drum roll please... It’s now fixed, it will play from file & buffer.

Yes I’ve made a mistake, here’s a quick fix from what I can remember as I no longer have that code since I've already stripped all the hacky stuff from my version of oggplayer also removing the unused buffers and some silly logic that can be done once at setup.

I don’t need this or like it but it does work, I purely did this out of curiosity to why/how oggplayer worked in the past before something was lost.

If ((*ogg==0x4f) && (*(ogg+1) == 0x67))
//from buffer
Else
//from file

Consider this,
You want to play a song from a buffer, so naturally you call this function and put this buffer as a parameter.
For the majority of cases this would work, but what if the first two chars in this buffer are 0x4F and 0x67. Then, instead of playing from your buffer, it calls a function PlayOggFromFILE(buffer). Then, since it isn't a file, your program crashes. Wouldn't it just be easier to not have to worry, and to just create two function, PlayOggFromMem(u8*) and PlayOggFromFile(FILE*). That could possible save you debugging time, and you'll never have to worry about playing an Ogg.

G_man, sorry no. I even go as far as saying it will never fail as long as the Ogg header is of that type. I was shocked how reliable it would be. (I’ve used the word never - I can feel the flames already)

Don’t forget we are passing a complete Ogg buffer.
The Ogg buffer is complete with header information of “4f67” in network order (i.e. the first two letters in the header are ox4f=’O’, ox67 =’g’ as in “Oggs” - load up a ogg in a hex editor and you will see what I mean)

I don't have a problem with the word "never". Here's an example: the code in your first post would never work because you're opening the .ogg file in write only mode.

Well spotted – No idea what I was thinking at the time of the post as my original test code used ZERO.
Probably stuck the named #define in at the last minute picking the wrong one from the web - oh well.

But anyway I’ve proved it does work the way I said it would even though you said it can’t.

The “devkitPro\examples\wii\audio\oggplayer\source\oggplayer.c” code is just old and I don’t like it when someone second guesses and bitches about other people’s code.
Well to Francisco Muñoz 'Hermes' (author), and Tantric (corrections) for taking the time to give us the example.

I never said it wouldn't work, I said it was a very bad thing to do and if you write code like this you should be ashamed of yourself.

This method does go against the grain, but it does appear to be safe. Techniques like this do come in handy from time to time; it's outright stupid dismissing this technique from every single future design. I’m amazed you even said that!
I would be more than happy to hold up my hand I say, yes I’ve use that technique giving a justification for its use.

I'm not sure if this is what the original author had that in mind, but I'm willing to take the hit.

"Techniques like this" should only be used when there is no other option. 2 people have already suggested a much saner method of declaring a new function specifically designed to take a file as input instead of a memory buffer, yet you've ignored them and pushed forward with your hacky method that assumes open() will always return a low value for the fd - this simply isn't true and assumptions like this ("it works for me therefore it's safe") are how cross-platform incompatibilities/time-bomb bugs are created.
There are so many different ways to achieve what you're trying to do and you picked one of the worst.

No this is not true, what exactly is your game – you keep trying to twist what I say?
You know full well why I got this working; it was a purely academic exercise just to see if it was possible, we might not all be at your level of knowledge so we have to learn and improve ourselves by trying things out.

In this tread I said: “… I've already stripped all the hacky stuff from my version of oggplayer also removing the unused buffers and some silly logic that can be done once at setup.”

I’ve also said: “ …I'm going to break the code into two parts since I don't require the FILE support. If the File stuff does not work then I might as well just delete those parts from my version and benefit from the tiny optimisations.” (In the origianl “odd player” thread)