Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#1863 closed defect (wontfix)

Remove all abort() calls from ffmpeg libraries

Reported by: DonMoir Owned by:
Priority: normal Component: undetermined
Version: unspecified Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

In certain failures, abort() is called in:

for libavcodec:

bitstream.c
mpegvideo.c
vp8.c

for libavformat:

sctp.c

This is ok if the app is a simple command line app that only handles one video.

It's not ok at all to bring down a GUI app this way. The abort calls need to removed and proper error handling needs to put into place so that the app will have a chance to handle it gracefully.

NOTE: This is not an enhancement request. It should be considered a bug because it's ridiculous and could cause all kinds of problems with people wondering what the hell happened.

One possible solution might be to have an application settable ff_abort call but this might be difficult to handle from an application perspective since it would not know what state the thing is in and what exactly to do about it. Probably best to just handle the error in the first place correctly and return the result code back to the app.

Change History (7)

comment:1 by Carl Eugen Hoyos, 11 years ago

Resolution: wontfix
Status: newclosed

If you are able to trigger one of the abort() calls (either from ffmpeg or from your application) please provide a backtrace etc. as explained on https://ffmpeg.org/bugreports.html

comment:2 by DonMoir, 11 years ago

Typical insanity and you should let a developer have a look before you just up and close the ticket. geeze

comment:3 by Carl Eugen Hoyos, 11 years ago

You did realize meanwhile that there are not ~4 calls to abort() in the FFmpeg source code, but >150 (or didn't you)?

comment:4 by DonMoir, 11 years ago

I only see 4 files effected that matter. They all should be removed regardless of how many and handled in a professional way.

You should also reopen ticket so someone with common sense can look at it.

What did you search for ? just the word abort ?

Last edited 11 years ago by DonMoir (previous) (diff)

comment:5 by reimar, 11 years ago

All av_assert* use abort().
Unfortunately all av_assert and aborts I can find are for cases where a serious error occurred, we are in an unknown state and generally it is simply unfixable and continuing would potentially make bugs easier to exploit.
Or to put it differently:

but this might be difficult to handle from an application perspective since it would not know what state the thing is in and what exactly to do about it.

It is used in cases where FFmpeg itself has exactly this problem.
A ff_abort seems kind of pointless, since you can already catch SIGABRT for a more or less similar effect.
So I do not see a way to avoid them that provides a reasonable cost/benefit ratio, however
1) All abort() that are called directly should probably be replaced with av_assert0
2) There might be _some_ that would be better to replace with a more robust solution (for example the one in ff_init_sparse_vlc that you probably encountered could probably be enhanced with a check that all static tables are initialized under a lock, thus making this kind of mistake immediately obvious).

comment:6 by DonMoir, 11 years ago

The problem is in a GUI app it's plain ridiculous. No one knows why it suddenly shut down and then you have people chasing ghosts. Also in my case, the GUI app is doing a lot more then just playing a video. It's also running laser shows etc. in public view. Why shut the whole program down when just one video or whatever failed ?

We now might have a laser stuck on hold shooting airplanes out of the sky. (just kidding)

When I first found this it was one of those spurious crashes from the unmentionable ticket #1876 and so was busy looking at that. I ignored it because I knew it was just a random crash due to the nature of threading but it made me aware of the abort calls.

In the real world we need to be more accountable for problems such as these.

There is enough justification in this ticket to have a closer look. If only one video failed, then so what. It's not justification to shut the whole program down.

comment:7 by DonMoir, 11 years ago

Also, I have never ever had a need to call abort at anytime. It's just today where people are more app to take the easy way out. In a command line app no one cares but the ffmpeg libraries are not just used for command line apps.

I used to take my programs before we shipped, turn off virtual memory, and allocate most of the rest of memory to see what would happen. If there was a problem this would find it. How many programs written today do you think would not crash under these conditions? Probably none of any complexity.

Last edited 11 years ago by DonMoir (previous) (diff)
Note: See TracTickets for help on using tickets.