Bug 22589

This is good enough to not block merging the spec, but contains mistakes.

+            g_object_get (object, "immutable-streams", &immutable_streams, NULL);
+            tp_asv_set_boolean (props,
+                g_strdup_printf ("%s.%s",
+                    GABBLE_IFACE_CHANNEL_TYPE_STREAMED_MEDIA_FUTURE,
+                    "ImmutableStreams"), immutable_streams);

This works by mistake: it would leak the dup'd string if the hash table was actually tp_asv_new-compatible, but the table returned by tp_dbus_properties_mixin_make_properties_hash expects dup'd strings as keys too. We should document what tp_dbus_properties_mixin_make_properties_hash actually wants, and make this code use g_hash_table_insert explicitly.

Also, this doesn't need to do g_strdup_printf, it can just use g_strdup (G_I_C_T_S_M_F ".ImmutableStreams") thanks to C string concatenation.

   static TpDBusPropertiesMixinPropImpl streamed_media_props[] = {
       { "InitialAudio", "initial-audio", NULL },
       { "InitialVideo", "initial-video", NULL },
+      { "ImmutableStreams", "immutable-streams", NULL },
       { NULL }
   };

This is on the wrong interface (not FUTURE), although that problem will go away when the spec is merged.

+  param_spec = g_param_spec_boolean ("immutable-streams", "ImmutableStreams",
+      "Whether the set of streams on this channel are fixed once requested",
+      FALSE,
+      G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
+  g_object_class_install_property (object_class, PROP_INITIAL_VIDEO,
+      param_spec);

I think you mean IMMUTABLE_STREAMS, not INITIAL_VIDEO!

+    typeflags |= 32; /* FIXME: use the constant when it's in tp-glib */

Indeed. We shouldn't actually merge this until the spec lands in tp-glib, I think.

With telepathy-glib 0.7.37 out, this can be picked up again.

I've updated the branch, addressing the review comments:

(In reply to comment #1)
> This is good enough to not block merging the spec, but contains mistakes.
> 
> +            g_object_get (object, "immutable-streams", &immutable_streams,
> NULL);
> +            tp_asv_set_boolean (props,
> +                g_strdup_printf ("%s.%s",
> +                    GABBLE_IFACE_CHANNEL_TYPE_STREAMED_MEDIA_FUTURE,
> +                    "ImmutableStreams"), immutable_streams);
> 
> This works by mistake: it would leak the dup'd string if the hash table was
> actually tp_asv_new-compatible, but the table returned by
> tp_dbus_properties_mixin_make_properties_hash expects dup'd strings as keys
> too. We should document what tp_dbus_properties_mixin_make_properties_hash
> actually wants, and make this code use g_hash_table_insert explicitly.
> 
> Also, this doesn't need to do g_strdup_printf, it can just use g_strdup
> (G_I_C_T_S_M_F ".ImmutableStreams") thanks to C string concatenation.

Deleted all this code!

>    static TpDBusPropertiesMixinPropImpl streamed_media_props[] = {
>        { "InitialAudio", "initial-audio", NULL },
>        { "InitialVideo", "initial-video", NULL },
> +      { "ImmutableStreams", "immutable-streams", NULL },
>        { NULL }
>    };
> 
> This is on the wrong interface (not FUTURE), although that problem will go away
> when the spec is merged.

Those properties were all in the FUTURE when I wrote that branch. But the updated branch adds IS to the non-future, and (if we take the last patch) duplicates IA and IV in the non-future.

> +  param_spec = g_param_spec_boolean ("immutable-streams", "ImmutableStreams",
> +      "Whether the set of streams on this channel are fixed once requested",
> +      FALSE,
> +      G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
> +  g_object_class_install_property (object_class, PROP_INITIAL_VIDEO,
> +      param_spec);
> 
> I think you mean IMMUTABLE_STREAMS, not INITIAL_VIDEO!

Good catch!

> +    typeflags |= 32; /* FIXME: use the constant when it's in tp-glib */
> 
> Indeed. We shouldn't actually merge this until the spec lands in tp-glib, I
> think.

Fixed.

Yay! Ship it!

Did we already implement this in 0.9.x? If not, please clone this (I expect it'll need a non-trivial merge).

(In reply to comment #4)
> Yay! Ship it!

Shipped. I'll clone the bug for 0.9, and attach the 0.9 branch.