Opened 4 years ago

Closed 4 years ago

Last modified 4 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 Changed 4 years ago by cehoyos

  • Resolution set to wontfix
  • Status changed from new to closed

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 Changed 4 years ago by DonMoir

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

comment:3 Changed 4 years ago by cehoyos

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 Changed 4 years ago by DonMoir

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 4 years ago by DonMoir (previous) (diff)

comment:5 Changed 4 years ago by reimar

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 Changed 4 years ago by DonMoir

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 Changed 4 years ago by DonMoir

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 know 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.

Version 3, edited 4 years ago by DonMoir (previous) (next) (diff)
Note: See TracTickets for help on using tickets.