Created attachment 329096 [details] [review]
audio: Fix gst_audio_channel_positions_to_mask annotations

Channels direction=out, I am not sure how a int value (not a pointer) could be out? :)

But but but ... the annotation looks correct, doesn't it? So it does seem like the parser is broken or makes some wrong assumptions, no?

I think we were missing `(out caller-allocates)` but appart from that I believe this is correct now, just opening the bug here so you check if I am right, otherwise we should move it to gobject-introspection I think

Yeah, that part (caller-allocates) is obviously correct imho, I just think it should not be required to mark the 'channels' gint explicitly as (in). If you remove just the (in) does it work fine?

Fwiw, I'm not getting the direction=out here with g-i 1.48.0.

OK, that is weird I am also using g-i 1.48 (I am inside flatpak sandbox so this is what is being used exactly: https://github.com/GNOME/gnome-sdk-images/blob/gnome-3-20/org.gnome.Sdk.json.in#L106)

What does the .gir file look like exactly? (with and without that patch?)

Are you able to do something like:

  $ python3 -c "from gi.repository import Gst; Gst.init(None); from gi.repository import GstAudio; GstAudio.audio_channel_positions_to_mask([GstAudio.AudioChannelPosition.MONO], True)"

also what does that return:

  $python -c "from gi.repository import Gst; Gst.init(None); from gi.repository import GstAudio; print(GstAudio.audio_channel_positions_to_mask.__doc__)"

Thanks.

Also - am I confused or does your patch patch _from_mask() while you talk about _to_mask() everywhere? :)

> Also - am I confused or does your patch patch _from_mask() while you talk about _to_mask() everywhere? :)

Sorry, none works actually, and my patch is about from_mask indeed.

That looks like a problem in the parser indeed, but "channels" is also not marked as "out" for me. Also adding (caller-allocates) does not make any difference in the generated gir here, it's always the following, which is obviously wrong in a few different ways:

      <parameters>
        <parameter name="channels" transfer-ownership="none">
          <doc xml:space="preserve">The number of channels</doc>
          <type name="gint" c:type="gint"/>
        </parameter>
        <parameter name="channel_mask" transfer-ownership="none">
          <doc xml:space="preserve">The input channel_mask</doc>
          <type name="guint64" c:type="guint64"/>
        </parameter>
        <parameter name="position" transfer-ownership="none">
          <doc xml:space="preserve">The
  %GstAudioChannelPosition&lt;!-- --&gt;s</doc>
          <array length="0"
                 zero-terminated="0"
                 c:type="GstAudioChannelPosition*">
            <type name="AudioChannelPosition"
                  c:type="GstAudioChannelPosition"/>
          </array>
        </parameter>
      </parameters>

This was pushed:

commit 8bbf67c37d055d65fff4a18e854de8fda72d76d9
Author: Thibault Saunier <tsaunier@gnome.org>
Date:   Thu Dec 15 10:57:14 2016 -0300

    audio: Fix introspection annotation
    
    In gst_audio_check_valid_channel_positions the mask
    is an out parameter.
    
    And minor conversion from a print to a GST_ERROR.


I don't know if it fixes anything, or what else there is to do with this bug?

If the parser is broken we should file an issue against g-i I suppose?

(In reply to Tim-Philipp Müller from comment #10)
> This was pushed:
> 
> commit 8bbf67c37d055d65fff4a18e854de8fda72d76d9
> Author: Thibault Saunier <tsaunier@gnome.org>
> Date:   Thu Dec 15 10:57:14 2016 -0300
> 
>     audio: Fix introspection annotation
>     
>     In gst_audio_check_valid_channel_positions the mask
>     is an out parameter.
>     
>     And minor conversion from a print to a GST_ERROR.
> 
> 
> I don't know if it fixes anything, or what else there is to do with this bug?
> 
> If the parser is broken we should file an issue against g-i I suppose?

Well, those are 2 different function, the patch I just pushed surely fixed an issue for that function (it could not work before).

I guess this bug is not related and we should just keep it opened.

Ah right, sorry. Still, any idea what to do with this though?

OK, it seems to work here now, let's close. Sorry for the noise :)