Hi, On Sat, Jul 8, 2017 at 5:28 PM, Wan-Teh Chang <wtc-at-google....@ffmpeg.org> wrote:
> Hi Ronald, > > On Sat, Jul 8, 2017 at 6:19 AM, Ronald S. Bultje <rsbul...@gmail.com> > wrote: > > Hi, > > > > On Fri, Jul 7, 2017 at 5:02 PM, Wan-Teh Chang < > wtc-at-google....@ffmpeg.org> > > wrote: > > > >> @@ -763,6 +764,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) > >> > >> fctx->async_lock = 1; > >> fctx->delaying = 1; > >> + fctx->debug_threads = (avctx->debug & FF_DEBUG_THREADS) != 0; > > > > Shouldn't this be done in update_context_from_user()? > > This patch differs from the approach you outlined at the end of > http://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213259.html > as follows: > > * The debug_threads field is added to FrameThreadContext and applies to > all the decoding threads owned by the FrameThreadContext. > * The debug_threads field is set when avcodec_open2() is called, and > never changes thereafter. > > In this design, we just need to set fctx->debug_threads in > ff_frame_thread_init() (which is called by avcodec_open2()). I can see the design from the patch. What's missing is a justification for the downside of the design, which is that updates to this variable by the user are no longer propagated to the worker threads. The syncing is extremely trivial (simply by moving the assignment from init to update_from_user) and has no threading implications (since it's a boolean, so you can make it atomic) so this really shouldn't be a big deal to amend. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
