Re: [gst-devel] error handling in GStreamer

[Bastien Nocera <hadess hadess net>]

On Thu, 2003-09-25 at 12:02, Thomas Vander Stichele wrote:
> > > a) pop up an error dialog with an advanced button, where "advanced"
> > > would show (d) and (e), as well as source code file, line number,
> > > function
> > 
> > I don't like that one, I must say. If it's a programming error, it
> > should go as a g_warning, not be user-visible.
> 
> I've gotten a lot of "if it's a programming error".  The simple fact is
> that I've never regarded errors as either "programming" or "user"
> errors.  Especially for multimedia apps/libs, this is a wrong
> distinction to make.  Consider:
> a) a user tries to play a video file
> b) GStreamer tries to detect type and play it
> c) Suppose gst gets the type wrong and can't play the file.  The user
> will get a popup saying "This file cannot be played/is broken/...".  The
> user tries with some other app, and it plays.  What went wrong ?
> d) Suppose gst gets the type right, but there is some error in the file
> and it can't decode it properly.  User gets the same popup.  It plays
> mostly fine in another app.  What went wrong ?

You just need more fine-grained errors then.

> In c) it could be considered a programming error, but the code has no
> way of verifying if it made a programming error.  IMO, g_warnings are
> for cases where the code has figured out that something shouldn't have
> happened and really is a programming mistake.  In c), the user will join
> #gstreamer, say that his file can't be played, and ask what went wrong. 
> Without having a good way of reproducing the bug, this won't be very
> helpful to us.  If we then ask the user to click "Advanced" and paste
> the message there, that will be helpful to us.
> For example, it will say "avidemux: AVI header invalid" while the file
> is an .ogm.  Hence, we know where to look for what went wrong.

That should be appearing in the debug info, on the command-line. And
we'd just add another error message: "File is not of expected type".

> In d), it isn't even really a gst programming error, but maybe an
> underlying library.  The advanced button will say what is wrong (ie
> mpeg2dec: invalid frame) and we can look at whether or not something is
> wrong with the mpeg2dec version.

You probably wouldn't error out on this kind of mpeg2dec error, but if
you did, you wouldn't

> Both are very valid use cases for where it makes sense to let the user
> provide this info to the developers; which is why we want to see this
> implemented.

And you'll live with crappy debug in all your applications forever?
I don't want debug in my applications unless I want to debug it.

You would tell the user: "Turn on debugging in the application, try
playing it again, and send us the output". Even xine is very quiet these
days when it comes to outputting debug.

> I know that for xine/mplayer this need isn't there, since most of the
> interesting code/codecs is in the tarball and thus under direct
> developer control.
> 
> However with GStreamer we deliberately wanted to move away from this
> model and I think that the error signaling system should help us realize
> this model.
> 
> Please feel free to argue these specific points as I feel they are
> important.
> 
> > > b) have the application trap errors of a specific kind so that it can
> > > decide to try plan b instead of popping up a dialog
> > 
> > That sounds good, as long as the filtering is done on the application
> > side (ie. all the signals are still sent to the application, whatever
> > happens).
> 
> Yeah, it is - app connects to signal, app chooses based on enum what to
> do.

Nod.

> > > c) have the application use available translated strings (from core and
> > > plugins) if it chooses to do so
> > 
> > > d) have the application be able to provide it's own error messages if it
> > > thinks it needs to, based on the enum
> > 
> > (aren't c) and d) basically the same?)
> no; in c) the library provides error messages, which handles the common
> cases.  in d), the application can override these if it wishes to do
> so.  For example, if it knows what it's doing for some specific purpose
> that might not be what the plugin author intended, and it can choose to
> provide an error message more appropriate to the situation.

That would be d) then.

> > I'd rather see that in the application itself. For example, an error in
> > playback will be different if we have a video playback application, or a
> > webcam application (taking totem and vanity as examples).
> 
> Yep; which is why for the less common case (the webcam) the error
> message could be different.
> 
> What I absolutely want to avoid is for each webcam application to
> provide different error messages for the same error.  Especially in the
> cases where it makes no sense to do so.

When we get to the point where we have 5 webcam applications, I'll start
worrying ;)

> > > some things to consider here:
> > > a) we can provide functions that map the very specific finegrained enums
> > > to coarse enum domains (ie, all device errors could map to one class).
> > 
> > Things like device errors should be finegrained, is it a permission
> > error, the device isn't there, the media is damaged, etc.
> 
> Possibly; I've had people tell me that "permission errors" are not
> acceptable to be presented.  I would beg to differ, but I'm not going to
> make this decision alone :)
> 
> > > b) we need to decide on whether or not we want plug-ins to be able to
> > > provide custom translated messages.  If there is a direct one-to-one
> > > mapping between enums and error strings, this is not really necessary. 
> > > If the enum would be coarse-grained (ie, only apply to a domain, like
> > > GST_ERROR_DEVICE), then the element needs to be able to give more
> > > specific data.
> > 
> > I'd rather not see that. See gnome-vfs, we would just add more error
> > elements as we find things that need error messages.
> 
> "that" = coarse-grained enums ? OK.  So you want direct
> one-to-one-mapping and finegrained enums, just to make sure I understood
> you ?

I don't think there's a need to have multiple error domains, you would
end up have a lot of overlap between the different ones. So having a
catch-all generic error, and a lot of fine-grained error messages.

> > > c) the API for elements could be something like
> > >    gst_element_gerror (GstErrorType GST_ERROR_(type), gchar *message,
> > > gchart *debug);
> > >    since we want both strings to be printf-like if needed, this would
> > > also mean that
> > >    - gst_element_gerror takes ownership of the strings and frees them
> > >    - a typical call would look like
> > >      gst_element_gerror (GST_ERROR_DEVICE,
> > >                          g_strdup_printf (_("%s not accessible",
> > >                                           device),
> > >                          g_strdup_printf ("no write permissions on %s",
> > >                                           device));
> > 
> > Either that code will leak, or it will crash when I pass non-malloc'ed
> > strings ;)
> 
> The point is that _gerror will unref the passed strings, it will likely
> also be wrapped in a macro that does the strdup for you, and you should
> never pass non-malloc'd strings :) The reason we want to do this is
> because this is the only way we could come up with a way of having two
> printf-like strings.
> The actual call will probably end up being GST_ELEMENT_GERROR
> (GST_ERROR_DEVICE, ("%s not accessible", device), ("no write permissions
> on %s", device));

I was just being picky ;)

> > I really don't like the idea of debug info being presented to the user.
> Well, a) it's hidden (a button press away) and b) I think the use cases
> validate the need.  What do you think ?

I prefer hidden -> the user needs to explicitely ask for the debug info
to be shown.

---
Bastien Nocera <hadess hadess net> 
The thunder was ominous sounding, much like the sound of a thin sheet of
metal being shaken backstage during the storm scene in a play.