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 584252 - enhancements to OSSv4 plugin
enhancements to OSSv4 plugin
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
Other All
: Normal enhancement
: 0.10.14
Assigned To: Brian Cameron
GStreamer Maintainers
Depends on:
Reported: 2009-05-30 04:54 UTC by Garrett D'Amore
Modified: 2010-07-02 19:18 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26

My patches for OSSv4 plugin (29.41 KB, patch)
2009-05-30 04:56 UTC, Garrett D'Amore
none Details | Review
updated patch (26.33 KB, patch)
2009-06-03 18:23 UTC, Brian Cameron
none Details | Review

Description Garrett D'Amore 2009-05-30 04:54:57 UTC
Please describe the problem:
I'm the lead developer for Solaris' Boomer project, and as such, I've developed a significant set of enhancements to the OSSv4 plugin in gst-plugins-bad.  These enhancements make Gstreamer work much better on Solaris with Boomer (OpenSolaris build 115 and later), but also make it work better with stock OSSv4 from 4Front (verified by Dev Mazumdar at 4Front.)

Most of the changes involve general code cleanup and quality improvements, but I've also significantly revamped the way the audio controls are presented, so that gnome-volume-control presents a much clearer and more consistent picture of the controls available on an audio device.

This fixes incorrect handling of the MUTE option, and incorrect placement of some of the controls on gnome-volume-control.

It also significantly enhances the ability to localize GStreamer controls on OSSv4 systems.

Steps to reproduce:
1. Run gstreamer with gnome-volume-control using the OSSv4 plugin
2. Notice that some controls have improper mute values, are not localized, etc.

Actual results:

Expected results:

Does this happen every time?

Other information:
The detailed changes in behavior are too many to list here.  This is basically a major overhaul of the entire plugin.

Additionally Brian Cameron and I have worked on this, and I believe that Brian and I will be committing to long term sustaining, and working to move this out of -bad.

Finally, I want to note that Dev Mazumdar at 4Front is pleased with these changes -- they work better with stock OSSv4 than the previous code, so the code is an unqualified improvement.
Comment 1 Garrett D'Amore 2009-05-30 04:56:38 UTC
Created attachment 135597 [details] [review]
My patches for OSSv4 plugin

This patch has been in use at Sun Microsystems, and represents the code that we would like to see in place on official gstreamer for Solaris.
Comment 2 Tim-Philipp Müller 2009-06-02 22:47:12 UTC
Hi, thanks for the patch. Do you by any chance have a set of cumulative patches, like a git/mercurial branch or so? Or is it just this giant patch?

There seem to be quite a few cosmetic changes, and also broken indenting. It would be great if you could update your patch after running 'gst-indent *.c' on the changed files (required GNU indent to be installed). (At this point you'll probably also realise why there are those semicolons after those macros :))
Comment 3 Brian Cameron 2009-06-03 18:23:13 UTC
Created attachment 135901 [details] [review]
updated patch

Note that many of the changes in this patch are related to adding new flags to gst-plugins-base.  Refer:

Now that these changes are upstream, it would be good to support the new flags in the OSSv4 plugin.

I have attached an updated patch.  I believe this updated patch cleans up the code so it conforms to the style and removes some unnecessary changes.

Here is some detail on the changes:

1) oss4-audio.c is modified to simplify the fragsize computation.

2) oss4-mixer-slider.c is modified to support the new mixer-related flag and 
   also contains this change:

       if (mute) {
  +      /* make sure the current volume values get saved. */
  +      gst_oss4_mixer_slider_get_volume (s, s->volumes);
         volume = 0;
       } else {

  This change fixes a problem where the volumes weren't getting saved before
  muting for the first time.  Calling gst_oss4_mixer_slider_get_volume makes
  sure the volumes are cached before setting to 0.  Otherwise, when you mute
  and then unmute, the volume wouldn't get restored.

3) oss4-mixer.c contains improvements on how mixer labels are translated,
   the GST_OSS4_MIXER_WATCH_INTERVAL was changed from 500 to 150ms. the
   guess_master code was improved, and support for the new flags.

4) oss4-mixer-switch now supports the new mute-related flag.
Comment 4 Jan Schmidt 2009-06-08 20:33:40 UTC
I get some warnings compiling with this patch:

oss4-mixer.c: In function `gst_oss4_mixer_control_get_translated_name':
oss4-mixer.c:862: warning: 'num' might be used uninitialized in this function
oss4-mixer.c: At top level:
oss4-mixer.c:678: warning: 'gst_oss4_mixer_control_get_pretty_name' defined but not used

oss4-audio.c: In function `gst_oss4_audio_set_format':
oss4-audio.c:502: warning: 'fragsize' might be used uninitialized in this function

The fixes for the last 2 are pretty obvious, but it's less obvious how to fix the first.
Comment 5 Tim-Philipp Müller 2009-06-09 14:58:10 UTC
Btw, I'm not particularly in favour of reducing the poll interval of the notification thread from 500ms to 150ms in general. A patch that adjusts the interval dynamically (ie. wake up every 500ms or so by default, but more frequently if changes were detected recently) would be more acceptable imo. Even better would be if the OSS folks fixed this on their end so that we don't have to poll at all, but could just watch an fd or so for change notifications.

This would all be much easier if you provided separate patches for separate changes btw. (git add -i comes in handy here).
Comment 6 Brian Cameron 2009-06-09 20:09:29 UTC
I just talked with Garrett on the phone, and have some feedback about the two outstanding issues.

Tim-Phillipp: Garret agreed that keeping the poll interval at 500ms is 

Jan: if you look at the oss4-mixer.c code, in the gst_oss4_mixer_control_get_translated_name function you will notice that the
num variable is only used in a call to g_snprintf call that looks like this:

   g_snprintf(name, sizeof (name), fmtbuf, _(labels[i].label), num);

Note that fmtbuf by default is "%s", so the num argument is ignored in this case.  However when the code falls into the previous if-case where num is set, 
then fmtbuf is also changed to "%s %n".  So, in other words, the num variable in the g_snprintf call is only used if it falls into the if-block where num is set to a value.

To make the compiler happier, and avoid the warning, it would be okay to just initialize it to -1 at the beginning of the function.

Jan, can you go ahead and make these two minor changes (reset the poll back to 500ms and initialize the num variable to -1) before you commit?  Or would you like me to regenerate the patch with these two changes?


Comment 7 Jan Schmidt 2009-06-09 20:38:58 UTC
No worries, I'll fix it up in the morning. The Bad plugins are frozen for release at the moment - I'll commit it after the thaw.
Comment 8 Jan Schmidt 2009-07-16 09:23:05 UTC
After a reminder from Brian, I finally remembered to actually push the commit to master:

commit fac1dbab6e19d5a79a417864f496d2be86250bbb
Author: Garrett D'Amore <>
Date:   Wed Jun 10 19:21:21 2009 +0100

    oss4: Enhancements to the mixer and audio output
    Code cleanups, general improvements, support for the
    new mixer flags in latest gst-plugins-base.
    Fixes: #584252
    Patch By: Brian Cameron <>
    Patch By: Garrett D'Amore <>