[FFmpeg-devel] [PATCH 3/3] Add assert that the avcodec lockisheld when initializing static VLC tables.

[Don Moir]

> On 3 Dec 2012, at 01:09, "Don Moir" <donmoir at comcast.net> wrote:
>>> On Sun, Dec 02, 2012 at 05:35:33PM -0500, Don Moir wrote:
>>>> 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 ?
>>>
>>> As Nicolas pointed out, the why is completely unknown (otherwise we
>>> probably should indeed not be using an assert).
>>> We do not have a context around either most of the time I'm afraid.
>>>
>>>> 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.
>>>
>>> While that shouldn't be a big issue, what would you then do
>>> after your callback is being called?
>>> Without context and reason being available you won't really have
>>> anything more than you'd get in a signal handler...
>>>
>>>> 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.
>>>
>>> If by "handler" you mean signal handler, I'd think the obvious question
>>> would be: why not improve that signal handler so it can provide you
>>> all the information you need?
>>> Obviously not as easy as it sounds, but at least a backtrace should
>>> be possible and is definitely a very good start.
>>
>> Yep could do this but essentially the trace is going to be some high level ffmpeg call (we already know that) with probably quite 
>> a bit of depth of assembler.
>
> You should be getting file and line number as long as you have the debug info around (if you have it make a core dump, it's even 
> enough to have it on a different machine).
> The build system might not support this that well yet.
> There is also the option of having a new log level for av_log so you can have it print only these absolutely critical things (and 
> yes a abort callback is an option as well), but
> it would not solve the problem for other crashes, and it will only give you the lowest function which will still be useless in a 
> lot of cases.

The main thing for me with abort / av_assert0 was I suspected it might be overused with comand line app in mind. Like who cares if a 
command line app aborts. I think it might be that in some cases but I hope always because of some unusual circumstance.

An av_log level for only av_assert0 would be good. At least I would get a chance to log the condition without dealing with the other 
av_log junk. Also I think there are 4 straight abort calls in ffmpeg that should probably be changed to av_assert0.

If a crash or in case of abort happens to me then its ok. I test ffmpeg quite a bit before I hand it out, but generally I will get 
some kind of report from a beta tester that is not very clear and from someone that does not know much program wise. We create a log 
file for any kind of crash but right now it's kind of limited for ffmpeg. I know the high level function where the crash occurred 
plus registers plus unnamed assembler dump.

Largely though ffmpeg has behaved very well for me over the last several months. I had the one threading crash with vc1dec and had 
to fix a few for dshow so I could deliver camera support.

For a 'normal' crash not sure. For beta testing I could pass out debug dll's (I think).