[Bug 24385] Implement media in Haze

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Oct 23 12:21:28 CEST 2009


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


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Implement the Capabilities  |Implement media in Haze
                   |interface                   |




--- Comment #2 from Will Thompson <will.thompson at collabora.co.uk>  2009-10-23 03:21:27 PST ---
Retitling the bug since it's kind of an umbrella for this branch.

Up to f0aa818 and 1b75416 (media-buddy-caps-changed-signal and
media-ui-changed-signal):

Gabble does the:

  unsigned ready: 1;

crap for bitfielding booleans, but I really don't think it's worth bitfielding
these variables: the extra few words used by the channel and manager's private
structs are the least of your worries when making a/v calls. :)

(If you were feeling keen I guess you could write a patch for Gabble too :P)

I don't think Haze needs to support the RequestChannel() methods of creating
streamed media channels at all. This would simplify a bunch of code, because
you can require a handle, and not have to have the peer_in_rp stuff, and
simplify the property getters, etc.

In theory we've got D-Bus properties that replace all the Tp properties on
MediaSignalling, so Haze never needs to implement them. (Which simplifies the
code.

+          /*
+           * This isn't always true. We might need to have this happen on
+           * REJECT/HANGUP signals instead
+           */

That sounds like something that needs fixing :P.

+          "Streams can't be removed in this Haze protocol's calls" };

Would be nicer to include the protocol's name. This is just a debugging string,
but...

Why does haze_media_channel_remove_streams() dup a list of IDs before checking
if the method's even supported?

+          g_set_error (error, TP_ERRORS, TP_ERROR_INVALID_ARGUMENT,
+              "given media type %u is invalid", media_type);
+          return FALSE;

Technically this is the wrong error, because someone could add
Media_Type_Smell. From RequestStreams() in the spec:

    # Not Implemented

    A stream type given is not implemented by the connection manager. Since
    0.17.23, connection managers SHOULD raise this error in preference to
    InvalidArgument.

    (Connection managers can't know whether an unknown number is a valid stream
    type that was introduced in a later spec version.)

MediaStream's D-Bus properties are undrafted now, so Haze (and Gabble) should
just use the D-Bus properties mixin rather than manually implementing Get and
GetAll.

media_error_cb:

+  HazeMediaStream *stream = g_ptr_array_index (chan->priv->streams, 0);

Why're you picking the first one at random? And, if there are no streams this
will crash.

A bunch of patches add trailing whitespace.


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



More information about the telepathy-bugs mailing list