| Message ID | 20170324124710.10576-1-nfxjfg@googlemail.com |
|---|---|
| State | Accepted |
| Commit | 9e703ae30f911d4df3f80647266e65d3b2dcf30d |
| Headers | show |
On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > async_mutex has is used in a very strange but intentional way: it is > locked by default, and unlocked only in regions that can be run > concurrently. > > If the user was calling API functions to the same context from different > threads (in a safe way), this could unintentionally unlock the mutex on > a different thread than the previous lock operation. It's not allowed by > the pthread API. > > Fix this by emulating a binary semaphore using a mutex and condition > variable. (Posix semaphores are not available on all platforms.) > --- > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > 1 file changed, 31 insertions(+), 10 deletions(-) this breaks flac make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav Input #0, flac, from 'out.flac': Metadata: ENCODER : Lavf57.67.100 Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 Stream mapping: Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) Press [q] to stop, [?] for help Output #0, null, to 'pipe:': Metadata: encoder : Lavf57.67.100 Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s Metadata: encoder : Lavc57.86.103 pcm_s16le Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 Aborted [...]
On Fri, 24 Mar 2017 18:53:41 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > > async_mutex has is used in a very strange but intentional way: it is > > locked by default, and unlocked only in regions that can be run > > concurrently. > > > > If the user was calling API functions to the same context from different > > threads (in a safe way), this could unintentionally unlock the mutex on > > a different thread than the previous lock operation. It's not allowed by > > the pthread API. > > > > Fix this by emulating a binary semaphore using a mutex and condition > > variable. (Posix semaphores are not available on all platforms.) > > --- > > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > > 1 file changed, 31 insertions(+), 10 deletions(-) > > this breaks flac > > make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav > > Input #0, flac, from 'out.flac': > Metadata: > ENCODER : Lavf57.67.100 > Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s > Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 > Stream mapping: > Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) > Press [q] to stop, [?] for help > Output #0, null, to 'pipe:': > Metadata: > encoder : Lavf57.67.100 > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > Metadata: > encoder : Lavc57.86.103 pcm_s16le > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > Aborted > > [...] I get: testwav_cut.wav: Invalid data found when processing input Can't reproduce.
On Fri, 24 Mar 2017 18:53:41 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > > async_mutex has is used in a very strange but intentional way: it is > > locked by default, and unlocked only in regions that can be run > > concurrently. > > > > If the user was calling API functions to the same context from different > > threads (in a safe way), this could unintentionally unlock the mutex on > > a different thread than the previous lock operation. It's not allowed by > > the pthread API. > > > > Fix this by emulating a binary semaphore using a mutex and condition > > variable. (Posix semaphores are not available on all platforms.) > > --- > > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > > 1 file changed, 31 insertions(+), 10 deletions(-) > > this breaks flac > > make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav > > Input #0, flac, from 'out.flac': > Metadata: > ENCODER : Lavf57.67.100 > Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s > Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 > Stream mapping: > Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) > Press [q] to stop, [?] for help > Output #0, null, to 'pipe:': > Metadata: > encoder : Lavf57.67.100 > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > Metadata: > encoder : Lavc57.86.103 pcm_s16le > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > Aborted > > [...] Actually that was a HTML link. Tried it again with the proper file, still cannot reproduce, both commands run successfully. I tried 3 times. Please post only things that can be reproduced.
On Sat, Mar 25, 2017 at 12:03:23PM +0100, wm4 wrote: > On Fri, 24 Mar 2017 18:53:41 +0100 > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > > > async_mutex has is used in a very strange but intentional way: it is > > > locked by default, and unlocked only in regions that can be run > > > concurrently. > > > > > > If the user was calling API functions to the same context from different > > > threads (in a safe way), this could unintentionally unlock the mutex on > > > a different thread than the previous lock operation. It's not allowed by > > > the pthread API. > > > > > > Fix this by emulating a binary semaphore using a mutex and condition > > > variable. (Posix semaphores are not available on all platforms.) > > > --- > > > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > > > 1 file changed, 31 insertions(+), 10 deletions(-) > > > > this breaks flac > > > > make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav > > > > Input #0, flac, from 'out.flac': > > Metadata: > > ENCODER : Lavf57.67.100 > > Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s > > Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 > > Stream mapping: > > Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) > > Press [q] to stop, [?] for help > > Output #0, null, to 'pipe:': > > Metadata: > > encoder : Lavf57.67.100 > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > Metadata: > > encoder : Lavc57.86.103 pcm_s16le > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > Aborted > > > > [...] > > Actually that was a HTML link. Tried it again with the proper file, > still cannot reproduce, both commands run successfully. I tried 3 times. > > Please post only things that can be reproduced. It reproduces 100% here: git log -2 --oneline 4bf2209f29 pthread_frame: do not attempt to unlock a mutex on the wrong thread 09ce5519f3 fate/checkasm: fix use of uninitialized memory on hevc_add_res tests make distclean ; ./configure --enable-pthreads --assert-level=2 make -j12 ffmpeg && ./ffmpeg -y -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - 77700c5eaf9daf4d5d5092af79774882 /home/michael/tickets/4421/testwav_cut.wav 1fb29c0b2086a7cabf389de647316249 out.flac ... Press [q] to stop, [?] for help Output #0, null, to 'pipe:': Metadata: encoder : Lavf57.67.100 Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s Metadata: encoder : Lavc57.86.103 pcm_s16le Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 Aborted 2nd ffmpeg under valgrind Output #0, null, to 'pipe:': Metadata: encoder : Lavf57.67.100 Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s Metadata: encoder : Lavc57.86.103 pcm_s16le ==20594== Conditional jump or move depends on uninitialised value(s) ==20594== at 0xADBE5D: avcodec_decode_audio4 (utils.c:2390) ==20594== by 0xADC797: do_decode (utils.c:2827) ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) ==20594== by 0x483B17: main (ffmpeg.c:4740) ==20594== ==20594== Conditional jump or move depends on uninitialised value(s) ==20594== at 0xADC0F5: avcodec_decode_audio4 (utils.c:2497) ==20594== by 0xADC797: do_decode (utils.c:2827) ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) ==20594== by 0x483B17: main (ffmpeg.c:4740) ==20594== ==20594== Conditional jump or move depends on uninitialised value(s) ==20594== at 0xADC10D: avcodec_decode_audio4 (utils.c:2507) ==20594== by 0xADC797: do_decode (utils.c:2827) ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) ==20594== by 0x483B17: main (ffmpeg.c:4740) ==20594== Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507
On Sat, 25 Mar 2017 13:01:58 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Mar 25, 2017 at 12:03:23PM +0100, wm4 wrote: > > On Fri, 24 Mar 2017 18:53:41 +0100 > > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > > On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > > > > async_mutex has is used in a very strange but intentional way: it is > > > > locked by default, and unlocked only in regions that can be run > > > > concurrently. > > > > > > > > If the user was calling API functions to the same context from different > > > > threads (in a safe way), this could unintentionally unlock the mutex on > > > > a different thread than the previous lock operation. It's not allowed by > > > > the pthread API. > > > > > > > > Fix this by emulating a binary semaphore using a mutex and condition > > > > variable. (Posix semaphores are not available on all platforms.) > > > > --- > > > > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > > > > 1 file changed, 31 insertions(+), 10 deletions(-) > > > > > > this breaks flac > > > > > > make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > > https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav > > > > > > Input #0, flac, from 'out.flac': > > > Metadata: > > > ENCODER : Lavf57.67.100 > > > Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s > > > Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 > > > Stream mapping: > > > Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) > > > Press [q] to stop, [?] for help > > > Output #0, null, to 'pipe:': > > > Metadata: > > > encoder : Lavf57.67.100 > > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > > Metadata: > > > encoder : Lavc57.86.103 pcm_s16le > > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > > Aborted > > > > > > [...] > > > > Actually that was a HTML link. Tried it again with the proper file, > > still cannot reproduce, both commands run successfully. I tried 3 times. > > > > > Please post only things that can be reproduced. > > It reproduces 100% here: > > git log -2 --oneline > 4bf2209f29 pthread_frame: do not attempt to unlock a mutex on the wrong thread > 09ce5519f3 fate/checkasm: fix use of uninitialized memory on hevc_add_res tests > > make distclean ; ./configure --enable-pthreads --assert-level=2 > make -j12 ffmpeg && ./ffmpeg -y -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > 77700c5eaf9daf4d5d5092af79774882 /home/michael/tickets/4421/testwav_cut.wav > 1fb29c0b2086a7cabf389de647316249 out.flac > > ... > Press [q] to stop, [?] for help > Output #0, null, to 'pipe:': > Metadata: > encoder : Lavf57.67.100 > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > Metadata: > encoder : Lavc57.86.103 pcm_s16le > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > Aborted > > 2nd ffmpeg under valgrind > > Output #0, null, to 'pipe:': > Metadata: > encoder : Lavf57.67.100 > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > Metadata: > encoder : Lavc57.86.103 pcm_s16le > ==20594== Conditional jump or move depends on uninitialised value(s) > ==20594== at 0xADBE5D: avcodec_decode_audio4 (utils.c:2390) > ==20594== by 0xADC797: do_decode (utils.c:2827) > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > ==20594== by 0x483B17: main (ffmpeg.c:4740) > ==20594== > ==20594== Conditional jump or move depends on uninitialised value(s) > ==20594== at 0xADC0F5: avcodec_decode_audio4 (utils.c:2497) > ==20594== by 0xADC797: do_decode (utils.c:2827) > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > ==20594== by 0x483B17: main (ffmpeg.c:4740) > ==20594== > ==20594== Conditional jump or move depends on uninitialised value(s) > ==20594== at 0xADC10D: avcodec_decode_audio4 (utils.c:2507) > ==20594== by 0xADC797: do_decode (utils.c:2827) > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > ==20594== by 0x483B17: main (ffmpeg.c:4740) > ==20594== > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > It's because ff_thread_decode_frame() does not initialize ret in all code paths, now it works, fixed locally.
On Sat, Mar 25, 2017 at 01:13:13PM +0100, wm4 wrote: > On Sat, 25 Mar 2017 13:01:58 +0100 > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > On Sat, Mar 25, 2017 at 12:03:23PM +0100, wm4 wrote: > > > On Fri, 24 Mar 2017 18:53:41 +0100 > > > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > > > > On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > > > > > async_mutex has is used in a very strange but intentional way: it is > > > > > locked by default, and unlocked only in regions that can be run > > > > > concurrently. > > > > > > > > > > If the user was calling API functions to the same context from different > > > > > threads (in a safe way), this could unintentionally unlock the mutex on > > > > > a different thread than the previous lock operation. It's not allowed by > > > > > the pthread API. > > > > > > > > > > Fix this by emulating a binary semaphore using a mutex and condition > > > > > variable. (Posix semaphores are not available on all platforms.) > > > > > --- > > > > > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > > > > > 1 file changed, 31 insertions(+), 10 deletions(-) > > > > > > > > this breaks flac > > > > > > > > make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > > > https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav > > > > > > > > Input #0, flac, from 'out.flac': > > > > Metadata: > > > > ENCODER : Lavf57.67.100 > > > > Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s > > > > Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 > > > > Stream mapping: > > > > Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) > > > > Press [q] to stop, [?] for help > > > > Output #0, null, to 'pipe:': > > > > Metadata: > > > > encoder : Lavf57.67.100 > > > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > > > Metadata: > > > > encoder : Lavc57.86.103 pcm_s16le > > > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > > > Aborted > > > > > > > > [...] > > > > > > Actually that was a HTML link. Tried it again with the proper file, > > > still cannot reproduce, both commands run successfully. I tried 3 times. > > > > > > > > Please post only things that can be reproduced. > > > > It reproduces 100% here: > > > > git log -2 --oneline > > 4bf2209f29 pthread_frame: do not attempt to unlock a mutex on the wrong thread > > 09ce5519f3 fate/checkasm: fix use of uninitialized memory on hevc_add_res tests > > > > make distclean ; ./configure --enable-pthreads --assert-level=2 > > make -j12 ffmpeg && ./ffmpeg -y -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > > > 77700c5eaf9daf4d5d5092af79774882 /home/michael/tickets/4421/testwav_cut.wav > > 1fb29c0b2086a7cabf389de647316249 out.flac > > > > ... > > Press [q] to stop, [?] for help > > Output #0, null, to 'pipe:': > > Metadata: > > encoder : Lavf57.67.100 > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > Metadata: > > encoder : Lavc57.86.103 pcm_s16le > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > Aborted > > > > 2nd ffmpeg under valgrind > > > > Output #0, null, to 'pipe:': > > Metadata: > > encoder : Lavf57.67.100 > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > Metadata: > > encoder : Lavc57.86.103 pcm_s16le > > ==20594== Conditional jump or move depends on uninitialised value(s) > > ==20594== at 0xADBE5D: avcodec_decode_audio4 (utils.c:2390) > > ==20594== by 0xADC797: do_decode (utils.c:2827) > > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > > ==20594== by 0x483B17: main (ffmpeg.c:4740) > > ==20594== > > ==20594== Conditional jump or move depends on uninitialised value(s) > > ==20594== at 0xADC0F5: avcodec_decode_audio4 (utils.c:2497) > > ==20594== by 0xADC797: do_decode (utils.c:2827) > > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > > ==20594== by 0x483B17: main (ffmpeg.c:4740) > > ==20594== > > ==20594== Conditional jump or move depends on uninitialised value(s) > > ==20594== at 0xADC10D: avcodec_decode_audio4 (utils.c:2507) > > ==20594== by 0xADC797: do_decode (utils.c:2827) > > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > > ==20594== by 0x483B17: main (ffmpeg.c:4740) > > ==20594== > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > > > It's because ff_thread_decode_frame() does not initialize ret in all > code paths, now it works, fixed locally. yes, do you still wonder why some people are a bit lack-luster about taking code from libav ? Thanks for fixing it btw [...]
On Sat, 25 Mar 2017 13:25:32 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Mar 25, 2017 at 01:13:13PM +0100, wm4 wrote: > > On Sat, 25 Mar 2017 13:01:58 +0100 > > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > > On Sat, Mar 25, 2017 at 12:03:23PM +0100, wm4 wrote: > > > > On Fri, 24 Mar 2017 18:53:41 +0100 > > > > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > > > > > > On Fri, Mar 24, 2017 at 01:47:10PM +0100, wm4 wrote: > > > > > > async_mutex has is used in a very strange but intentional way: it is > > > > > > locked by default, and unlocked only in regions that can be run > > > > > > concurrently. > > > > > > > > > > > > If the user was calling API functions to the same context from different > > > > > > threads (in a safe way), this could unintentionally unlock the mutex on > > > > > > a different thread than the previous lock operation. It's not allowed by > > > > > > the pthread API. > > > > > > > > > > > > Fix this by emulating a binary semaphore using a mutex and condition > > > > > > variable. (Posix semaphores are not available on all platforms.) > > > > > > --- > > > > > > libavcodec/pthread_frame.c | 41 +++++++++++++++++++++++++++++++---------- > > > > > > 1 file changed, 31 insertions(+), 10 deletions(-) > > > > > > > > > > this breaks flac > > > > > > > > > > make -j12 ffmpeg && ./ffmpeg -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > > > > https://trac.ffmpeg.org/attachment/ticket/4421/testwav_cut.wav > > > > > > > > > > Input #0, flac, from 'out.flac': > > > > > Metadata: > > > > > ENCODER : Lavf57.67.100 > > > > > Duration: 00:00:10.00, start: 0.000000, bitrate: 912 kb/s > > > > > Stream #0:0: Audio: flac, 44100 Hz, stereo, s16 > > > > > Stream mapping: > > > > > Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) > > > > > Press [q] to stop, [?] for help > > > > > Output #0, null, to 'pipe:': > > > > > Metadata: > > > > > encoder : Lavf57.67.100 > > > > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > > > > Metadata: > > > > > encoder : Lavc57.86.103 pcm_s16le > > > > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > > > > Aborted > > > > > > > > > > [...] > > > > > > > > Actually that was a HTML link. Tried it again with the proper file, > > > > still cannot reproduce, both commands run successfully. I tried 3 times. > > > > > > > > > > > Please post only things that can be reproduced. > > > > > > It reproduces 100% here: > > > > > > git log -2 --oneline > > > 4bf2209f29 pthread_frame: do not attempt to unlock a mutex on the wrong thread > > > 09ce5519f3 fate/checkasm: fix use of uninitialized memory on hevc_add_res tests > > > > > > make distclean ; ./configure --enable-pthreads --assert-level=2 > > > make -j12 ffmpeg && ./ffmpeg -y -i ~/tickets/4421/testwav_cut.wav out.flac ; ./ffmpeg -i out.flac -f null - > > > > > > 77700c5eaf9daf4d5d5092af79774882 /home/michael/tickets/4421/testwav_cut.wav > > > 1fb29c0b2086a7cabf389de647316249 out.flac > > > > > > ... > > > Press [q] to stop, [?] for help > > > Output #0, null, to 'pipe:': > > > Metadata: > > > encoder : Lavf57.67.100 > > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > > Metadata: > > > encoder : Lavc57.86.103 pcm_s16le > > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > > Aborted > > > > > > 2nd ffmpeg under valgrind > > > > > > Output #0, null, to 'pipe:': > > > Metadata: > > > encoder : Lavf57.67.100 > > > Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s > > > Metadata: > > > encoder : Lavc57.86.103 pcm_s16le > > > ==20594== Conditional jump or move depends on uninitialised value(s) > > > ==20594== at 0xADBE5D: avcodec_decode_audio4 (utils.c:2390) > > > ==20594== by 0xADC797: do_decode (utils.c:2827) > > > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > > > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > > > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > > > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > > > ==20594== by 0x483B17: main (ffmpeg.c:4740) > > > ==20594== > > > ==20594== Conditional jump or move depends on uninitialised value(s) > > > ==20594== at 0xADC0F5: avcodec_decode_audio4 (utils.c:2497) > > > ==20594== by 0xADC797: do_decode (utils.c:2827) > > > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > > > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > > > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > > > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > > > ==20594== by 0x483B17: main (ffmpeg.c:4740) > > > ==20594== > > > ==20594== Conditional jump or move depends on uninitialised value(s) > > > ==20594== at 0xADC10D: avcodec_decode_audio4 (utils.c:2507) > > > ==20594== by 0xADC797: do_decode (utils.c:2827) > > > ==20594== by 0xADD7EF: avcodec_receive_frame (utils.c:2949) > > > ==20594== by 0x49F99A: process_input_packet (ffmpeg.c:2256) > > > ==20594== by 0x4A11F8: process_input (ffmpeg.c:4171) > > > ==20594== by 0x4A362E: transcode (ffmpeg.c:4481) > > > ==20594== by 0x483B17: main (ffmpeg.c:4740) > > > ==20594== > > > Assertion ret <= avpkt->size failed at libavcodec/utils.c:2507 > > > > > > > It's because ff_thread_decode_frame() does not initialize ret in all > > code paths, now it works, fixed locally. > > yes, > do you still wonder why some people are a bit lack-luster about taking > code from libav ? > > Thanks for fixing it btw > > [...] Pushed, with the fixup patch (d7896e9b4228e5b7ffc). Will probably send a new patch that cleans up the mess with the ret/err variables.
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c index 6768402ed8..6620a8d324 100644 --- a/libavcodec/pthread_frame.c +++ b/libavcodec/pthread_frame.c @@ -123,6 +123,8 @@ typedef struct FrameThreadContext { */ pthread_mutex_t hwaccel_mutex; pthread_mutex_t async_mutex; + pthread_cond_t async_cond; + int async_lock; int next_decoding; ///< The next context to submit a packet to. int next_finished; ///< The next context to return output from. @@ -136,6 +138,24 @@ typedef struct FrameThreadContext { #define THREAD_SAFE_CALLBACKS(avctx) \ ((avctx)->thread_safe_callbacks || (avctx)->get_buffer2 == avcodec_default_get_buffer2) +static void async_lock(FrameThreadContext *fctx) +{ + pthread_mutex_lock(&fctx->async_mutex); + while (fctx->async_lock) + pthread_cond_wait(&fctx->async_cond, &fctx->async_mutex); + fctx->async_lock = 1; + pthread_mutex_unlock(&fctx->async_mutex); +} + +static void async_unlock(FrameThreadContext *fctx) +{ + pthread_mutex_lock(&fctx->async_mutex); + av_assert0(fctx->async_lock); + fctx->async_lock = 0; + pthread_cond_broadcast(&fctx->async_cond); + pthread_mutex_unlock(&fctx->async_mutex); +} + /** * Codec worker thread. * @@ -195,7 +215,8 @@ static attribute_align_arg void *frame_worker_thread(void *arg) if (p->async_serializing) { p->async_serializing = 0; - pthread_mutex_unlock(&p->parent->async_mutex); + + async_unlock(p->parent); } pthread_mutex_lock(&p->progress_mutex); @@ -451,7 +472,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx, /* release the async lock, permitting blocked hwaccel threads to * go forward while we are in this function */ - pthread_mutex_unlock(&fctx->async_mutex); + async_unlock(fctx); /* * Submit a packet to the next decoding thread. @@ -532,7 +553,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx, /* return the size of the consumed packet if no error occurred */ ret = (p->result >= 0) ? avpkt->size : p->result; finish: - pthread_mutex_lock(&fctx->async_mutex); + async_lock(fctx); if (err < 0) return err; return ret; @@ -594,7 +615,8 @@ void ff_thread_finish_setup(AVCodecContext *avctx) { if (avctx->hwaccel && !(avctx->hwaccel->caps_internal & HWACCEL_CAP_ASYNC_SAFE)) { p->async_serializing = 1; - pthread_mutex_lock(&p->parent->async_mutex); + + async_lock(p->parent); } pthread_mutex_lock(&p->progress_mutex); @@ -613,7 +635,7 @@ static void park_frame_worker_threads(FrameThreadContext *fctx, int thread_count { int i; - pthread_mutex_unlock(&fctx->async_mutex); + async_unlock(fctx); for (i = 0; i < thread_count; i++) { PerThreadContext *p = &fctx->threads[i]; @@ -627,7 +649,7 @@ static void park_frame_worker_threads(FrameThreadContext *fctx, int thread_count p->got_frame = 0; } - pthread_mutex_lock(&fctx->async_mutex); + async_lock(fctx); } void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) @@ -691,9 +713,8 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count) av_freep(&fctx->threads); pthread_mutex_destroy(&fctx->buffer_mutex); pthread_mutex_destroy(&fctx->hwaccel_mutex); - - pthread_mutex_unlock(&fctx->async_mutex); pthread_mutex_destroy(&fctx->async_mutex); + pthread_cond_destroy(&fctx->async_cond); av_freep(&avctx->internal->thread_ctx); @@ -742,10 +763,10 @@ int ff_frame_thread_init(AVCodecContext *avctx) pthread_mutex_init(&fctx->buffer_mutex, NULL); pthread_mutex_init(&fctx->hwaccel_mutex, NULL); - pthread_mutex_init(&fctx->async_mutex, NULL); - pthread_mutex_lock(&fctx->async_mutex); + pthread_cond_init(&fctx->async_cond, NULL); + fctx->async_lock = 1; fctx->delaying = 1; for (i = 0; i < thread_count; i++) {