Skip to content
This repository has been archived by the owner on Mar 11, 2019. It is now read-only.

portaudio.vapi #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

portaudio.vapi #7

wants to merge 2 commits into from

Conversation

Hikawa
Copy link
Contributor

@Hikawa Hikawa commented Sep 27, 2013

I tried to use portaudio.vapi and found next problems:

  1. on my fedora package is named potraudio-2.0
  2. there was no constructor for PortAudio.Stream.Parameters and I made it struct
  3. HostApiInfo and DeviceInfo must be unowned

@Hikawa Hikawa mentioned this pull request Sep 27, 2013
@nemequ
Copy link
Owner

nemequ commented Sep 27, 2013

  1. on my fedora package is named potraudio-2.0

Looks like that is the case upstream, and has been for 7 years or so, so renaming that seems reasonable.

  1. there was no constructor for PortAudio.Stream.Parameters and I made it struct

That seems reasonable, but why did you make them pointers when passed as arguments? In Vala, we try to only use pointers when there are no other options. Based on a quick look I don't think the parameters need to be anything special (only [SimpleType] structs are passed by value). Other options are making the arguments nullable, out, or ref.

  1. HostApiInfo and DeviceInfo must be unowned

Instead of making them static methods, why not instance methods ("get_info", perhaps) in HostApiIndex and DeviceIndex? If you really want to keep them where they are, something other than "get" for the name would be better ("from_index", perhaps?).

add portaudio test

In order to keep the size down we don't include test cases or examples in the vala-extra-vapis repository.

@Hikawa
Copy link
Contributor Author

Hikawa commented Sep 27, 2013

  1. I am not expert in vala but I think we need for sturct for auto constructor and parameter must be nullable. It is good if nullable struct will be translated to C pointer.
  2. instance methods ("get_info", perhaps) in HostApiIndex - it is not usual for C#/C++ programmers to create methods in int wrapper type (ruby style) but bug must be fixed anyways.
  3. add portaudio test - I don't mean to include file in current repo. Just worked sample.

@nemequ
Copy link
Owner

nemequ commented Sep 27, 2013

  1. I am not expert in vala but I think we need for sturct for auto constructor and parameter must be nullable. It is good if nullable struct will be translated to C pointer.

Yes, nullable struct will map to a pointer. So, assuming null is a valid value, the arguments to Stream.open should be "Stream.Parameters?", not "Stream.Parameters*". If you just want a pointer but it shouldn't be nullable then "Stream.Parameters" should do the trick. The only time Vala will pass something that isn't a pointer is if the type is annotated with [SimpleType], which Stream.Parameters is not.

  1. instance methods ("get_info", perhaps) in HostApiIndex - it is not usual for C#/C++ programmers to create methods in int wrapper type (ruby style) but bug must be fixed anyways.

Not sure what you're trying to say here. I'm not suggesting you create a wrapper. I'm suggesting something like:

public class DeviceIndex {
  [CCode (cname = "Pa_GetDeviceInfo")]
  public unowned DeviceInfo get_info ();
}

which would be my preference. Or, if you prefer:

public class DeviceInfo {
  [CCode (cname = "Pa_GetDeviceInfo")]
  public unowned DeviceInfo from_index (DeviceIndex index);
}

Or both. Methods named "get" are typically used for containers in order to retrieve a child,

@Hikawa
Copy link
Contributor Author

Hikawa commented Sep 28, 2013

we have "public struct DeviceIndex : int {}"

@nemequ
Copy link
Owner

nemequ commented Oct 5, 2013

So?

public struct DeviceIndex : int {
  [CCode (cname = "Pa_GetDeviceInfo")]
  public unowned DeviceInfo get_info ();
}

@Hikawa
Copy link
Contributor Author

Hikawa commented Oct 5, 2013

I know that function with this signature can be naturally translated to method of DeviceIndex but I preffer to consider DeviceIndex as a synonym of type int and think that Pa_GetDeviceInfo is something about DeviceInfo object taken from system by index. But OK, writting both is good.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants