miniaudio icon indicating copy to clipboard operation
miniaudio copied to clipboard

Pre-configured decoding calls have their `result` overwritten

Open ace-n opened this issue 1 year ago • 2 comments

Summary

When ma_decoder_init_file is provided a decoderConfig with a specific encodingFormat, the "fallback" decoding calls can overwrite the result value from the initial specific-format decoding call.

Example Scenario

Call ma_decoder_init_file with a preconfigured encodingFormat and a bad filepath as shown below:

int CHANNEL_COUNT = 2;
int MAX_FRAMES_READ = 100;

const char *filepath = "BAD PATH";
ma_decoder decoder;

ma_format FORMAT = ma_format_f32;
ma_decoder_config decoderConfig = ma_decoder_config_init(
    FORMAT,
    CHANNEL_COUNT,
    48000);
decoderConfig.encodingFormat = ma_encoding_format_mp3;

ma_result decoderInitResult = ma_decoder_init_file(
    filepath,
    &decoderConfig,
    &decoder);

What happens

  • The initial (MP3-specific) decoding operation returns an MA_INVALID_FILE error code.
  • The fallback decoding calls overwrite that with MA_NO_BACKEND.

In the end, decoderInitResult resolves to MA_NO_BACKEND.

What should happen

Ideally, the MA_INVALID_FILE error would not have been overwritten!

Potential fixes

1) Log format-specific errors

Log the initial error code (in this case, MA_INVALID_FILE) and return the error code from the fallback decoding calls.

2) Preserve format-specific error codes

Preserve (and return) the initial error code while still performing the fallback decoding calls.

Note: returning the initial error code immediately would be a breaking change, as the default decoders would no longer be called if the specified-format decoding call fails.

Of these two options, my preference is for the second.

Next steps

@mackron if you're on board with this, let me know:

  • whether to create a PR
  • if so, which of these approaches you prefer

Thank you! 😃

ace-n avatar Oct 06 '24 20:10 ace-n

Thanks. I'm going to sit on this for the moment. In the dev-0.12 branch I've actually done a complete overhaul of the decoding backend selection logic and I'm not sure off the top of my head if what you've mentioned still applies. But I'm also not sure if I agree with your suggestions just yet and want to consider it. If you're curious, here's my comment on the changes in dev-0.12: https://github.com/mackron/miniaudio/issues/765#issuecomment-1972063577

mackron avatar Oct 06 '24 21:10 mackron

Thanks for the quick response!

Sounds good - as long as you're aware of this. 🙂

(If this behavior is intended behavior or can't be reproduced in the dev branch, feel free to close this issue.)

ace-n avatar Oct 06 '24 22:10 ace-n

So I've taken a look at what I'm doing in the dev-0.12 branch and it was indeed overwriting the result code and returning MA_NO_BACKEND at all times in the result of an error. I've updated this in the dev-0.12 branch, but I'm going to leave it as-is in the 0.11.x cycle because 0.12 is going to be replacing all of that backend selection logic anyway.

This is the change I made in case it might be of interest to you: https://github.com/mackron/miniaudio/commit/69df19f0b6330d597f0cc278e4adb2eda33cd117

mackron avatar Jan 11 '25 07:01 mackron