[MPlayer-dev-eng] [PATCH] Abort on Fatal Messages

[Tobias Diedrich]

Alexander Strasser wrote:
> Hi,
> 
> Tobias Diedrich wrote:
> > Abort on fatal messages.
> > 
> > mp_msg.h says we abort on fatal messages, but we currently don't.
> > 
> > Index: mplayer-patchset1/mp_msg.c
> > ===================================================================
> > --- mplayer-patchset1.orig/mp_msg.c	2011-02-18 20:01:54.436871000 +0100
> > +++ mplayer-patchset1/mp_msg.c	2011-02-18 20:02:26.823321000 +0100
> > @@ -242,4 +242,7 @@
> >      if (mp_msg_color)
> >          fprintf(stream, "\033[0m");
> >      fflush(stream);
> > +
> > +    if (lev == MSGL_FATAL)
> > +        abort();
> >  }
> 
>   I like the idea, but I very much dislike quitting inside a function
> named mp_msg.
> 
>   IMHO the approach of having a new function, e.g. mp_msg_and_die
> or mp_die(msg...), would be better. This function would call mp_msg
> with level MSGL_FATAL and then terminate the process.
> 
>   I guess you had already places in mind where an exit would have
> been better than only printing the fatal error message. My proposal
> would need those places to be changed to use the new function. So
> a review of all mp_msg invocations with MSGL_FATAL would be in order
> to use the new function in all appropriate places.

Personally I'd say for failing malloc()s and similarily fatal errors
mp_die() would make sense.
Another option might be to have a malloc wrapper that sleeps and
retries the malloc a few times in case it was a transient error (it
might be if the OOM-killer kicked in the meantime and killed the
offending process).
I think currently when a memory allocation fails MPlayer might limp
on, for example I saw somewhere in the subtitle code malloc failure
gets passed down and I assume if a subtitle fails to load we will
continue regardless.  The cause of the load failure is not passed
down.

Maybe it would make more sense and be more acceptable to have a '-fail-early'
option that is not default.

I.e. when I watch a Video I may want MPlayer to try to continue
despite failures, but when I run mencoder to encode a video I
probably do want it to fail if it couldn't load a subtitle because
of a malloc problem.

-- 
Tobias						PGP: http://8ef7ddba.uguu.de