[FFmpeg-devel] [PATCH 3/3] Add assert that the avcodec lock isheld when initializing static VLC tables.
donmoir at comcast.net
Sun Dec 2 23:35:33 CET 2012
On Sun, Dec 02, 2012 at 04:37:20PM -0500, Don Moir wrote:
> On Sun, Dec 02, 2012 at 03:42:06PM -0500, Don Moir wrote:
> >----- Original Message ----- From: "Reimar Döffinger"
> ><Reimar.Doeffinger at gmx.de>
> >To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
> >Sent: Sunday, December 02, 2012 2:35 PM
> >Subject: Re: [FFmpeg-devel] [PATCH 3/3] Add assert that the avcodec lock isheld when initializing static VLC tables.
> >>Any comments on this patch series?
> >I don't think you should be calling abort. Works for command line app but really sucks for a GUI app.
> >>Can't the places where abort or assert is called exit gracefully ? I
> >>mean like whats the point if no one knows why the GUI app suddenly
> >>shut down that might be doing a lot more than just playing a video ?
> >And what do you suggest that we should be doing when we are in the
> >middle of a critical section and notice we have no lock?
> >Things are already broken and beyond saving at that point.
> >What does your GUI app want FFmpeg to do when there is an out-of-bounds
> >read? This assert failing is by all means just as critical an issue
> >as and out-of-bounds memory access.
> >If your GUI app is that critical there are loads of ways to handle it,
> >starting from a signal handler, over signal handler with longjmp, over
> >using a separate process, using a watchdog process or if it's really
> >critical using watchdog hardware.
> Yeah right. Without looking at numerous areas where assert and the
> few places where abort is called, just asking if abort is being
> used as a simple out since it doesn't matter for command line apps.
> But with that said, how the hell am I supposed to know where and
> what video caused the problem because we might have several videos
> playing at the same time. All we know right now is the app exited.
> I just never find the need to call abort and expect the code to
> behave properly. I know with the wide spread submissions this is
> more difficult to control for ffmpeg. At least a settable ff_abort
> that would pass the AVFormatContext back to the app would be better.
> I have the feeling you are not even trying to read and understand what I
> I'll try to make it simpler:
> What are you doing on a SIGSEGV?
> You should apply exactly, 100% the same rules for our aborts.
> Treat them as if a SIGSEGV happened.
> Anything else is a _bad idea_, you do _not_ just continue and pretend
> nothing happened when things went seriously wrong, that is like
> converting a SIGSEGV into a normal error, perfectly possible but
> a truly bad idea.
> If you cannot debug/properly inform your user about a SIGSEGV
> that is where your problem lies, not in that we also have a
> few cases where we use abort
Yes I do understand what you are saying and yes I can trap these out and I not suggesting the app just continues on like nothing
Looks to me there are hundreds of places av_assert0 is called and not just a few and av_assert0 calls abort. The thing is I would
like to have an easy indication where and why it failed. This is simple enough for you isn't it ?
If appropriate it could be ff_abort (AVFormatContext,reason); or whatever. ff_abort is set by app. With your aborts you have the
opportunity to give it back to the app in a cleaner way if you choose.
Let me guess, you are now going to point me to the av_log callback to check for some unknown context and some unknown message which
is generate by av_assert0.
We actually already have a handler in place but its not convienent for tracking the actual cause of the problem somewhere in ffmpeg
and by allowing a settable abort call this would help to isolate the problem.
More information about the ffmpeg-devel