[Bug 31274] Call: NewCodecOffer signal should contain the properties of the optional CodecOffer interfaces

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=31274

--- Comment #5 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-12-13 03:48:59 PST ---
General comment: rationales and hyperlinks would be useful.

+    <tp:mapping name="ContactDescriptionPropertiesMap">

C_D_P_M please, otherwise it'll come out as
TP_HASH_TYPE_CONTACTDESCRIPTIONPROPERTIESMAP in codegen. Likewise for
DescriptionProperties, DescriptionOffer.

We usually use Ugly_Case for <arg> and <tp:member> names, too.

In DescriptionOffer:

       <tp:member name="Codec_Offer" type="o">

you probably want to rename this.


-          The contact handle that this codec offer applies to.
+          The contact handle that this description applies.

"... applies to" (or, "the contact handle to which this description applies")

+        <p>A map from contact handles to descriptions give supported by that

s/give // I think?

+          The object path of the new description. This replaces any previous
+          description.

If '/' means no description, please say so here too.

+    <signal name="LocalDescriptionsChanged"

In practice, do several local descriptions really change atomically? If not,
it'd be a lot simpler to have a LocalDescriptionChanged (which implies Added if
necessary) and a ...Removed. Likewise for RemoteDescriptionsChanged.

In DescriptionOffer:

+            >RemoteContact</tp:dbus-ref> and
+            a of the Descriptions properties.

and a what? "and a mapping containing the _Description_'s properties",
presumably.

The semantics of NewDescriptionOffer look as though it ought to be called
DescriptionOfferChanged - it seems astonishing to have a NewFoo signal when a
Foo is deleted.

> True if this offer contains no information from the remote side. Iotw, the
> Accept response solely depends on the capabilities and preferences of the
> local side. In most protocols this property will be True for the initial
> DescriptionOffer on an outgoing call. 

side: in other words, the

> This propery will not be part of the DescriptionProperties used to
> describe this object.

"property"

I think it might make more sense if the sense of EmptyDescription was reversed:
"RemoteDescriptionReceived" or something?

The semantics of Description_Properties (with EmptyDescription implicit from
the presence/absence of RemoteCodecs even if empty) seem rather odd. I assume
that the rationale here is that it lets us distinguish between "this is an
empty description" and "this is a non-empty description with an empty list of
remote codecs", which we couldn't previously do?

> The contact handle that this codec offer applies to or 0 a description
> that will apply globally.

"0 for a description that"

"... that will apply to all remote contacts in the _Call_" perhaps?

Department of namespacing: some of the type names seem pretty vague. I think
it's OK for the Content.Media interface to just have "Description" in its
property/signal names, because in the context of a Call, the prevailing meaning
for "description" is (perhaps) the SDP one.

However, types are a flat global namespace (in our codegen) and if I say
"description" with only Telepathy as context, I'd expect it to be some sort of
human-readable blurb about a chatroom or something. Perhaps the types could say
Media_, so we get Media_Description_Properties:a{sv},
Contact_Media_Description[_Properties]_Map:a{ua{sv}} and
Media_Description_Offer:(oua{sv}), or something?

I'd be very tempted to rename Description_Properties to Description_Description
:-)

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