[jitsi-dev] [libjitsi] Remove condition for S16LE sample format (#36)


#1

This prevents any device registered as a different format from
showing up in the device list.

This can be reproduced by overriding the pulseaudio default for sample rate in /etc/pulse/default.pa

default-sample-format = s32le

This condition prevented one of two sound cards from appearing in the device list only for pulseaudio. PortAudio worked Ok when selecting the "default" devices and configuring pulseaudio appropriately. It was clear that the sound card was fully functional. After removing this condition, the sound card now appears (and functions) as expected in both capture and playback device lists.
You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/libjitsi/pull/36

-- Commit Summary --

  * Remove condition for S16LE sample format

-- File Changes --

    M src/org/jitsi/impl/neomedia/device/PulseAudioSystem.java (11)

-- Patch Links --

https://github.com/jitsi/libjitsi/pull/36.patch
https://github.com/jitsi/libjitsi/pull/36.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/36


#2

What are the values of the variable sampleSpecFormat on your system i.e. what are the values returned by the functions sink_info_get_sample_spec_format and source_info_get_sample_spec_format? I'm asking because our PulseAudio-based AudioSystem implementation does require S16LE.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/36#issuecomment-82857082


#3

source_info_get_sample_spec_format() returns 7 instead of the expected 3 as defined in PA.java when default-sample-format = s32le.

Where is the requirement enforced in AudioSystem? I could instead add commentary (and logging) indicating what's happening instead of removing the condition. I could also address Matt's concerns about superfluous checks, and monitors of sinks.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/36#issuecomment-83026305


#4

7 appears to represent S32LE.

The value 7 appears to coincide with the position in the table as defined in src/pulse/sample.h here:
http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/pulse/sample.h#n134

If the table is 0 indexed, it puts S16LE at 3, and S32LE at 7. I don't know C very well so I can only speculate, but it makes sense to me.

Matt had mentioned that the checks are superfluous and it seems he's correct since it appears to not matter at all what format the device is since S16LE is chosen as the only format.

I imagine that the checks may be there to make sure that S16LE is only used on a device configured for S16LE, however, in my environment, my device was S32LE, and worked regardless. Matt also mentioned this: I guess this is because Jitsi internally works with s16le and leaves pulseaudio the task of down/up sampling. If that's indeed how it functions, then the down/up sampling would be handled by pulseaudio which might explain why it worked with my device set as S32LE. So libjitsi was using S16LE, and pulseaudio up sampled it to S32LE.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/36#issuecomment-83054802


#5

I could also address Matt's
      concerns about superfluous checks, and monitors of sinks.

I agree with Matt's points on superfluous checks. Thank you very much, Matt! However, since they are not necessary in order to fix the issue targeted by this pull request, I'd like to keep the changes implied by them separate.

@agh1467, if we are to merge any pull request of yours, we'll need you to sign our contributor agreement at http://bluejimp.com/bca.pdf and send it to support@bluejimp.com.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/36#issuecomment-83066330


#6

Merged #36.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/36#event-265112925