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 737683 - fakesrc: the "pattern" property can be gotten but it is never set or used
fakesrc: the "pattern" property can be gotten but it is never set or used
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
Other Linux
: Normal normal
: 1.5.1
Assigned To: Luis de Bethencourt
GStreamer Maintainers
Depends on:
Reported: 2014-09-30 20:36 UTC by Antonio Ospite
Modified: 2014-10-03 09:38 UTC
See Also:
GNOME target: ---
GNOME version: ---

Removing unused property and variable (2.85 KB, patch)
2014-10-02 12:52 UTC, Luis de Bethencourt
committed Details | Review
revert previous commit (3.00 KB, patch)
2014-10-03 09:01 UTC, Luis de Bethencourt
accepted-commit_now Details | Review
revert previous commit (3.03 KB, patch)
2014-10-03 09:22 UTC, Luis de Bethencourt
none Details | Review
One more little fix. (2.45 KB, patch)
2014-10-03 09:31 UTC, Luis de Bethencourt
committed Details | Review

Description Antonio Ospite 2014-09-30 20:36:10 UTC
The "pattern" property of fakesrc can be set, but it never actually used in the code; even when filltype==FAKE_SRC_FILLTYPE_PATTERN the code only uses src->pattern_byte, not the pattern passed from the user.

Maybe there was the idea to add support for arbitrary user-defined patterns but the implementation was never completed.

Should the property be removed?
Comment 1 Antonio Ospite 2014-09-30 20:41:28 UTC
Mmh, I meant that it can be gotten, but it can't be set, even if it's marked as G_PARAM_READWRITE.
Comment 2 Luis de Bethencourt 2014-10-01 09:42:19 UTC
Going to check this today.

Thanks for reporting it Antonio!
Comment 3 Luis de Bethencourt 2014-10-02 12:52:01 UTC
Created attachment 287579 [details] [review]
Removing unused property and variable

This patch removes the unused property. If there is interest I could bring it back having it fully support it.
Comment 4 Antonio Ospite 2014-10-02 14:17:01 UTC
Thanks for the patch.

It could be useful indeed to have a way to pass an arbitrary pattern to fill a buffer with in fakesrc.

I am doing some experiments with MIDI and GStreamer and if I was able to set the content of a buffer I could generate a certain note; passing data with fdsrc won't work because fdsrc cannot set the format to GST_FORMAT_TIME as needed downstream in the pipeline, while fakesrc is generic enough about segment formats and timing.

Just as an example of the possibilities, here is a funny pipeline with fakesrc and fluiddec:

gst-launch-1.0 fakesrc datarate=400 sizetype=2 sizemin=3 sizemax=3 num-buffers=1000 format=3 filltype=3 ! audio/x-midi-event ! fluiddec ! audioconvert ! autoaudiosink

The pipeline makes the random data (filltype=3) be interpreted as MIDI, which is fun but not very useful, if I could set the pattern I could decide what note to play.

Of course I am exploring the proper way to do things here, but for a quick hack fakesrc with pattern is a cool tool to have in the box.
Comment 5 Luis de Bethencourt 2014-10-02 14:19:50 UTC
What would be the pattern="" you would set for your MIDI example? I'm curious.
Comment 6 Antonio Ospite 2014-10-02 14:59:58 UTC
What I had in mind was a Note On message, something like pattern="\x90\x41\x7f" (Note On, key 65, velocity 127), adjusting the datarate property.

BTW, do we need a new filltype for a user specified pattern? The doc about fill types 5 and 6 explicitly mentions the sequence from 0x00 to 0xff.
Comment 7 Luis de Bethencourt 2014-10-02 16:31:39 UTC

The element had the pattern property as a string. So it looked like it was meant to support what you are asking for but it was never implemented. Going to discuss it with the maintainers and decide if this is worth implementing.

Comment 8 Luis de Bethencourt 2014-10-02 16:52:10 UTC
Comment on attachment 287579 [details] [review]
Removing unused property and variable

Comment 9 Luis de Bethencourt 2014-10-02 16:54:01 UTC
GStreamer maintainer thinks this property isn't useful enough. So he recommends we remove it.

He suggest you write a little script and use appsrc instead for your use case.
Comment 10 Antonio Ospite 2014-10-02 21:40:41 UTC
That's fine. Thanks.
Comment 11 Luis de Bethencourt 2014-10-03 09:01:15 UTC
Created attachment 287648 [details] [review]
revert previous commit

Revert previous commit, bring the property back (to not break ABI) but mark it as unused.
Comment 12 Tim-Philipp Müller 2014-10-03 09:09:15 UTC
Comment on attachment 287648 [details] [review]
revert previous commit

Looks mostly fine, but:

>+    case PROP_PATTERN:
>+      g_value_set_string (value, src->pattern);
>+      break;

Don't need the set_string here, since the pattern wasn't set anyway, just make it a break. The only thing we want to avoid is that people who have been using this property without realising that it doesn't work now don't get nasty criticals.

>diff --git a/plugins/elements/gstfakesrc.h b/plugins/elements/gstfakesrc.h
>index 5190ee8..e4acb9c 100644
>--- a/plugins/elements/gstfakesrc.h
>+++ b/plugins/elements/gstfakesrc.h
>@@ -134,6 +134,7 @@ struct _GstFakeSrc {
>   guint		parentsize;
>   guint		parentoffset;
>   guint8	 pattern_byte;
>+  gchar		*pattern;

Not needed any more then.
Comment 13 Luis de Bethencourt 2014-10-03 09:22:01 UTC
Created attachment 287651 [details] [review]
revert previous commit

Comment 14 Luis de Bethencourt 2014-10-03 09:31:58 UTC
Created attachment 287652 [details] [review]
One more little fix.

Ready to merge.
Comment 15 Luis de Bethencourt 2014-10-03 09:38:33 UTC
Comment on attachment 287652 [details] [review]
One more little fix.