[Bug 46484] Add high-level Call bindings

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Mar 14 11:48:42 CET 2012


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

--- Comment #3 from George Kiagiadakis <kiagiadakis.george at gmail.com> 2012-03-14 03:48:42 PDT ---
(In reply to comment #1)
> - missing pretty headers for CallContent and CallStream, and the newly added
> public pending operations.
> 

CallContent/CallStream headers are there. You added them ;)
PendingCallContent was missing one, fixed now.

> - I guess audio/video content name are not that important API wise and may be
> ignored in some cases and be generated internally? So I believe we should have
> helper methods overloads in Account that don't take the content name as
> argument. For all cases.

The content name is considered significant, but not required, so I made it an
optional argument:

    PendingChannelRequest *ensureAudioCall(
            const QString &contactIdentifier,
            const QString &initialAudioContentName = QString(),
            const QDateTime &userActionTime = QDateTime::currentDateTime(),
            const QString &preferredHandler = QString(),
            const ChannelRequestHints &hints = ChannelRequestHints());

Do you think we really need extra overloads without this argument?


> +Features ChannelFactory::featuresForMediaCalls(const QVariantMap
> &additionalProps) const
> +{
> +    return featuresFor(ChannelClassSpec::audioCall(additionalProps));
> +}
> +
> +void ChannelFactory::addFeaturesForMediaCalls(const Features &features,
> +        const QVariantMap &additionalProps)
> +{
> +    addFeaturesFor(ChannelClassSpec::audioCall(additionalProps), features);
> +    addFeaturesFor(ChannelClassSpec::videoCall(additionalProps), features);
> +}
> +
> +void ChannelFactory::setConstructorForMediaCalls(const ConstructorConstPtr
> &ctor,
> +        const QVariantMap &additionalProps)
> +{
> +    setConstructorFor(ChannelClassSpec::audioCall(additionalProps), ctor);
> +    setConstructorFor(ChannelClassSpec::videoCall(additionalProps), ctor);
> +}
> +
> I guess you want a ChannelClassSpec::mediaCall and use it here instead?

I purposefully didn't add a mediCall() method there, as according to the spec a
Call1 channel that has neither InitialAudio nor InitialVideo is not valid. I
think the public API should only have things that users are safe to use.

Quoting the spec:
"The RequestableChannelClasses for Call1 channels can be:
[( Fixed = { ...ChannelType: ...Call1,
            ...TargetHandleType: Contact,
            ...InitialVideo: True
          },
  Allowed = [ ...InitialVideoName,
              ...InitialAudio,
              ...InitialAudioName
            ]
),
( Fixed = { ...ChannelType: ...Call1,
            ...TargetHandleType: Contact,
            ...InitialAudio: True
          },
  Allowed = [ ...InitialAudioName,
              ...InitialVideo,
              ...InitialVideoName
            ]
)]
Clients aren't allowed to make outgoing calls that have neither initial audio
nor initial video"


Btw, now that I look at it again, maybe I should do s/MediaCall/Call/g in the
relevant ChannelFactory and ChannelClassSpec methods? We have things like:

bool hasMediaCallInitialAudioFlag() const
bool hasStreamedMediaInitialAudioFlag() const

which is a bit confusing... If it reflected the channel name in the spec, maybe
it would be more obvious that the first method is for Call and the other is for
SM.

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