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 349015 - [sunaudio] open source with O_NONBLOCK
[sunaudio] open source with O_NONBLOCK
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other opensolaris
: Normal normal
: 0.10.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2006-07-28 00:26 UTC by Brian Cameron
Modified: 2006-12-08 14:33 UTC
See Also:
GNOME target: ---
GNOME version: ---

patch fixing open to be non-blocking (518 bytes, patch)
2006-07-28 00:26 UTC, Brian Cameron
committed Details | Review
patch (737 bytes, patch)
2006-08-10 07:45 UTC, jerry tan
committed Details | Review
possible alternative (1.75 KB, patch)
2006-09-14 12:16 UTC, Tim-Philipp Müller
rejected Details | Review

Description Brian Cameron 2006-07-28 00:26:16 UTC
Attaching patch that simply modifies the open() call to add O_NONBLOCK so
the device is opened in non-blocking mode.  Otherwise programs like gnome-sound-recorder hang if you try to record more than one file at the same time.
Comment 1 Brian Cameron 2006-07-28 00:26:56 UTC
Created attachment 69774 [details] [review]
patch fixing open to be non-blocking
Comment 2 jerry tan 2006-08-10 07:45:17 UTC
Created attachment 70617 [details] [review]

please use this patch, 
under non-blocking mode, read may lost some data, according to audio man page on solaris.

so please use this patch instead, thanks.
Comment 3 Jan Schmidt 2006-08-10 08:56:40 UTC
This patch doesn't look right - it takes away the NONBLOCK parameter to open, which means open will block and the ioctl won't get to run until later, no?

Comment 4 Tim-Philipp Müller 2006-08-10 09:24:04 UTC
From the audio man page on Solaris 9:

     When a process cannot open /dev/audio because the device  is

        o  if either the O_NDELAY or O_NONBLOCK flags are set  in
           the  open()  oflag  argument,  then  -1 is immediately
           returned, with errno set to EBUSY.

        o  if neither the O_NDELAY nor the  O_NONBLOCK  flag  are
           set,  then  open() hangs until the device is available
           or a signal is delivered to the process, in which case
           a -1 is returned with errno set to EINTR.  This allows
           a process to block in the open call while waiting  for
           the audio device to become available.

As Jan already said, the purpose of the NONBLOCK parameter was to avoid hanging in open if opening the device is not possible right away.


  Recording Audio Data
     The read() system call copies data from the system's buffers
     to the application. Ordinarily, read() blocks until the user
     buffer is filled. The I_NREAD ioctl (see  streamio(7I))  may
     be  used  to  determine  the amount of data that may be read
     without blocking.  The device may alternatively be set to  a
     non-blocking  mode,  in  which case read() completes immedi-
     ately, but may return fewer bytes than requested.  Refer  to
     the  read(2)  manual page for a complete description of this

only says that in non-blocking mode read() may return fewer bytes than requested, which doesn't mean data is lost, does it? (this is standard behaviour for read() in non-blocking mode for all devices as far as I know).

But maybe we should just set the fd back to non-blocking again with fcntl() after opening it successfully?
Comment 5 Brian Cameron 2006-08-10 18:58:47 UTC
The main reason that we want to remove NONBLOCK is because when we use this flag we notice that the recorded input "stutters" so there are segments of silence in the recorded data, like it is only recording sometimes.

I think we need to research this a bit more and make sure that we use the correct options here.

Note that we might not be managing the return code from read() properly.  When a device is non-blocking, it will reutrn empty stuff from read with EINTR, indicating that we should retry the read() or it will return partial reads.  This may be the cause of the "stuttering" effect we notice when we turn on NONBLOCK mode.  Can we check this?

Also, according to Jerry, multiple processes can open the audio device even if it open in NONBLOCK mode.  Using NONBLOCK only allows the same program to open the device more than once.  Can we verify that this is all that NONBLOCK provides?
Comment 6 jerry tan 2006-08-11 04:18:31 UTC
     The device may alternatively be set to  a
     non-blocking  mode,  in  which case read() completes immedi-
     ately, but may return fewer bytes than requested.

according to my testcase, it really lost data.
so I make the patch to remove it.

And for the hang issue, 
I have verified that two process open /dev/audio device wont hang, 
only when a process open it twice, it will hang.
to avoid it, I set its properties as AUDIO_MIXER_MULTIPLE_OPEN
Comment 7 Brian Cameron 2006-08-11 06:05:10 UTC
Jerry, it doesn't seem like you have done any of the research that was pointed out by the engineers who have reviewed your patch.

Some people have suggested that NONBLOCK may be better, if we can get it working.  Just because we notice lost data when we have NONBLOCK does not mean that the NONBLOCK flag is the problem.  As I mentioned in my previous comment, we need to test to see if NONBLOCK works if we manage EINTR/EAGAIN making the read
function look something like this

   do {
      ret = read (sunaudiosrc->fd, data, length);
   while (ret == EINTR || ret == EAGAIN);

   return ret

Does NONBLOCK work if you try something like this?  

In other words, the patch you provided may be a good workaround to make things
work for now, but might not really be the best solution.

Also, I'm not sure that the MIXER ioctl belongs in the source.  Note that the source plugin calls gst_sunaudiomixer_ctrl_new, which calls gst_sunaudiomixer_ctrl_open.  Might be better to put this ioctl in the mixer code.

Comment 8 Brian Cameron 2006-08-11 22:27:56 UTC
After further review and testing with Jerry we have determined that the MIXER ioctl does belong in the source plugin since the ioctl must be called on /dev/audio and not /dev/audioctrl.  So I think that part of the patch is ready to go in.  After Jerry tests to see if handling EINTR and EAGAIN makes the plugin work with NONBLOCK turned on, we can decide whether to go with this patch or a modified patch.

Note that we here at Sun are doing a lot of work trying to figure out the best way to call the SunAudio interfaces to support the various GStreamer-based applications.  I suspect we will be suggesting various tweaks to this code over the next few weeks as we figure things out.

I do recommend that we get something like the proposed patch into the codebase soon, though, because the source plugin doesn't work if NONBLOCK is there, and gnome-sound-recorder hangs without the ioctl call.  So without fixing these issues a lot of functionality is unuable.  Even if this isn't the final patch, this at least gets things so they mostly work.
Comment 9 Tim-Philipp Müller 2006-09-04 15:13:37 UTC
Any update on this one? Is there a modified patch you would like committed in the meantime?
Comment 10 Brian Cameron 2006-09-04 20:30:19 UTC
Please accept Jerry's patch.  I think this is the best solution for now.  Without this patch gnome-sound-recorder simply does not work on Solaris.

We are seeing some performance issues on Solaris, so we may further rework the way we manage the audio device.  But let's handle that in a separate bug if we discover ways to improve it.
Comment 11 Tim-Philipp Müller 2006-09-14 12:16:48 UTC
Created attachment 72771 [details] [review]
possible alternative

What about something like this? Does this make things work as well?

Basically, IMHO at least, open() should never ever block under any circumstances, not if done from within the same process or done from multiple processes, not if done once or twice or fifteen times concurrently. It doesn't look like your patch achieves this to me.
Comment 12 jerry tan 2006-09-15 08:27:23 UTC
Tim's patch doesnot work, I have verified on my sparc.
open with NON_BLOCK option lost some data.
for example, when recording, I count 9, it only record 3 or less.

for sunaudio's API, I found that open device twice will be blocked for one process, 
but it works in two process.
one engineer tell me that "There is a multiple open ioctl that needs to be called first before a process can do multiple opens. It's documented in mixer (7i) man page."

so, I suggest to use my patch, since it is simple and works.

Comment 13 Brian Cameron 2006-12-06 19:15:33 UTC
I think we should accept this patch for now.  Recording doesn't work at all with solaris without this fix.  This fix seems to work okay, and unless you have other tests for us to try out, let's go with this workaround for now.  Okay?  We can leave the bug open if we want, but the patch should go in.
Comment 14 Jan Schmidt 2006-12-08 14:33:15 UTC
        * sys/sunaudio/gstsunaudiosrc.c: (gst_sunaudiosrc_open):
        Apply patch to open the mixer control and set the MULTIPLE_OPEN
        ioctl. On solaris, the mixer device doesn't need opening non-blocking 
        - it can be opened by multiple processes by default, but needs the ioctl
        for multiple opens within 1 process.
        Patch by: Jerry Tan <jerry.tan at sun dot com>
        Fixes: #349015