[Bug 22589] Implement Immutable_Streams capability flag, and ImmutableStreams channel property

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=22589


Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch




--- Comment #3 from Will Thompson <will.thompson at collabora.co.uk>  2009-10-05 04:55:54 PST ---
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.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.