Hi, On Wed, Jan 18, 2012 at 1:15 PM, Uoti Urpala <uoti.urp...@pp1.inet.fi> wrote: > On Wed, 2012-01-18 at 06:35 -0800, Ronald S. Bultje wrote: >> On Wed, Jan 18, 2012 at 5:52 AM, Janne Grunau <janne-li...@jannau.net> wrote: >> > On 2012-01-18 12:46:28 +0000, Måns Rullgård wrote: >> >> "Ronald S. Bultje" <rsbul...@gmail.com> writes: >> >> > On Wed, Jan 18, 2012 at 2:46 AM, Janne Grunau <janne-li...@jannau.net> >> >> > wrote: >> >> >> Users of libavcodec could break if they do not threadsafe callbacks. >> >> >> Avconv and avplay still use -threads auto if not specified. >> >> > >> >> > What callbacks? >> >> >> >> get_buffer() and friends I presume, since there are no others. > > get_buffer, draw_horiz_band, av_log at least.
get_buffer is threadsafe, so is release_buffer, unless you set avctx->thread_safe_callbacks. There is logic in pthread.c to proxy this to the calling thread. >> > get/release_buffer are safe since we call them only from the calling >> > thread when thread_safe_callbacks is not set. >> > >> > Uoti was worrying about draw_horiz_band. A custom av_log cb might be >> > problematic too although would suspect that it'll just cause message >> > corruption. >> >> I wonder how draw_horiz_band is used in this context? Uoti? > > Setting a thread-unsafe draw_horiz_band() was perfectly valid as long as > you did not explicitly set a thread count greater than 1. Suddenly > automatically enabling threads without warning breaks API/ABI in this > case (now the application may get callbacks from different threads, and > outside of the times it calls libavcodec). > > I tested this and found a case where it was actually possible to trigger > a crash in mplayer code due to this issue. There's a draw_horiz_band() > callback which must be disabled when threading is used; the code calling > libavcodec disabled it when it explicitly requested multiple threads. > But if libavcodec enables threading behind the application's back when > the application thinks threads are not used this triggers a crash. AFAIK > it was never an API requirement to explicitly change thread count from > the default 0 to 1 if you want single-threaded semantics. OK, so it can crash. What's the use case of draw_horiz_band if threading is enabled? Shouldn't the whole callback never be used, since we force codec delay? Is there a valid use case where the user-supplied draw_horiz_band should be called while threading is active and enabled? If not, we shouldn't revert the default back to 1 thread, but we should disable draw_horiz_band calls when threading is enabled. After all, threading gives a much bigger speed benefit than draw_horiz_band. Ronald _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
