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 743925 - osxaudiosink won't reconfigure sink caps
osxaudiosink won't reconfigure sink caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.4.5
Other Mac OS
: Normal normal
: 1.5.1
Assigned To: Ilya Konstantinov
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-03 11:52 UTC by Ilya Konstantinov
Modified: 2015-03-03 18:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.23 KB, patch)
2015-02-21 16:52 UTC, Ilya Konstantinov
none Details | Review
osxaudiosink: Allow renegotiating caps (1.76 KB, patch)
2015-03-03 08:33 UTC, Arun Raghavan
none Details | Review
Test program for ringbuffer renegotiation (659 bytes, text/plain)
2015-03-03 08:35 UTC, Arun Raghavan
  Details
My patch revised (1.39 KB, patch)
2015-03-03 14:11 UTC, Ilya Konstantinov
committed Details | Review

Description Ilya Konstantinov 2015-02-03 11:52:49 UTC
Once osxaudiosink's device is open, it fixates on the initial caps and refuses to accept new caps. This is erroneous since the Audio Unit is can accept a new ASBD, and GstAudioRingBuffer supports reconfiguration as well.

(This causes a problem, e.g. with interaudiosrc, which might produce a silent buffer with rate:48000 before producing one coming from the interaudiosink with actual rate:$MY_PIPELINE_RATE.)

Therefore, we need to change:

static GstCaps *
gst_osx_audio_sink_getcaps (GstBaseSink * sink, GstCaps * filter)
{
   ...
#if 0
    if (buf->acquired && buf->spec.caps) {
      /* Caps are fixed, use what we have */
      ret = gst_caps_ref (buf->spec.caps);
    }
#endif
    ....

I've tested this on iOS 8. I will submit a patch once I test it on OS X too.
Comment 1 Sebastian Dröge (slomo) 2015-02-12 09:00:05 UTC
What's the status here? It should indeed just work, the ringbuffer handles all the reconfiguration for us.
Comment 2 Ilya Konstantinov 2015-02-12 19:36:46 UTC
I've researched it, implemented above fix in my code and it works fine on iOS.

I won't have time till mid-next week, so either I'll submit a patch (after I take care of the osx test code) or, go ahead :)
Comment 3 Ilya Konstantinov 2015-02-21 16:52:22 UTC
Created attachment 297508 [details] [review]
Patch

Tested on iOS, by observing the actual interaudiosrc / interaudiosink issue we've seen in OpenWebRTC is fixed.

Tested on OS X, through a custom test program that:
1) Creates 'osxaudiosrc ! queue ! audioconvert ! audioresample ! capsfilter ! queue ! osxaudiosink'.
2) After a few seconds, sets caps=audio/x-raw,rate=8000 on the capsfilter.
   Before this fix, this will result in a negotiation error.
Comment 4 Arun Raghavan 2015-03-03 08:33:25 UTC
Created attachment 298378 [details] [review]
osxaudiosink: Allow renegotiating caps

I've made a couple of small modifications to your patch for correctness. But this doesn't actually work for me (neither did your old patch) -- after renegotiation, I hear no audio.

I'll attach a small test program to make sure we're on the same page.
Comment 5 Arun Raghavan 2015-03-03 08:35:32 UTC
Created attachment 298379 [details]
Test program for ringbuffer renegotiation

Tested that this works with pulsesink on Linux, but with osxaudiosink, there is only silence after renegotiation. I do see the render function being called on the sink, so not sure why the data is lost.
Comment 6 Sebastian Dröge (slomo) 2015-03-03 08:53:07 UTC
We might be reading silence from the ringbuffer, which happens if we read areas that are not written yet (i.e. read to fast). Check where the silence first appears with gst_util_dump_mem() or similar :)
Comment 7 Arun Raghavan 2015-03-03 11:45:38 UTC
Nice catch -- if I wait longer after reconfigure it does start playing, so I guess it seems to be writing at the offset it expected, but the ringbuffer has been reset, so we have silence for that period.
Comment 8 Ilya Konstantinov 2015-03-03 14:03:01 UTC
On my OS X, the test program exhibits the same behavior without the patch as well.

About the correction to the (In reply to Arun Raghavan from comment #4)
> I've made a couple of small modifications to your patch for correctness.

The old code didn't get template caps either if the ring buffer wasn't open.

I would however change this:
    if (!ret && osxsink->cached_caps)
      ret = gst_caps_ref (osxsink->cached_caps);
to:
    if (osxsink->cached_caps)
      ret = gst_caps_ref (osxsink->cached_caps);

I know the current get_caps are bad, but I was planning to address this in the mega-patch to bug 743758.
Comment 9 Ilya Konstantinov 2015-03-03 14:11:20 UTC
Created attachment 298433 [details] [review]
My patch revised
Comment 10 Arun Raghavan 2015-03-03 18:01:47 UTC
Comment on attachment 298433 [details] [review]
My patch revised

I'd already fixed this -- guess I'd attached an old version of the patch. Pushed now, and fixed for osxaudiosrc too. Thanks!
Comment 11 Arun Raghavan 2015-03-03 18:03:07 UTC
As an additional note, baseaudiosink was also fixed to reset the audio clock if needed (the problem discussed in comment #7).