Outgoing DTMF / dialpad support

[David Woodhouse]

On Mon, 2013-10-14 at 13:43 +0100, David Woodhouse wrote:
>  - https://developer.pidgin.im/ticket/12617
>    (Patch to add out-of-band DTMF support)

This patch adds a reference to 'PurpleMedia' in prpl.h:

+	gboolean (*media_send_dtmf)(PurpleMedia *media, gchar dtmf,
+				    guint8 volume, guint8 duration);

This works fine for the build of pidgin itself, but it causes the SIPE
protocol plugin to fail to detect that pidgin has been built with voice
and video support. In config.log the failure is as follows:

In file included from /opt/purple/include/libpurple/media/../buddyicon.h:39:0,
                 from /opt/purple/include/libpurple/media/../blist.h:110,
                 from /opt/purple/include/libpurple/media/../status.h:133,
                 from /opt/purple/include/libpurple/media/../connection.h:151,
                 from /opt/purple/include/libpurple/media/../account.h:47,
                 from /opt/purple/include/libpurple/media/../util.h:44,
                 from /opt/purple/include/libpurple/media/codec.h:35,
                 from /opt/purple/include/libpurple/media.h:31,
                 from conftest.c:28:
/opt/purple/include/libpurple/media/../prpl.h:657:30: error: unknown type name 'PurpleMedia'
  gboolean (*media_send_dtmf)(PurpleMedia *media, gchar dtmf,


I see two options for fixing this. Which is better? ...

The first is to declare that the SIPE plugin shouldn't be using
<media.h> in its build test; it should be using <prpl.h> instead. We'd
apply this patch to it (and to any other similarly affected plugin):

diff --git a/configure.ac b/configure.ac
index 3141191..9a8e819 100644
--- a/configure.ac
+++ b/configure.ac
@@ -378,7 +378,7 @@ system defaults.
 			 [AC_MSG_CHECKING(for purple voice and video support)
 			  AC_RUN_IFELSE(
 				[AC_LANG_PROGRAM([[
-#include <media.h>
+#include <prpl.h>
 					]],
 					[[return (purple_media_get_type() == G_TYPE_NONE ? 1 : 0);]]
 			  	)],


The other option is to move the forward declaration of PurpleMedia up to
the top of media.h so that it's included before prpl.h when media.h is
"abused" in this way. If indeed it is considered abuse.

diff -u b/libpurple/media.h b/libpurple/media.h
--- b/libpurple/media.h
+++ b/libpurple/media.h
@@ -27,6 +27,9 @@
 #ifndef _PURPLE_MEDIA_H_
 #define _PURPLE_MEDIA_H_
 
+/** An opaque structure representing a media call. */
+typedef struct _PurpleMedia PurpleMedia;
+
 #include "media/candidate.h"
 #include "media/codec.h"
 #include "media/enum-types.h"
@@ -43,9 +46,6 @@
 #define PURPLE_IS_MEDIA_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), PURPLE_TYPE_MEDIA))
 #define PURPLE_MEDIA_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS((obj), PURPLE_TYPE_MEDIA, PurpleMediaClass))
 
-/** An opaque structure representing a media call. */
-typedef struct _PurpleMedia PurpleMedia;
-
 #include "signals.h"
 #include "util.h"
 


I'm going to go with the latter option and submit a new patch for ticket
12617 for now, unless anyone feels strongly. I just didn't want to sneak
that change in without comment, since I'm not particularly familiar with
the conventions for how these headers are used.

Other than that problem, I think Jan has already fixed all the other
outstanding criticisms with this patch? (Thanks)

Apart from the request that the local user be able to hear the DTMF too.
But that's non-trivial, and is a feature which "real" phones often don't
offer either, so I'd prefer not to let that block the inclusion of the
patch.

-- 
dwmw2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://pidgin.im/pipermail/devel/attachments/20131114/ec6c87f7/attachment.bin>