[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 #7 from Sjoerd Simons <sjoerd at luon.net> 2010-12-14 11:19:02 PST ---
Mostly at notes to myself what i fixed in my branch thusfar :)

(In reply to comment #5)
> 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.

fixed
> 
> In DescriptionOffer:
> 
>        <tp:member name="Codec_Offer" type="o">
> 
> you probably want to rename this.

fixed

> -          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")

fixed

> +        <p>A map from contact handles to descriptions give supported by that
> 
> s/give // I think?
>

fixed

> +          The object path of the new description. This replaces any previous
> +          description.
> 
> If '/' means no description, please say so here too.

done

> 
> +    <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.

FIXME

> In DescriptionOffer:
> 
> +            >RemoteContact</tp:dbus-ref> and
> +            a of the Descriptions properties.
> 
> and a what? "and a mapping containing the _Description_'s properties",
> presumably.

fixed

> 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.

fixed, FIXME should require to always go via DescriptionOfferDone though

> > 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

fixed

> > 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?

fixed

> 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?

fixed

> > 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?

FIXME clarify what this mean

> 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
> :-)

FIXME

-- 
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.