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 685249 - add audio to tab-separated values encoder
add audio to tab-separated values encoder
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-10-01 21:03 UTC by Kipp
Modified: 2018-11-03 13:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add tsvenc to the debugutils plugin (21.49 KB, patch)
2012-10-01 21:03 UTC, Kipp
needs-work Details | Review
add tsvenc element to debugutilsbad plugin (23.71 KB, patch)
2014-02-26 21:04 UTC, Kipp
none Details | Review
add tsvenc element to debugutilsbad plugin (22.77 KB, patch)
2014-02-26 23:56 UTC, Kipp
none Details | Review

Description Kipp 2012-10-01 21:03:32 UTC
Created attachment 225536 [details] [review]
add tsvenc to the debugutils plugin

the gstlal project provides an "nxydump" element that converts audio streams to white-space delimited multi-column ascii text.  the nxydump element gets its name from the corresponding input format option for grace, and has been used extensively for sample-by-sample decoding of audio filters.

this ehancement request is for pushing the nxydump element upstream to gst-plugins-bad, adding it to the debugutils plugin.  after some research, it was found that "tab-separated values" is an official mime type, so the element's output format now matches that format specifically, the output caps are "text/tab-separated-values", and the element's name in this patch is "tsvenc".  searching for "tsv" on both google and duckduckgo gives wikipedia's page on the tab-separated values format as the first link (at least when I try it).
Comment 1 Kipp 2012-10-01 21:06:38 UTC
sorry, "... sample-by-sample *debugging* of audio filter."
Comment 2 Sebastian Dröge (slomo) 2012-10-14 08:46:23 UTC
Review of attachment 225536 [details] [review]:

* Could you run all this through gst-indent (gstreamer/tools)?
* Maybe the separator character could be made configurable via a property (and be part of the caps then, defaulting to \t)
* Maybe add some docs there about how this output can be used with nxydump and where nxydump is found

::: gst/debugutils/gsttsvenc.c
@@ +141,3 @@
+
+
+static int printsample_double(char *location, const void **sample)

All these printsample functions could be generalized with a macro that generates the functions:
#define CREATE_PRINTFUNC(type, fmt) \
static int printsample_#name (char *location, const void ** sample) \
  return sprintf (location), "\t" fmt, (g#type) *(*(const g#type **) sample)++); \
}

CREATE_PRINTFUNC(double, "%.16g");
CREATE_PRINTFUNC(uint32, "%u");

@@ +228,3 @@
+
+    for(channel = 0; channel < element->channels; channel++)
+      location += printsample(location, &samples);

Am I missing something or is this never incrementing samples, thus always printing the same value (not incrementing for iterating over the channels and also not incrementing for iterating the samples).

@@ +295,3 @@
+{
+  ARG_START_TIME = 1,
+  ARG_STOP_TIME

Rename to PROP_ from ARG_ please
Also shouldn't this be taken from the configured GstSegment?

@@ +327,3 @@
+
+    if(success)
+      *size = GST_AUDIO_INFO_WIDTH(&info) / 8 * GST_AUDIO_INFO_CHANNELS(&info);

You can use BPF here

@@ +352,3 @@
+      caps =
+          gst_caps_copy(gst_pad_get_pad_template_caps
+          (GST_BASE_TRANSFORM_SINK_PAD(trans)));

You're leaking the caps here, get_pad_template_caps() already returns a new ref in 1.0 (also no need to copy, returning a new ref is enough)

@@ +358,3 @@
+      caps =
+          gst_caps_copy(gst_pad_get_pad_template_caps(GST_BASE_TRANSFORM_SRC_PAD
+              (trans)));

Same here

@@ +499,3 @@
+          && GST_BUFFER_OFFSET_END_IS_VALID(inbuf))) {
+    GST_ERROR_OBJECT(element,
+        "cannot compute number of input samples:  invalid offset and/or end offset");

Don't depend on the offset to be set, just calculate the number of samples yourself with the unit size

@@ +516,3 @@
+    stop =
+        timestamp_to_sample_clipped(GST_BUFFER_TIMESTAMP(inbuf), length,
+        element->rate, element->stop_time);

I don't think this is doing what you want, if I understand this correctly you want to convert the timestamp to samples and clip to the [0, stop-start] and offset by start. The function does not do that with these parameters

::: gst/debugutils/gsttsvenc.h
@@ +54,3 @@
+  gint rate;
+  gint channels;
+  gint unit_size;

Maybe use GstAudioInfo for this?
Comment 3 Kipp 2014-02-26 21:04:40 UTC
Created attachment 270419 [details] [review]
add tsvenc element to debugutilsbad plugin

rework of tsvenc patch.

 * add filter constraint to caps negotiation
 * add documentation (sort of)
 * passes gst-indent check

with respect to documentation, I can't figure out how to incorporate the documentation into the build.  this patch takes a stab at it, but it's not correct.  can I get help with this?

with respect to the request to configure the delimeter, the "tab-separated value" format is strict that the delimeter be the tab character (not whitespace, not "any number tabs", but 1 tab).  allowing the delimeter to be a comma would make this also a CSV encoder, so I can see the desire to provide that generalization, but the problem with comma-delimited fields and numerical data is that some locales use "," as the separator between integer and fractional parts of numbers.  the column count and column boundaries in a CSV-format file containing purely numerical data is ambiguous because of this (unless the locale is known).  are there use cases where the tab-separated value format would not be suffiicent?
Comment 4 Kipp 2014-02-26 23:56:10 UTC
Created attachment 270440 [details] [review]
add tsvenc element to debugutilsbad plugin

rework again, addressing more of the comments above (sorry, I hadn't noticed the comments, I thought the text was quoting the diff from gst-indent).
Comment 5 GStreamer system administrator 2018-11-03 13:12:44 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/77.