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 795144 - gstsample: new API
gstsample: new API
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal enhancement
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 795145
 
 
Reported: 2018-04-10 22:57 UTC by Mathieu Duponchelle
Modified: 2018-04-13 21:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gstsample: new API (4.72 KB, patch)
2018-04-10 22:57 UTC, Mathieu Duponchelle
none Details | Review
gstsample: new API (7.19 KB, patch)
2018-04-11 17:08 UTC, Mathieu Duponchelle
committed Details | Review

Description Mathieu Duponchelle 2018-04-10 22:57:04 UTC
See commit message
Comment 1 Mathieu Duponchelle 2018-04-10 22:57:12 UTC
Created attachment 370759 [details] [review]
gstsample: new API

gst_sample_set_buffer
gst_sample_is_writable
gst_sample_make_writable

This commit makes it possible to reuse a sample object and avoid
unnecessary memory allocations, for example in appsink.

In addition, writability is now required to set the buffer list.
Comment 2 Sebastian Dröge (slomo) 2018-04-11 08:39:50 UTC
Comment on attachment 370759 [details] [review]
gstsample: new API

This is not enough. You potentially also need to change the caps, segment, etc. when you change the buffer :) This also needs some new API
Comment 3 Mathieu Duponchelle 2018-04-11 17:08:01 UTC
Created attachment 370815 [details] [review]
gstsample: new API

gst_sample_set_buffer
gst_sample_set_caps
gst_sample_set_segment
gst_sample_set_info
gst_sample_is_writable
gst_sample_make_writable

This commit makes it possible to reuse a sample object and avoid
unnecessary memory allocations, for example in appsink.

In addition, writability is now required to set the buffer list.
Comment 4 Sebastian Dröge (slomo) 2018-04-11 18:58:46 UTC
Review of attachment 370815 [details] [review]:

::: gst/gstsample.c
@@ +312,3 @@
+    gst_segment_copy_into (segment, &sample->segment);
+  else
+    gst_segment_init (&sample->segment, GST_FORMAT_TIME);

Why TIME and not UNDEFINED?
Comment 5 Mathieu Duponchelle 2018-04-12 14:10:26 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)

> Why TIME and not UNDEFINED?

For consistency with gst_sample_new :)
Comment 6 Sebastian Dröge (slomo) 2018-04-12 15:55:02 UTC
(In reply to Mathieu Duponchelle from comment #5)
> (In reply to Sebastian Dröge (slomo) from comment #4)
> 
> > Why TIME and not UNDEFINED?
> 
> For consistency with gst_sample_new :)

I don't like that but ok :) Go for it then and add a FIXME 2.0 comment there for switching to UNDEFINED
Comment 7 Mathieu Duponchelle 2018-04-13 21:55:19 UTC
Attachment 370815 [details] pushed as ed5d888 - gstsample: new API