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 393099 - GstBuffer copy vfunc broken?
GstBuffer copy vfunc broken?
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.10.13
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-01-05 11:41 UTC by Tim-Philipp Müller
Modified: 2007-03-09 16:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (3.62 KB, patch)
2007-03-06 09:35 UTC, Wim Taymans
none Details | Review

Description Tim-Philipp Müller 2007-01-05 11:41:51 UTC
The GstBuffer class ::copy() vfunc wrongly creates a new buffer with a GstBuffer GType and the size of a GstBuffer structure, instead of creating a new buffer with the same type and size as the input buffer.

This fails spectactularly in the case of GstNetBuffer from gst-plugins-base, which just chains up to GstBuffer::copy and then copies over some additional fields.

This might have been done on purpose, but I still wonder whether this isn't broken and should be fixed, since it kind of defeats the purpose of sub-classing (copying over the code from the copy vfunc into every subclass doesn't seem like a particularly good idea).

However, changing GstBuffer::copy() to create a new buffer struct of the type and size of the incoming buffer will probably break implementations of read-only buffers that have been built on top of this behaviour (such as GstSubBuffer and GstFileSrcMMapBuffer). The reason this breaks stuff is because ::copy() is used to create a writable buffer.

Possibilities:

 1) declare current behaviour as broken, and use
      copy = gst_mini_object_new (G_TYPE_FROM_INSTANCE (buffer))
    in GstBuffer::copy() and fix up read-only buffer subclasses to
    munge the GType before chaining up to the parent class copy function.
    This might break external uses of read-only buffer subclasses that
    chain up to GstBuffer::copy() though.

 2) just fix GstNetBuffer not to chain up and copy over code from gstbuffer.c

 3) make functionality from GstBuffer::copy() available as new function that
    operates on an already-copied buffer structure, so sub-classes just
    create their own copied buffer instance and then use that new function
    instead of chaining up to the parent class's copy function.

Opinions?
Comment 1 Antoine Tremblay 2007-01-08 18:29:24 UTC
If we can't do option 1)

I guess option 3) is best then since other plugins like v4lsrc (in copy mode) will need to copy the code from gstbuffer.c

Comment 2 Wim Taymans 2007-03-06 09:35:46 UTC
Created attachment 84043 [details] [review]
proposed patch

This patch implements option 3), it adds a new method to copy the metadata from one buffer into another one.
Comment 3 Wim Taymans 2007-03-09 16:30:59 UTC
I think this is the better API:

        * docs/gst/gstreamer-sections.txt:
        * gst/gstbuffer.c: (gst_buffer_copy_metadata), (_gst_buffer_copy):
        * gst/gstbuffer.h:
        Add metadata copy functions. Fixes #393099.

        * gst/gstutils.c: (gst_buffer_stamp):
        * libs/gst/base/gstbasetransform.c:
        (gst_base_transform_prepare_output_buffer):
        Use new metadata copy functions.