On Fri, Jan 13, 2017 at 07:13:10PM +0100, wm4 wrote:
> On Fri, 13 Jan 2017 12:19:14 -0500
> "Ronald S. Bultje" <rsbul...@gmail.com> wrote:
>
> > Hi,
> >
> > On Fri, Jan 13, 2017 at 9:39 AM, wm4 <nfx...@googlemail.com> wrote:
> >
> > > On Fri, 13 Jan 2017 09:07:48 -0500
> > > "Ronald S. Bultje" <rsbul...@gmail.com> wrote:
> > >
> > > > Hi,
> > > >
> > > > On Jan 13, 2017 6:40 AM, "Clément Bœsch" <u...@pkh.me> wrote:
> > > >
> > > > From: Clément Bœsch <cboe...@gopro.com>
> > > >
> > > > ---
> > > > libavcodec/pthread_frame.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > > > index 7ef5e9f6be..cb6d76284e 100644
> > > > --- a/libavcodec/pthread_frame.c
> > > > +++ b/libavcodec/pthread_frame.c
> > > > @@ -509,11 +509,11 @@ void ff_thread_finish_setup(AVCodecContext
> > > *avctx) {
> > > >
> > > > if (!(avctx->active_thread_type&FF_THREAD_FRAME)) return;
> > > >
> > > > + pthread_mutex_lock(&p->progress_mutex);
> > > > if(p->state == STATE_SETUP_FINISHED){
> > > > av_log(avctx, AV_LOG_WARNING, "Multiple
> > > > ff_thread_finish_setup()
> > > > calls\n");
> > > > }
> > > >
> > > > - pthread_mutex_lock(&p->progress_mutex);
> > > >
> > > >
> > > > This has a problem in that we're potentially calling an I/O function
> > > > that
> > > > potentially directly enters application space with a lock held.
> > >
> > > I think the user should be taking care of this (or just don't spam
> > > av_log in a performance critical function). The log call here seems
> > > more like an internal warning though, and I've never seen it happening.
> > >
> >
> > But it's easy to resolve right?
> >
> > lock();
> > int state = p->state;
> > ..
> > unlock();
> > if (state == ..)
> > av_log(..);
>
> I still don't see what the problem is. Why make it more complex than
> necessary?2017-01-13 21:02:31 @ubitux wm4: BBB: so patch is ok as is, or i need to split the av_log out? 2017-01-13 21:03:47 @BBB no it’s fine 2017-01-13 21:03:50 @BBB just keep it 2017-01-13 21:04:02 @BBB if assumption is that doing what I said is a bug, then no need to adjust to it 2017-01-13 21:08:20 +wm4 ok with me patch applied thanks -- Clément B.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
