xenia icon indicating copy to clipboard operation
xenia copied to clipboard

[APU] Fix crash from null sample channel

Open philpax opened this issue 3 years ago • 5 comments

Certain games, such as Forza Motorsport 3, submit XMA data with the stereo flag set with a null second channel. This falls back to mono conversion when the second channel is null, preventing a crash.

philpax avatar Aug 09 '21 17:08 philpax

This seems like only bypassing a symptom, and may have dangerous effects on other code, not only that depends on this part or generates data for it, but overall XMA decoding wrongly assuming that the track has 2 channels (or not providing the input pointer correctly). If this is safe enough, at least an assertion should be made in debug builds that if the track is stereo, there will be valid data for both channels (otherwise we will just forget about this issue and think that it's a well-defined part of our decoder's contract that the stereo flag may be a lie), but this doesn't seem safe overall at a glance. I think more investigation should be done to find out why this situation happens at all. Can you please provide step-by-step info about how to reproduce this crash in Forza Motorsport 3 and other games (it's nice if there are multiple test cases as the actual reason may vary between games)?

Triang3l avatar Aug 09 '21 18:08 Triang3l

Great concerns! You're absolutely right about silently ignoring the channel here potentially causing issues:

      ConvertFrame((const uint8_t**)av_frame_->data, bool(data->is_stereo),
                   raw_frame_.data());
      // decoded_consumed_samples_ += kSamplesPerFrame;

      auto byte_count = kBytesPerFrameChannel << data->is_stereo;
      assert_true(output_remaining_bytes >= byte_count);
      output_rb.Write(raw_frame_.data(), byte_count);
      output_remaining_bytes -= byte_count;

This means that it'll always be writing whatever was previously in the second half of raw_frame_. If we wanted to do this properly, perhaps it's worth copying the first channel to the second channel whenever this case happens?

As for a repro, I've only really been testing with FM3, but it seems to happen somewhat inconsistently for me during races and at the post-race screen. I would say probably when there are specific sound effects happening during gameplay; at one point I was getting it consistently after a race.

In terms of stability: this issue appears to have been responsible for all the crashes I've been experiencing in FM3. I was able to successfully complete several races without crashing with this fix applied, so addressing the issue (even if not with this fix) should improve crash numbers.

philpax avatar Aug 09 '21 18:08 philpax

This problem occurs in Hydro Thunder Hurricane in regular gameplay.

I don't think it's a matter of the input having a single channel and being flagged as stereo, but FFmpeg only giving a single channel back from the decoded data.

gibbed avatar Aug 09 '21 21:08 gibbed

That seems reasonable, but I don't have enough domain knowledge to understand what would cause that - the obvious speculation is that the XMA decoder isn't producing valid data for FFmpeg to decode for the second channel, but I wouldn't know how to investigate that.

What would be the best way to approach this? It appears that this workaround makes several games playable where they weren't before, but it would also be masking a potential decoding error.

philpax avatar Aug 10 '21 12:08 philpax

it would be helpful if somebody can dump (a portion of) a stream. I.e. beginning with the buffer pointer and maybe even up to the end (if the chunk is small enough so it wouldnt violate copyright laws)

JoelLinn avatar Aug 22 '21 20:08 JoelLinn