[vsipl++] Matlab IO Patch

[Stefan Seefeld]

Assem Salama wrote:

[...]

> +  template <int Dim>
> +  struct view_header
> +  {
> +    data_element header;
> +    data_element array_flags_header;
> +    char array_flags[8];
> +    data_element dim_header;
> +    int32_t dim[Dim + Dim%2];

It would be good to add a little comment with the rationale for
this 'Dim + Dim % 2' expression.

> +    data_element array_name_header;
> +  };
> +
> +  // some structures to helps determine if a type is single precision
> +  template <typename T>
> +  struct Is_single
> +  { static bool const value = false; };

By the way, wouldn't it make sense to add these Is_single helpers to
metaprogramming.hpp ?


> +
> +  template <>
> +  struct Is_single<float>
> +  { static bool const value = true; };
> +
> +  template <>
> +  struct Is_single<std::complex<float> >
> +  { static bool const value = true; };
> +
> +  // a generic reader that allows us to read a generic type and cast to another
> +  template<typename T1,typename T2>
> +  struct Generic_reader
> +  {
> +    // the read function
> +    template <typename T,
> +	      typename Block0>
> +    void read(std::istream& is,vsip::Matrix<T,Block0> m)
> +    {
> +      for(int i=0;i<m.size(1);i++) {
> +        for(int j=0;j<m.size(0);j++) {
> +          is.read(reinterpret_cast<char*>(&data),sizeof(data));
> +	  converted_data = data;
> +	  m.put(j,i,converted_data);
> +	}
> +      }
> +    }
> +
> +    T1 data;
> +    T2 converted_data;
> +  };

You define three distinct types here (T, T1, and T2). I can see the
need for two (if you are reading in a record of floats, but want to
create a block of double, say), what about the third ?
And a related question: what is the use of 'data' and 'converted_data'
outside the read() function ? Why do you use a struct, instead of
simply a function ? (For example:

template <typename T1, typename T2, typename BlockT>
inline void
read_matlab_binary(std::istream &is, Matrix<T2, BlockT> m)
{
   for(int i=0;i<m.size(1);i++)
     for(int j=0;j<m.size(0);j++)
     {
       T1 data;
       is.read(reinterpret_cast<char*>(&data),sizeof(data));
       m.put(j,i,T2(data));
     }
}


which would be used as

Matrix<double> m(rows, cols);
read_matlab_binary<float>(input, m);

[...]

> +  template <typename ViewT>
> +  struct Matlab_bin_formatter
> +  {
> +    Matlab_bin_formatter(ViewT v,std::string const& name) :
> +      v(v), view_name(name)  {}
> +
> +    ViewT v;
> +    std::string view_name;

What about 'view' and 'name' instead of 'v' and 'view_name' ?
Also, this whole struct could be moved into the 'matlab' namespace,
may be as 'Binary_formatter'...

> +
> +  };
> +
> +  struct Matlab_bin_hdr

...and this too, as 'Binary_header'.

Regards,
		Stefan