GNOME Bugzilla – Bug 685249
add audio to tab-separated values encoder
Last modified: 2018-11-03 13:12:44 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).
sorry, "... sample-by-sample *debugging* of audio filter."
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?
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?
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).
-- 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.