Hi,
I will admit that my understanding of both ACM drivers as well as the os x audio libraries are not perfect and mostly come from work on this code itself. So i will fully admit it is most likely full of areas needing improvement.
Kristofer Henriksson wrote:
Aric,
I may have been operating under some faulty assumptions. I thought that all mp3 files would "begin at the beginning" and also keep track of how much of the source data the convert function said that it used. I am very careful to set the value of *nsrc to agree with how much source data was successfully used and assume that the calling function with resend the data that is not used. If this is done, it ensures a valid frame at the start of each invocation.
Yes, a proper application should do just that. It should be tracking the value in nsrc and resend unused data at the beginning of the next buffer.
You get a lot of the expected source stream format from MPEG3_StreamOpen. With that i wonder if the presence of an ID3 tag becomes optional. My familiarity with mp3 format makes me unsure if a packet header would also be required.
As far as using AudioFileStream, I tried for a bit to get it to work before I wrote my own custom mp3 parser. I'm running Snow Leopard (10.6.3) and never got the background music to work in Oblivion. One of the problems is that Mp3AudioConverterComplexInputDataProc needs to return an error if it will have more data in the future, rather than returning zero. However, even changing this, AudioFileStream seemed to be skipping a lot of mp3 frames.
The lack of an error in that return is probably an oversight on my part. All the mp3 clips in the game i was working with where sent in one piece so there was only one pass through the loop that consumed all of the data. I am unsure why skipping mp3 frames would occure. My guess is that I am feeding the data in incorrectly and on those boundries between feeds frames are being ignored/corrupted or the like.
If you tell me what are more appropriate assumptions I should be making in my parser, I can fix it to work the way it should. I suppose I am also assuming that it will usually receive at least one full packet, and if this is unjustified I can fix this as well. If I should expect calling functions to ignore the value of *nsrc, then I can just implement an internal single-packet buffer, and this should take care of the problem.
I think the assumptions we need to investigate is the one about there being a packet header at the beginning. Maybe for non variable bit rate mp3s we just get the data given to us when the stream is opened.
In fact, going on a hunch i just got your code to work great with my game!
What i did was this:
I added PACMDRVSTREAMINSTANCE adsi to Mp3GetPacketLength and then when you would return -1 if you could not determin the header I instead returned (192000 * 144) / adsi->pwfxDst->nSamplesPerSec.
I picked that because it shows up in other areas of the code where we need to get a length.
This is exciting also because there where a few mp3s that whre returning 'Fmt?' errors from the stream code and would not play. But with your patch and this change those clips played as well.
Does that give you something to work with? You clearly understand mp3 format better than I.
-aric
There is a bug in the code i just posted, It should clearly be adsi->pwfxSrc->nSamplesPerSec not Dst.
Still works great, probibly because my tests had the same nSamplesPerSec for both Src and Dst.
-aric
Aric Stewart wrote:
Hi,
I will admit that my understanding of both ACM drivers as well as the os x audio libraries are not perfect and mostly come from work on this code itself. So i will fully admit it is most likely full of areas needing improvement.
Kristofer Henriksson wrote:
Aric,
I may have been operating under some faulty assumptions. I thought that all mp3 files would "begin at the beginning" and also keep track of how much of the source data the convert function said that it used. I am very careful to set the value of *nsrc to agree with how much source data was successfully used and assume that the calling function with resend the data that is not used. If this is done, it ensures a valid frame at the start of each invocation.
Yes, a proper application should do just that. It should be tracking the value in nsrc and resend unused data at the beginning of the next buffer.
You get a lot of the expected source stream format from MPEG3_StreamOpen. With that i wonder if the presence of an ID3 tag becomes optional. My familiarity with mp3 format makes me unsure if a packet header would also be required.
As far as using AudioFileStream, I tried for a bit to get it to work before I wrote my own custom mp3 parser. I'm running Snow Leopard (10.6.3) and never got the background music to work in Oblivion. One of the problems is that Mp3AudioConverterComplexInputDataProc needs to return an error if it will have more data in the future, rather than returning zero. However, even changing this, AudioFileStream seemed to be skipping a lot of mp3 frames.
The lack of an error in that return is probably an oversight on my part. All the mp3 clips in the game i was working with where sent in one piece so there was only one pass through the loop that consumed all of the data. I am unsure why skipping mp3 frames would occure. My guess is that I am feeding the data in incorrectly and on those boundries between feeds frames are being ignored/corrupted or the like.
If you tell me what are more appropriate assumptions I should be making in my parser, I can fix it to work the way it should. I suppose I am also assuming that it will usually receive at least one full packet, and if this is unjustified I can fix this as well. If I should expect calling functions to ignore the value of *nsrc, then I can just implement an internal single-packet buffer, and this should take care of the problem.
I think the assumptions we need to investigate is the one about there being a packet header at the beginning. Maybe for non variable bit rate mp3s we just get the data given to us when the stream is opened.
In fact, going on a hunch i just got your code to work great with my game!
What i did was this:
I added PACMDRVSTREAMINSTANCE adsi to Mp3GetPacketLength and then when you would return -1 if you could not determin the header I instead returned (192000 * 144) / adsi->pwfxDst->nSamplesPerSec.
I picked that because it shows up in other areas of the code where we need to get a length.
This is exciting also because there where a few mp3s that whre returning 'Fmt?' errors from the stream code and would not play. But with your patch and this change those clips played as well.
Does that give you something to work with? You clearly understand mp3 format better than I.
-aric
I think the reason why your change works is that it seems the AudioConverter doesn't care if it has invalid data at the beginning of its stream. There will be however a little creeping error in computation of frame lengths over time, which is that there may or may not be one extra byte of padding on any given frame, and you need to look at the frame header to know if it's there. Ignoring the padding is essentially equivalent to ignoring the remainder in the division you do to get the frame size.
I'm still perplexed why the very first call to the conversion function will not start the source data with an ID3 tag or a frame header. The data shouldn't start in the middle of a frame, and there should be nothing else in an MP3 file than frames and ID3.
At any rate, I can fix my code to properly sync to MP3 frames, and then the padding should be properly picked up as well.
Regards, Kristofer
On Thu, May 13, 2010 at 12:46 PM, Aric Stewart aric@codeweavers.com wrote:
There is a bug in the code i just posted, It should clearly be adsi->pwfxSrc->nSamplesPerSec not Dst.
Still works great, probibly because my tests had the same nSamplesPerSec for both Src and Dst.
-aric
Aric Stewart wrote:
Hi,
I will admit that my understanding of both ACM drivers as well as the os x audio libraries are not perfect and mostly come from work on this code itself. So i will fully admit it is most likely full of areas needing improvement.
Kristofer Henriksson wrote:
Aric,
I may have been operating under some faulty assumptions. I thought that all mp3 files would "begin at the beginning" and also keep track of how much of the source data the convert function said that it used. I am very careful to set the value of *nsrc to agree with how much source data was successfully used and assume that the calling function with resend the data that is not used. If this is done, it ensures a valid frame at the start of each invocation.
Yes, a proper application should do just that. It should be tracking the value in nsrc and resend unused data at the beginning of the next buffer.
You get a lot of the expected source stream format from MPEG3_StreamOpen. With that i wonder if the presence of an ID3 tag becomes optional. My familiarity with mp3 format makes me unsure if a packet header would also be required.
As far as using AudioFileStream, I tried for a bit to get it to work before I wrote my own custom mp3 parser. I'm running Snow Leopard (10.6.3) and never got the background music to work in Oblivion. One of the problems is that Mp3AudioConverterComplexInputDataProc needs to return an error if it will have more data in the future, rather than returning zero. However, even changing this, AudioFileStream seemed to be skipping a lot of mp3 frames.
The lack of an error in that return is probably an oversight on my part. All the mp3 clips in the game i was working with where sent in one piece so there was only one pass through the loop that consumed all of the data. I am unsure why skipping mp3 frames would occure. My guess is that I am feeding the data in incorrectly and on those boundries between feeds frames are being ignored/corrupted or the like.
If you tell me what are more appropriate assumptions I should be making in my parser, I can fix it to work the way it should. I suppose I am also assuming that it will usually receive at least one full packet, and if this is unjustified I can fix this as well. If I should expect calling functions to ignore the value of *nsrc, then I can just implement an internal single-packet buffer, and this should take care of the problem.
I think the assumptions we need to investigate is the one about there being a packet header at the beginning. Maybe for non variable bit rate mp3s we just get the data given to us when the stream is opened.
In fact, going on a hunch i just got your code to work great with my game!
What i did was this:
I added PACMDRVSTREAMINSTANCE adsi to Mp3GetPacketLength and then when you would return -1 if you could not determin the header I instead returned (192000 * 144) / adsi->pwfxDst->nSamplesPerSec.
I picked that because it shows up in other areas of the code where we need to get a length.
This is exciting also because there where a few mp3s that whre returning 'Fmt?' errors from the stream code and would not play. But with your patch and this change those clips played as well.
Does that give you something to work with? You clearly understand mp3 format better than I.
-aric
-
Aric Stewart -
Kristofer Henriksson