[Bug 744922] osxaudiosrc: iOS resampling is stuttering

[GStreamer (GNOME Bugzilla)]

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

--- Comment #17 from Ilya Konstantinov <ilya.konstantinov at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #15)
> Don't worry about non-interleaved audio for now, almost no element supports
> it yet :) Let's get this bug fixed first and then we can worry about
> non-interleaved audio at a later time.

Right. As I wrote in the comment:
    (This patch addresses some non-interleaved audio concerns, but
    at this moment the elements do not support non-interleaved audio
    and non-interleaved is untested.)

(In reply to Arun Raghavan from comment #13)
> +  /* TODO: To support non-interleaved audio, go over all mBuffers,
> +   *       not just the first one. */
> 
> Wouldn't the mDataByteSize be the same for all buffers (it'd be pretty weird
> to get a different number of buffers for each channel).

I was just referring to the fact current code says "mBuffers[0]", while
non-interleaved will have NumberOfChannel buffers. As to whether it makes sense
for them to have a different mDataByteSize (i.e. number of bytes returned)
values for different buffers, I guess you're right.

>    AudioBufferList *list;
> +  gsize list_size;
> +  UInt32 num_buffers, n;
> 
> Any reason to change this to UInt32? (and it'd be guint32, to be consistent
> and glib-y)

I'm using UInt32 *only* for Core Audio-derived values, same as I use gsize and
not size_t for Glib.

> @@ +170,3 @@
>      list->mBuffers[n].mDataByteSize = size;
>      list->mBuffers[n].mData = g_malloc (size);
> +    if (list->mBuffers[n].mData == NULL) {
> 
> I'm pretty sure we don't actually deal with g_malloc() failing anywhere, and
> if it did, the app would have a bigger catastrophe in its hands. The NULL
> check is likely just going to be deadcode, in that case, and is better just
> skipped.

Actually, it's my bad. g_malloc aborts on failure, g_try_malloc is the one that
returns NULL. I'll remove the check -- it's in fact dead code.

> @@ +182,3 @@
>  buffer_list_free (AudioBufferList * list)
>  {
> +  UInt32 n;
> 
> Again, any particular reason for changing the type?

Basically, it eliminated the (int) cast in:

  for (n = 0; n < (int) list->mNumberBuffers; ++n) {

> @@ +186,2 @@
> +  for (n = 0; n < list->mNumberBuffers; ++n) {
> +    g_free (list->mBuffers[n].mData);
> 
> g_free() is a noop if the data is NULL.

Which is exactly why I removed the "if (list->mBuffers[n].mData)" test.

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