GNOME Bugzilla – Bug 737683
fakesrc: the "pattern" property can be gotten but it is never set or used
Last modified: 2014-10-03 09:38:33 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?
Mmh, I meant that it can be gotten, but it can't be set, even if it's marked as G_PARAM_READWRITE.
Going to check this today. Thanks for reporting it Antonio!
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.
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.
What would be the pattern="" you would set for your MIDI example? I'm curious.
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.
Antonio, 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. Thanks, Luis
Comment on attachment 287579 [details] [review] Removing unused property and variable Merged
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.
That's fine. Thanks.
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 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.
Created attachment 287651 [details] [review] revert previous commit Cleaner
Created attachment 287652 [details] [review] One more little fix. Ready to merge.
Comment on attachment 287652 [details] [review] One more little fix. Merged