mp2 decoding of decoding_encoding.c API example is broken

[Sylvain Demongeot]

Summary of the bug:

doc/examples/decoding_encoding doesn't decode test.mp2 properly, i.e. the produced test.sw has incorrect contents.

How to reproduce:

Build the API examples from the latest source, then run decoding_encoding with argument "mp2"

git clone ​git://source.ffmpeg.org/ffmpeg.git ffmpeg
cd ffmpeg/
./configure
make
make examples
doc/examples/decoding_encoding mp2

The produced test.sw has random contents (although parts of the signal are similar to that of test.mp2). For example:

# doc/examples/decoding_encoding mp2
Encode audio file test.mp2
Decode audio file test.mp2 to test.sw
# md5sum test.*
ca203ada0aa42372edead60fd4dd928b test.mp2
9db91386e3cbdc8ff4069fe93feb818a test.sw
# doc/examples/decoding_encoding mp2
Encode audio file test.mp2
Decode audio file test.mp2 to test.sw
# md5sum test.*
ca203ada0aa42372edead60fd4dd928b test.mp2
5921a1f95ac5bf33510d58ffb7b90279 test.sw

[Carl Eugen Hoyos]

Replying to sdem:

doc/examples/decoding_encoding doesn't decode test.mp2 properly, i.e. the produced test.sw has incorrect contents.

What do you mean with "incorrect contents"?

[Sylvain Demongeot]

I assume "test.sw" should contain raw decoded samples (16 bits, signed) of "test.mp2" (left channel only, I further assume). I opened "test.sw" in "Amadeus", an audio editor, and the signal is very distorted.
Below is a link to a screen shot comparing the original audio ('test.mp2', as decoded by Amadeus) and two instances of "test.sw", made by running "decoding_encoding" twice. It appears that the decoded samples don't match the original everywhere. Also, there is some randomness in the distortion. I would expect the program to produce the same output when run several times, but it is not the case.
NB: the screen-shot only shows a fraction of the first second for clarity, but the signal is distorted throughout.

​http://www.wildbits.com/etc/ffmpeg/decoding_encoding_issue.png

[Carl Eugen Hoyos]

Replying to sdem:

I assume "test.sw" should contain raw decoded samples (16 bits, signed) of "test.mp2" (left channel only, I further assume). I opened "test.sw" in "Amadeus"

Why didn't you open it with FFmpeg?

[Clément Bœsch]

☭ valgrind doc/examples/decoding_encoding_g mp2
==26166== Memcheck, a memory error detector
==26166== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==26166== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
==26166== Command: doc/examples/decoding_encoding_g mp2
==26166== 
Encode audio file test.mp2
Decode audio file test.mp2 to test.sw
==26166== Syscall param write(buf) points to unaddressable byte(s)
==26166==    at 0x69109B0: __write_nocancel (in /usr/lib/libc-2.18.so)
==26166==    by 0x68ABFE2: _IO_file_write@@GLIBC_2.2.5 (in /usr/lib/libc-2.18.so)
==26166==    by 0x68AB6A2: new_do_write (in /usr/lib/libc-2.18.so)
==26166==    by 0x68AC5F5: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.18.so)
==26166==    by 0x68A273C: fwrite (in /usr/lib/libc-2.18.so)
==26166==    by 0x4481D0: audio_decode_example.constprop.6 (decoding_encoding.c:310)
==26166==    by 0x4477E1: main (decoding_encoding.c:642)
==26166==  Address 0x7023680 is 0 bytes after a block of size 2,304 alloc'd
==26166==    at 0x4C29D00: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26166==    by 0x4C29E17: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26166==    by 0x9AE6E9: av_malloc (mem.c:94)
==26166==    by 0x9A2A57: av_buffer_alloc (buffer.c:70)
==26166==    by 0x9A337B: av_buffer_pool_get (buffer.c:305)
==26166==    by 0x7A59A6: avcodec_default_get_buffer2 (utils.c:631)
==26166==    by 0x7A632E: ff_get_buffer (utils.c:973)
==26166==    by 0x6ABB1E: mp_decode_frame (mpegaudiodec_template.c:1608)
==26166==    by 0x6ABFED: decode_frame (mpegaudiodec_template.c:1684)
==26166==    by 0x7A7DD5: avcodec_decode_audio4 (utils.c:2277)
==26166==    by 0x4480C7: audio_decode_example.constprop.6 (decoding_encoding.c:300)
==26166==    by 0x4477E1: main (decoding_encoding.c:642)
==26166== 
==26166== Invalid read of size 8
==26166==    at 0x68BCA0B: __GI_mempcpy (in /usr/lib/libc-2.18.so)
==26166==    by 0x68AC4C6: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.18.so)
==26166==    by 0x68A273C: fwrite (in /usr/lib/libc-2.18.so)
==26166==    by 0x4481D0: audio_decode_example.constprop.6 (decoding_encoding.c:310)
==26166==    by 0x4477E1: main (decoding_encoding.c:642)
==26166==  Address 0x7024e80 is 0 bytes after a block of size 2,304 alloc'd
==26166==    at 0x4C29D00: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26166==    by 0x4C29E17: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26166==    by 0x9AE6E9: av_malloc (mem.c:94)
==26166==    by 0x9A2A57: av_buffer_alloc (buffer.c:70)
==26166==    by 0x9A337B: av_buffer_pool_get (buffer.c:305)
==26166==    by 0x7A59A6: avcodec_default_get_buffer2 (utils.c:631)
==26166==    by 0x7A632E: ff_get_buffer (utils.c:973)
==26166==    by 0x6ABB1E: mp_decode_frame (mpegaudiodec_template.c:1608)
==26166==    by 0x6ABFED: decode_frame (mpegaudiodec_template.c:1684)
==26166==    by 0x7A7DD5: avcodec_decode_audio4 (utils.c:2277)
==26166==    by 0x4480C7: audio_decode_example.constprop.6 (decoding_encoding.c:300)
==26166==    by 0x4477E1: main (decoding_encoding.c:642)
==26166== 
==26166== Invalid read of size 8
==26166==    at 0x68BCA0B: __GI_mempcpy (in /usr/lib/libc-2.18.so)
==26166==    by 0x68ADE3D: _IO_default_xsputn (in /usr/lib/libc-2.18.so)
==26166==    by 0x68AC551: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.18.so)
==26166==    by 0x68A273C: fwrite (in /usr/lib/libc-2.18.so)
==26166==    by 0x4481D0: audio_decode_example.constprop.6 (decoding_encoding.c:310)
==26166==    by 0x4477E1: main (decoding_encoding.c:642)
==26166==  Address 0x7025b80 is not stack'd, malloc'd or (recently) free'd
==26166== 
==26166== 
==26166== HEAP SUMMARY:
==26166==     in use at exit: 40 bytes in 1 blocks
==26166==   total heap usage: 1,435 allocs, 1,434 frees, 171,686 bytes allocated
==26166== 
==26166== LEAK SUMMARY:
==26166==    definitely lost: 0 bytes in 0 blocks
==26166==    indirectly lost: 0 bytes in 0 blocks
==26166==      possibly lost: 0 bytes in 0 blocks
==26166==    still reachable: 40 bytes in 1 blocks
==26166==         suppressed: 0 bytes in 0 blocks
==26166== Rerun with --leak-check=full to see details of leaked memory
==26166== 
==26166== For counts of detected and suppressed errors, rerun with: -v
==26166== ERROR SUMMARY: 20636 errors from 3 contexts (suppressed: 2 from 2)
☭ 

[Carl Eugen Hoyos]

Is this a regression?

[Sylvain Demongeot]

The issue occurs in the latest source available, as well as 2.1.3 and 1.2.5 (the symptom seems to be different in 1.2.5, but the output is still bad)

[Sylvain Demongeot]

The fix is to replace "c->channels" with "1" on line 304 of decoding_encoding.c.

That's because decoded_frame->data[0] only contains data for 1 channel, not for both.
After the fix, the output (test.sw) is as expected, and valgrind doesn't mention any more memory addressing error.
Also, the output is now stable, as expected.

==27570== Memcheck, a memory error detector
==27570== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==27570== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==27570== Command: ./decoding_encoding mp2
==27570==
Encode audio file test.mp2
Decode audio file test.mp2 to test.sw
==27570==
==27570== HEAP SUMMARY:
==27570== in use at exit: 40 bytes in 1 blocks
==27570== total heap usage: 1,429 allocs, 1,428 frees, 165,246 bytes allocated
==27570==
==27570== LEAK SUMMARY:
==27570== definitely lost: 0 bytes in 0 blocks
==27570== indirectly lost: 0 bytes in 0 blocks
==27570== possibly lost: 0 bytes in 0 blocks
==27570== still reachable: 40 bytes in 1 blocks
==27570== suppressed: 0 bytes in 0 blocks
==27570== Rerun with --leak-check=full to see details of leaked memory
==27570==
==27570== For counts of detected and suppressed errors, rerun with: -v
==27570== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)

$ ./decoding_encoding mp2
Encode audio file test.mp2
Decode audio file test.mp2 to test.sw
$ md5sum test.*
ca203ada0aa42372edead60fd4dd928b test.mp2
359318db3ed158e6d6b2f55ebd3f15a3 test.sw
$ ./decoding_encoding mp2
Encode audio file test.mp2
Decode audio file test.mp2 to test.sw
$ md5sum test.*
ca203ada0aa42372edead60fd4dd928b test.mp2
359318db3ed158e6d6b2f55ebd3f15a3 test.sw

[Carl Eugen Hoyos]

Fixed by Michael in e6ea75c5e6b67012c5e8961d3f9171b0849b9cd2