Created attachment 39237 [details] [review] [PATCH] StreamHandler: Add signal specifying the type of DTMF event to send Some protocols specify the type of DTMF event send and the payload type just at the time when the are sent. This signal should make these easier to implement.
Created attachment 39238 [details] [review] Updated spec patch
Created attachment 39241 [details] [review] Updated patch again
Review of attachment 39241 [details] [review]: 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>
Created attachment 39450 [details] [review] patch updated to satisfy Simon's comments Hopefully this should be good to go ?
Review of attachment 39450 [details] [review]: Thanks, this is much better. I'll apply it and turn the handle on a spec release.
Was fixed in 0.21.2, with codegen in telepathy-glib 0.13.2.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.