Garbled audio due to incorrect parameters on some codecs

[anevilyak]

I haven't yet been able to bisect what revision this regression was introduced in, but at least in hrev50216 AAC and Ogg Vorbis decoding appears to be broken. The audio plays with stuttering artifacts, which appears to be due to something along the chain deciding that the format sample size is 32 bits rather than 16. This occurs for both standalone audio files and audio tracks in video files. This problem is not observable with either MPEG 2 or 3 audio.

[anevilyak]

I should add that this was on an x86 primary host, ergo gcc5 and respective ffmpeg package.

[korli]

Tested on x86_64, works on hrev49856, doesn't on hrev50072.

[korli]

on x86_64, I tested with success this patch: it simply skips libswr, and interleaves samples.

diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
index 27ec813..a18f4d9 100644
--- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
+++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp
@@ -927,6 +927,7 @@ AVCodecDecoder::_MoveAudioFramesToRawDecodedAudioAndUpdateStartTimes()
        // Some decoders do not support format conversion on themselves, or use
        // "planar" audio (each channel separated instead of interleaved samples).
        // In that case, we use swresample to convert the data
+#if 0
        if (av_sample_fmt_is_planar(fContext->sample_fmt)) {
                const uint8_t* ptr[8];
                for (int i = 0; i < 8; i++) {
@@ -950,6 +951,21 @@ AVCodecDecoder::_MoveAudioFramesToRawDecodedAudioAndUpdateStartTimes()
 
                if (frames < 0)
                        debugger("resampling failed");
+#endif
+       if (fOutputFrameSize != fInputFrameSize) {
+               // planar audio
+               uintptr_t out = (uintptr_t)fRawDecodedAudio->data[0];
+               int32 offset = fDecodedDataBufferOffset;
+               for (int i = 0; i < frames; i++) {
+                       for (int j = 0; j < fContext->channels; j++) {
+                               memcpy((void*)out, fDecodedDataBuffer->data[j]
+                                       + offset, fInputFrameSize);
+                               out += fInputFrameSize;
+                       }
+                       offset += fInputFrameSize;
+               }
+               outFrames = frames;
+               inFrames = frames;
        } else {
                memcpy(fRawDecodedAudio->data[0], fDecodedDataBuffer->data[0]
                                + fDecodedDataBufferOffset, frames * fOutputFrameSize);

[pulkomandy]

I would still use av_sample_fmt_is_planar to detect planar formats, instead of trying to guess from the input/output sizes.

Also, the libswr is able to do other conversions (float<>int or whatever), which may be required by some codecs where only float output is supported.

So, this needs careful testing with a lot of audio/video files (for example there are a lot at ​http://samples.ffmpeg.org), to make sure we don't need something else.

It's a bit sad that we can't get libswr working and we have to rewrite it, however :/

[korli]

Thanks for the feedback!

I would still use av_sample_fmt_is_planar to detect planar formats, instead of trying to guess from the input/output sizes.

Yeah I guess.

Also, the libswr is able to do other conversions (float<>int or whatever), which may be required by some codecs where only float output is supported.

AFAIK the plugin maps the avcodec sample format to the media kit sample format, this means we don't require any format. I don't understand what might be required then.

So, this needs careful testing with a lot of audio/video files (for example there are a lot at ​http://samples.ffmpeg.org), to make sure we don't need something else.

Sure.

It's a bit sad that we can't get libswr working and we have to rewrite it, however :/

Yeah, or avfilter. In this case though, we don't really need some audio conversion, just another structure. IMO it's not too bad if we can skip a dependency on libswr.

[pulkomandy]

We always request a sample format that matches what we want to output, but some codecs in ffmpeg may ignore the request (we also request non-planar output, and they ignore that). It depends on the format used, and also on the version of ffmpeg.

This is why I tried to go with swr: it would solve all the possible mismatches and always make sure we ouput exactly what was requested. It is a bit nicer and more predictible for apps and they don't have to do the format conversion on their side this way.

But, as it doesn't work reliably, we can go with a simple rematrixing as you did, it should cover most cases for now.

[korli]

Applied in hrev50639.

[pulkomandy]

I guess that fixes the issue (no working soundcard on this laptop so can't be sure).

[anevilyak]

I'll try to verify this locally tonight.

[khallebal]

I can confirm that the stuttering is gone, i also mentioned this behaviour with AAC,FLAC, AC3 & OGG but not with MP3 in my ticket #12594.