Pre-configured decoding calls have their result overwritten

[ace-n]

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! 😃

[mackron]

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: #765 (comment)

[ace-n]

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.)*

[mackron]

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: 69df19f