Re: gsoc2013 octave audio system

[Vytautas Jancauskas]

On Thu, Apr 18, 2013 at 7:34 PM, Jordi Gutiérrez Hermoso
<address@hidden> wrote:
> 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;
> }


Thanks, I updated the code with these suggestions.
How come when I run audiodevinfo(1), args(0).is_integer_type() returns
0? what check do I use to see if the argument is an integer?