Re: gsoc2013 octave audio system

[Jordi Gutiérrez Hermoso]

On 18 April 2013 08:39, Vytautas Jancauskas <address@hidden> wrote:
> The code is here:
> https://bitbucket.org/bucket_brigade/octave-sound/src/24bf96f1847d9d9d1f4abe1f1401bba750d45f3b/src/audiodevinfo.cc?at=default

This looks like a good initial implementation. A few remarks follow.

You should follow the GNU coding standards:

    http://www.gnu.org/prep/standards/standards.html

For you, this mostly means: 80-column code, two-space indents, curly
brackets on their own line with their own indentation, space between
opening round bracket and function call. In Octave, since we need to
distinguish indexing operations from function calls, there is no space
before the round bracket when indexing. For example,

    sin (pi)

but

    A = [1 2; 3 4]
    A(1)

We keep this bit of style consistent between the m-files and C++,
since in the latter, we define Array::operator() for indexing.

While we do use goto sometimes, we do not use it for simply erroring
out. Instead, replace "goto error" with "return retval;" and define
retval at the top of your function as a generic octave_value. Of
course, you should also use the error () function to state what the
error is.

You should read more of our source code so you can get a feel for our
style.

There has been an API change in portaudio. The function
Pa_GetDeviceCount used to be called Pa_CountDevices. You will need to
write an autoconf check for this. If you need help writing this test,
we can offer assistance.

As to your actual question about how to use Octave classes, the
problem is that you're trying to create a struct array out of
octave_value_list, and this is coercing the type into the
much-misunderstood comma-separated list (cs-list), and cs-lists can't
be assigned to variables, which is why Octave has no function for
displaying cs-lists inside a struct. This is what the error message is
telling you.

Thus, instead of octave_value_list for the input and output variables (which
I've renamed inputdev and outputdev respectively), use octave_map. To
populate the maps, create Cells and use octave_map::setfield, sort of
like this in m-files:

    struct_array = struct("greeting", {"hi", "labas"})

To create the cells, which are arrays of fixed size like most other
Octave types, I had to modify this to loop twice through the devices
counting each, and then use the count to pre-allocate the cells.
Avoiding reallocations isn't so important here since num_devices is
likely to be very small, but it is an important optimisation principle
in general when working with Octave code.

Below is how we would write your code in Octave style. I made a few
other minor stylistic modifications:

   * under_scores instead of StudlyCaps

   * declaration and definition of variables in the same line when
     possible

   * a few more newlines

   * octave_map::setfield instead of octave_map::assign, I just prefer
     the more descriptive "setfield" name.

HTH,
- Jordi G. H.

--------------------------------------------------------------------

#include <octave/oct.h>
#include <octave/ov-struct.h>
#include <portaudio.h>

DEFUN_DLD (audiodevinfo, args, ,
           "Return information about available audio devices.")
{
  octave_value retval;

  PaError err = Pa_Initialize ();
  if (err != paNoError)
    {
      error ("audiodevinfo: cannot initialize audio device");
      return retval;
    }

  int num_devices = Pa_GetDeviceCount ();
  if (num_devices < 0)
    {
      error ("audiodevinfo: no audio device found");
      return retval;
    }


  octave_idx_type numinput = 0, numoutput = 0;
  for (int i = 0; i < num_devices; i++)
    {
      const PaDeviceInfo* device_info  = Pa_GetDeviceInfo (i);

      if (device_info->maxInputChannels != 0)
          numinput++;

      if (device_info->maxOutputChannels != 0)
          numoutput++;
    }

  Cell input_name (dim_vector (numinput, 1)),
    input_id (dim_vector (numinput, 1)),
    input_driver_version (dim_vector (numinput, 1)),

    output_name (dim_vector (numoutput, 1)),
    output_driver_version (dim_vector (numoutput, 1)),
    output_id (dim_vector (numoutput, 1));

  octave_idx_type idx_i = 0, idx_o = 0;
  for (int i = 0; i < num_devices; i++)
    {
      const PaDeviceInfo* device_info  = Pa_GetDeviceInfo (i);

      if (device_info->maxInputChannels != 0)
        {
          input_name(idx_i) =  device_info -> name;
          input_driver_version(idx_i) =  "NA";
          input_id(idx_i) = i;
          idx_i++;
        }

      if (device_info->maxOutputChannels != 0)
        {
          output_name(idx_o) =  device_info -> name;
          output_driver_version(idx_o) =  "NA";
          output_id(idx_o) =  i;
          idx_o++;
        }
    }

  octave_map inputdev, outputdev;
  inputdev.setfield ("Name", input_name);
  inputdev.setfield ("DriverVersion", input_driver_version);
  inputdev.setfield ("ID", input_id);

  outputdev.setfield ("Name", output_name);
  outputdev.setfield ("DriverVersion", output_driver_version);
  outputdev.setfield ("ID", output_id);

  octave_scalar_map devinfo;
  devinfo.setfield ("input", inputdev);
  devinfo.setfield ("output", outputdev);

  retval = devinfo;
  return retval;
}