[Bug 34985] rtcp-fb and rtp-hdrext support in spec and gabble

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Mar 11 21:37:56 CET 2011


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

--- Comment #5 from Olivier Crête <olivier.crete at ocrete.ca> 2011-03-11 12:37:55 PST ---
I fixed most of your comments and updated the branch .. except those:

(In reply to comment #3)
> Gabble:
> +static gboolean
> +content_has_cap (GabbleJingleContent *content, const gchar *cap)
> +{
> +  GabblePresence *presence = gabble_presence_cache_get (
> +      content->conn->presence_cache, content->session->peer);
> +
> +  return gabble_presence_resource_has_caps (presence,
> +      gabble_jingle_session_get_peer_resource (content->session),
> +      gabble_capability_set_predicate_has, cap);
> +}
> +
> 
> This should check whether 'presence' is NULL. (IIRC gabble_presence_cache_get()
> can return NULL.) In principle I guess this could happen if a contact becomes
> invisible mid-call, for instance.

I'm really not sure what do to about that ? Should I ignore the caps when
parsing the description ? Should I cache them when we start a call? But what
happens if I get a call from invisible? Should I cache them at the start if we
initiate a call and guess them from the incoming description if we receive one
?

> I've been trying to avoid nitpicking indentation (and if (foo) vs if (foo !=
> NULL) and other style trivia)

For the (foo) vs (foo != NULL), I though the TP convention was to only use if
(foo) is foo was a boolean and I tried to follow that everywhere

> +static gboolean
> +jingle_feedback_message_compare (const JingleFeedbackMessage *fb1,
> +    const JingleFeedbackMessage *fb2)
> +{
> +  if (!g_ascii_strcasecmp (fb1->type, fb2->type) &&
> +      !g_ascii_strcasecmp (fb1->subtype, fb2->subtype))
> +    return 0;
> +  else
> +    return 1;
> +}
> 
> FALSE and TRUE please.

It's not a gboolean, its 0, 1 or -1 for equal, larger or smaller..

I guess I could do a = g_ascii_strcasecmp (fb1->type, fb2->type); b =
g_ascii_strcasecmp (fb1->subtype, fb2->subtype)); return a ? a : b; if I cared
about the ordering

> I'm balking at jingle_media_description_simplify() — I will come back to it. It
> seems excessively complicated.

Added a bunch of comments to make it clearer what it's supposed to do.

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


More information about the telepathy-bugs mailing list