After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 109857 - GStreamer application development manual encourages incorrect g_error() usage
GStreamer application development manual encourages incorrect g_error() usage
Status: RESOLVED DUPLICATE of bug 157669
Product: GStreamer
Classification: Platform
Component: documentation
git master
Other Linux
: Normal enhancement
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 120994
 
 
Reported: 2003-04-03 08:35 UTC by Andres Salomon
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
propes patch for dynamic.xml (fixes code only, the pipeline itself does not work!) (9.88 KB, patch)
2004-03-13 17:11 UTC, Tim-Philipp Müller
none Details | Review

Description Andres Salomon 2003-04-03 08:35:40 UTC
This has been bugging me while reading the documentation.  A lot of the
examples have code like (taken from chap. 19):

int 
main (int argc, char *argv[]) 
{
  GstElement *pipeline, *src, *demux;

  gst_init (&argc, &argv);

  pipeline = gst_pipeline_new ("pipeline");
  g_return_val_if_fail (pipeline != NULL, -1);


According to
http://developer.gnome.org/doc/API/2.0/glib/glib-Error-Reporting.html:
"If the programmer has screwed up, then you should use g_warning(),
g_return_if_fail(), g_assert(), g_error(), or some similar facility.
(Incidentally, remember that the g_error() function should only be used for
programming errors, it should not be used to print any error reportable via
GError.)"

In other words, g_return_val_if_fail() should be used for programmer checks
(ie, at the beginning of functions, to ensure an argument that was passed
was not NULL), not runtime errors.  gst_pipeline_new() (well,
gst_element_factory_make) may return NULL; this is a valid return value,
according to the API docs.  It is not a programmer error, but rather, a
runtime error (running out of memory, unable to load a plugin/module, etc).
 Things like that would normally use GError (if they desire details of the
failure).

To encourage usage of g_return_val_if_fail() for such situations is a very
bad idea; programmers will later on have to unlearn techiques they've been
taught from the gst docs.  In place of g_return_val_if_fail(), i'd suggest
something similar to
if (pipeline == NULL) {
    g_printerr("could not allocate pipeline!");
    return -1;
}

I'll submit patches if you agree.
Comment 1 Benjamin Otte (Company) 2003-04-03 11:22:13 UTC
Agreed, this is wrong behaviour. Patches are very welcome.
Comment 2 Tim-Philipp Müller 2004-03-13 17:09:38 UTC
Attached a patch that has better error handling.  
 
Other things that were fixed: 
 
 * added a handler for the pipeline's "error" signal 
 * renamed 'eof' handler 
 * try not to just call 'exit()' anywhere (makes it 
    harder to integrate the example with application 
    error handling etc.). 
 
 
Not sure if the error handling doesn't make the example 
unnecessarily messy. 
 
 
*** NOTE *** 
 
The example seems to be outdated and does not work. The 'colorspace' 
element fails with an assertion warning on every mpeg video I've 
tried. Maybe the example needs to be changed to use gst-ffmpeg 
plugins instead? (I could make it work with ffmpegcolorspace, but 
then the video would run twice as fast as the audio, with any 
scheduler, so I just left it as it is) 
 
Cheers 
-Tim 
 
Comment 3 Tim-Philipp Müller 2004-03-13 17:11:02 UTC
Created attachment 25591 [details] [review]
propes patch for dynamic.xml  (fixes code only, the pipeline itself does not work!)
Comment 4 Ronald Bultje 2004-11-08 16:02:32 UTC
The whole appdevman is horribly out-of-date. I'll merge this bug with the
tracker, I'll work on rewriting large parts of it (including making those
examples work again) sometime soon.

*** This bug has been marked as a duplicate of 157669 ***