[Bug 30662] StreamHandler: Add method to send DTMF event of a specified type

[bugzilla-daemon at freedesktop.org]

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

--- Comment #3 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-10-14 03:57:38 PDT ---
Review of attachment 39241:
 --> (https://bugs.freedesktop.org/review?bug=30662&attachment=39241)

This API suggests that we might need these rather unpleasant semantics:

StartTelephonyEventFull(Event='#', Type=Auto, PT=42)
    => Choose either Named or Sound. If you choose Named, use PT 42.
       If you choose Sound, don't.

Are we going to need that?

If at all possible, I'd prefer this as two or more signals with much clearer
semantics:

StartNamedTelephonyEventWithPayloadType(Event='#', PT=42)

StartSoundTelephonyEvent(Event='#')

in which case we don't really need Auto (because that's
just StartTelephonyEvent).

::: spec/Media_Stream_Handler.xml
@@ +528,3 @@
+        <tp:docstring>Automatically chose the type of event to
send</tp:docstring>
+      </tp:enumvalue>
+      <tp:enumvalue suffix="RTP_RFC4733" value="1">

This is asking for misspelling (and indeed you spelled it as 473 below). Is
there a better thing we could call it?

The RFC talks about "named telephony events", so perhaps
Media_Stream_Telephony_Event_Type_Named would be better?

I'd tend to say "audio/telephone-event RTP packets" since Sound is also sent in
(a different flavour of) RTP packets.

@@ +539,3 @@
+      <arg name="Event" type="y">
+        <tp:docstring>
+          A telephony event code as defined by RFC 4733.

We have a tp:type for this, <arg name="Event" tp:type="DTMF_Event" type="y">.
With that added, you don't need to reference the RFC.

@@ +544,3 @@
+      <arg name="Type" type="u" tp:type="Media_Stream_Telephony_Event_Type">
+        <tp:docstring>
+          The type of event to send.

Perhaps add "If this is Media_Stream_Telephony_Event_Type_Auto, this signal is
exactly equivalent to StartTelephonyEvent"?

@@ +549,3 @@
+      <arg name="Codec_ID" type="u">
+        <tp:docstring>
+          The payload type to sent events of Type "RTP_RFC473".

Can this ever be significant for events of type Sound?

If the type is Auto, does it mean "send this by either Sound or Named, but if
you choose Named, use the given Codec_ID"?

Is there a reasonable dummy value to put here in the Auto case, if you don't
know or don't care?

@@ +555,3 @@
+        Request that a telephony event (as defined by RFC 4733) is transmitted
+        over this stream until StopTelephonyEvent is called. This differs from
+        StartTelephonyEvent in that you can specify how to send the event.

Needs rationale similar to the commit message:

<tp:rationale>
  StartTelephonyEvent is not sufficient for protocols in which the payload type
or event type is not known until the telephony event is sent.
</tp:rationale>

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