Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup some MP3 SDK version differences #13981

Merged
merged 4 commits into from
Jan 27, 2021

Conversation

unknownbrackets
Copy link
Collaborator

This matches validation better with firmware differences depending on the compiled SDK version.

That's how you make other bitrates work. I misunderstood the code before, the codecCtx sample rate is the detected sample rate. So this just makes us not resample - and we should do it for all valid sample rates, including 8kHz.

Also made it not crash when a game decodes with a NULL pcm offset pointer. This is valid, probably for skipping ahead.

-[Unknown]

The 6.xx behavior might be important if a game relies on it to add data.
Just like other audio decoding, you're allowed to skip audio.
Also prevents a crash if the mp3 is not yet inited.
Our codec context is updated with the source sample rate, so this makes us
not resample at all.

Converting to stereo still seems correct.
@sum2012
Copy link
Collaborator

sum2012 commented Jan 27, 2021

Will you try real case for sceMp3Decode return ERROR_AVCODEC_INVALID_DATA ?

ctx->MaxOutputSample = CalculateMp3SamplesPerFrame(versionBits, layerBits);
ctx->freq = ctx->SamplingRate;
int bitrate = __CalculateMp3Bitrates((header >> 12) & 0xF, versionBits, layerBits);
int samplerate = __CalculateMp3SampleRates((header >> 10) & 0x3, versionBits);;

DEBUG_LOG(ME, "sceMp3Init(): channels=%i, samplerate=%iHz, bitrate=%ikbps, layerBits=%d ,versionBits=%d,HEADER: %08x", ctx->Channels, ctx->SamplingRate, ctx->BitRate, layerBits, versionBits, header);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don' t think this log still okay.
You have deleted ctx->SamplingRate,ctx->Channels,ctx->BitRate,ctx->MaxOutputSample,ctx->freq

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, you're right.

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

Will you try real case for sceMp3Decode return ERROR_AVCODEC_INVALID_DATA ?

Later, I get these too in my tests, and it's related to the amount of data queued I think. I don't think it's as necessary and getting it to trigger at the same times will be tricky.

A thing that complicated this is that we're not even decoding mp3s to the same data. I was comparing to make sure the sample rate should be the same, but there seems to be additional 0s that are produced from the PSP firmware. This could effect timing and is all related.

-[Unknown]

@hrydgard hrydgard added the MP3 Issue involves sceMp3 features. label Jan 27, 2021
@hrydgard hrydgard merged commit 97e7a28 into hrydgard:master Jan 27, 2021

DEBUG_LOG(ME, "sceMp3Init(): channels=%i, samplerate=%iHz, bitrate=%ikbps, layerBits=%d ,versionBits=%d,HEADER: %08x", ctx->Channels, ctx->SamplingRate, ctx->BitRate, layerBits, versionBits, header);
DEBUG_LOG(ME, "sceMp3Init(): channels=%i, samplerate=%iHz, bitrate=%ikbps, layerBits=%d ,versionBits=%d,HEADER: %08x", channels, samplerate, bitrate, layerBits, versionBits, header);

if (layerBits != 1) {
// TODO: Should return ERROR_AVCODEC_INVALID_DATA.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@unknownbrackets Will you test this case ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've tested it, including now with newer firmware. This is the error code it returns on real firmware.

That said, I was worried that there might be more to it or cases, or that it might be making someone's custom MP3s work. It didn't seem important to block so I just added reporting.

It gets reported a lot but I think it's either game hacks or custom music files:
https://report.ppsspp.org/logs/kind/1041
(most of the games are PES hacks.)

-[Unknown]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MP3 Issue involves sceMp3 features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants