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 577827 - [appsink] Have appsink new_buffer-callback return GstFlowReturn
[appsink] Have appsink new_buffer-callback return GstFlowReturn
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal blocker
: 0.10.23
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-04-03 10:20 UTC by Jonas
Modified: 2009-04-09 21:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make new_buffer-callback return GstFlowReturn. Cleanup documentation. (2.04 KB, patch)
2009-04-03 12:45 UTC, Martin Samuelsson
committed Details | Review

Description Jonas 2009-04-03 10:20:44 UTC
Would it be possible to have a return value from the callback-function? So that if something goes wrong with the handling of the buffer, you could alert the pipeline easily. As I see it the callback-funktion is a part of the flow, a big part of the sink, if you use it, and returning a GstFlowReturn would make sense.


Thoughts?
Comment 1 Jonas 2009-04-03 10:21:39 UTC
Current prototypes:

typedef struct {
  void (*eos)           (GstAppSink *sink, gpointer user_data);
  void (*new_preroll)   (GstAppSink *sink, gpointer user_data);
  void (*new_buffer)    (GstAppSink *sink, gpointer user_data);

  /*< private >*/
  gpointer     _gst_reserved[GST_PADDING];
} GstAppSinkCallbacks;
Comment 2 Wim Taymans 2009-04-03 10:56:27 UTC
That's a good suggestion. I'm not sure we can change that now because it would break API and ABI.
Comment 3 Jan Schmidt 2009-04-03 11:12:05 UTC
yeah, it's too late to change those entries now. It'd require adding new variants that would be called (if set) in preference to these entries, and use up some of the padding.
Comment 4 Jan Schmidt 2009-04-03 11:26:04 UTC
Actually, after a bit more discussion on IRC, it looks like the API added for bug #571299 didn't make it into the 0.10.22 base release.

That would mean it's still open to change before the 0.10.23 release in a few weeks.
Comment 5 Martin Samuelsson 2009-04-03 12:45:46 UTC
Created attachment 131994 [details] [review]
Make new_buffer-callback return GstFlowReturn. Cleanup documentation.

- Make new_buffer-callback return GstFlowReturn.
- Cleanup documentation. These callbacks are not signals.

This patch does not take into consideration whether the new_preroll- or eos-callbacks should have return values added as well.
Comment 6 Wim Taymans 2009-04-09 16:32:04 UTC
Marking as blocker, we need to get this in before the api is frozen.
Comment 7 Wim Taymans 2009-04-09 21:45:18 UTC
I also changed the new_preroll callback, just because we can.

commit ee03bf5379d88dd33a7de0d68ea1cbe47011e312
Author: Martin Samuelsson <martin.samuelsson at axis.com>
Date:   Thu Apr 9 23:46:17 2009 +0200

    appsink: make callbacks return GstFlowReturn
    
    Make the new_buffer and new_preroll callbacks return a GstFlowReturn so that
    errors can be reported properly.
    Fixes #577827.