[Bug 751605] audio: Add proper support for non-interleaved / planar audio

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=751605

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #371747|none                        |reviewed
             status|                            |

--- Comment #27 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 371747:
 --> (https://bugzilla.gnome.org/review?bug=751605&attachment=371747)

::: gst-libs/gst/audio/audio-buffer.c
@@ +120,3 @@
+
+      if (!gst_buffer_map_range (gstbuffer, idx, length,
&buffer->map_infos[i],
+              flags))

This requires that each channel has all samples in the same memory, right?
Because if it was in two memories, they would have to be merged but we already
have one of the memories mapped then so they can't be merged

Should be documented

::: gst-libs/gst/audio/audio-buffer.h
@@ +52,3 @@
+  gpointer    *planes;
+
+  GstBuffer   *buffer;

I would prefer a GstAudioInfo to be here, and to get it from a parameter in
gst_audio_buffer_map(). Without the GstAudioInfo, you always need to carry two
things around to have a useful GstAudioBuffer, it would be nicer to have it all
in a single data structure

(And mismatch between the meta audio info and the parameter would be a
g_critical() and we use the one from the meta)

::: gst-libs/gst/audio/gstaudiometa.c
@@ +367,3 @@
+ * offsets[channel] = channel * @samples * bytes_per_sample
+ *
+ * Returns: the #GstAudioMeta that was attached on the @buffer

Somewhere it should be documented that it's either allowed for the offsets to
overlap, or not. And if not, we should check for that. I'd prefer not :)

@@ -307,0 +307,115 @@
+
+
+static gboolean
... 112 more ...

This looks more like a g_critical()

::: gst-libs/gst/audio/gstaudiometa.h
@@ +148,3 @@
+ * attached and to be mapped with gst_audio_buffer_map() in order to ensure
+ * correct handling of cliping and channel reordering (which is internally
+ * implemented by altering the @offsets and the @samples variables).

Maybe add a paragraph how this would be used for interleaved audio, and that
it's useless at this point but might get additional functionality added at a
later time

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.