[gst-devel] error explanation

[Thomas Vander Stichele]

El lun, 19-01-2004 a las 23:35, Benjamin Otte escribió:
> Ok, I have some issues with this. I'll list them in no order at all.

Thanks for reviewing.

> - GST_STREAM_ERROR_DECODE, GST_STREAM_ERROR_ENCODE,
> GST_STREAM_ERROR_DEMUX and GST_STREAM_ERROR_MUX imply a function of the
> element. All of these are basically just "format could not be read"
> depending on the type of element. I think this should be the same error.

I was pretty much 50/50 on this one.  I decided by thinking from the
user's perspective, and thought that the user would rather see "could
not encode format" or "could not decode format" than "format problem",
since he understands the first two better.
Of course, most users don't know what mux and demux are (and I thought
about just making DEMUX DECODE and MUX ENCODE, since for the user
they're probably the same anyway.  This is my preference atm)

But, what do others think ?

> - gst_element_error is the only function inside libc, glib and GStreamer
> that takes nested brackets. That's a style issue for me. The
> *_CALL_PARENT macroes do that, too and it annoys the hell out of me. This
> is C and not Lisp.

Yeah, it's awkward.  OTOH, the goal was to make it one function-like
call, and we didn't find any other way to do this.  If you have a
problem with the nestedness, we can do the same thing as the i18n
macros, and just use something like E_() and D_() to achieve the same
thing if this pleases you better.  Let me know.

> - I'm not sure if abbreviating error domains and codes is a good idea.
> It's especially bad when grepping.

I like the short statements coming from it.  grepping doesn't make it
harder, since they always appear together.  Ie, you can grep for
"RESOURCE, READ" or "RESOURCE.*READ" if our style isn't fixed yet, and
get the same results as you would for GST_RESOURCE_ERROR_READ

The goal here was to aim for making it so simple people stop wimping out
on actually having error statements in the code at all.  90% of the
plugins had only one or two errors, for either "didn't negotiate before
chain/loop" or "didn't negotiate", and they were all copies of each
other.  Some plugins had a bit more.  And all of Ronald's plugins had
huge amounts of error calls listing everything in detail :) So the only
one that actually thought hard about errors up to now was for Ronald -
but don't ask me why.

Now, there is some policy for throwing errors, and the syntax for is
might be quirky at first, but - and I can attest after having gone
through all of them - at least it's not too hard to remember and not too
long to type.


> - gst_element_error sacrifies type checking for simplicity by simply
> casting its first argument to a GstElement, no matter what. I'm not sure
> it should be done that way. All other functions take the exact argument
> for a reason (and casting GstElements to GstBins everytime you call
> gst_bin_add needs to be done, too...). This is more a general "should we
> always or never use gpointer instead of GstSomeType *" question.

I'm not sure either, but IIRC Ronald asked for that.  And, to be honest,
I don't really see why it's a problem either.  Again, the goal is "short
error throwing statements".  I don't know if GST_ELEMENT typechecks or
not (IIRC the glib macros do).  IMO it should, and it would catch misuse
here.

> - GstError totally bypasses the debugging system. I'm never informed via
> debugging output that a gst_element_error happened.

gstelement.c:2406:
  GST_CAT_INFO (GST_CAT_ERROR_SYSTEM, "signaling error in %s: %s",
                GST_ELEMENT_NAME (element),
                sent_message);

> - There is no clear requirement that debugging messages should include
> stuff that has already put into the error message, so that the debugging
> message is complete and understandable by the developer.

Yeah - and I disagree on this.  The developer can easily see the error
thrown anyway since he has source file, line number, and function name.

> - I'd let gst_element_error_extended (glib would have used "_full"
> there...) 

Changed to _full

> take const message and debug strings and let gst_element_error free
> them.

Originally I didn't want to as I wanted to keep the function macro a
true function replacement without using braces.  I had to change it
anyway because the C++ plugins were complaining.  OTOH, I'm not sure if
this is useful for anything.  What's the advantage ?

Thanks for the feedback, I'd like to settle things before we release.  I
hope others chime in too.

Thomas

Dave/Dina : future TV today ! - http://www.davedina.org/
<-*- thomas (dot) apestaart (dot) org -*->
And I know
they all look so good from a distance
but I tell you I'm the one
<-*- thomas (at) apestaart (dot) org -*->
URGent, best radio on the net - 24/7 ! - http://urgent.fm/