Ticket #393 (new patch)

Opened 3 years ago

Last modified 7 months ago

Add receive Handwritten messages, Winks and Voice Clips to MSN

Reported by: notak Owned by: khc
Milestone: Patches Needing Improvement Component: MSN
Version: 2.0 Keywords: msn handwritten wink
Cc: Universe_JDJ, IceWil, merihakar, ferk

Description (last modified by rlaager) (diff)

Originally SF Patch 1547654

Note - this patch is against the MSNP14 branch.

The official MSN client has the ability to send small handwritten images as .gif files. Currently Gaim reports these as being disabled and the client will not send them. This patch:

  1. Sets the enabled flag so that the official client can send them
  2. Gets the .gif data from the server
  3. Displays the .gif in the conversation window using the custom emoticon code.

The official MSN client has the ability to send small voice clips. Currently Gaim reports these as being disabled and the client will not send them. This patch creates an account-specific option to:

  1. Set the enabled flag so that the official client can send them
  2. Get the .wav data from the server and stores it as a temp file
  3. Play the temp file using the gaim sound subsystem.

NOTE: Being MS, these use a windows-specific codec which only Windows is likely to have, so in non-win32 builds the option defaults to off, and a warning is added to the text to say a specialized codec is required

The official MSN client has the ability to send 'winks', which are large annoying flash animations that explode all over the conversation window in a most distasteful manner. For some reason people like these. This patch creates an account-specific option to:

  1. Get the .cab file containing the wink, and extract it to a temp file using libmspack
  2. Display the picture of the wink (which is supplied as part of the .cab) using the custom emoticon code
  3. Display a link to the temp file of the animation so that anybody who is so inclined can click it and watch it do its stuff in their default browser
  4. Play the temp file using the gaim sound subsystem.

NOTE: libmspack is required to extract the .cab files. Most systems don't have this (it tends to be statically built in to applications like Wine and cabextract). Configure checks for it and doesn't bother to compile in the wink functionality if you don't have it. A makefile is provided here for win32.

Attachments

libmspack.Makefile.mingw (0.7 kB) - added by notak 3 years ago.
Makefile.mingw for compiling libmspack on windows using the Pidgin build environment (the libmspack project default is Visual C)
backtrace.txt (19.3 kB) - added by notak 3 years ago.
Blows up doing a gfree in got_voice_clip
voice_clip.wav (3.8 kB) - added by notak 3 years ago.
Voice clip as generated by MSN
msn_extras.diff (26.6 kB) - added by notak 3 years ago.
patch against msnp14 branch. (v3 - displays voice clip as an audio:// uri and plays those back when clicked)
handwrittenmsgs.patch (9.8 kB) - added by galt 2 years ago.
The patch for 2.3.1 to enable receiving handwritten messages.
handwrittenmsgs.2.patch (9.5 kB) - added by galt 2 years ago.
Took out a line about voice clips.

Change History

Changed 3 years ago by notak

Makefile.mingw for compiling libmspack on windows using the Pidgin build environment (the libmspack project default is Visual C)

  Changed 3 years ago by notak

What rocks: - This patch builds against the Pidgin MSNP14 branch just fine for me - Handwritten pictures sent from conversations get received and displayed - The old patch had preference checks that were just plain wrong, and the latest patch doesn't

What sucks: - Nobody is on MSN, or they don't have a microphone or the journal viewer software installed, so I haven't been able to test that the following still work: - Handwritten messages from chats (they get sent completely differently - go figure) - Voice clips - Winks (I'm fairly certain these are okay - we're certainly fetching them but I had the prefs check backwards so I wasn't actually attempting to show them) - I no longer use win32, and even if I was willing to reboot cygwin has gone totally fubar on me, so I don't know if the pidgin changes to the win build stuff are correct or not

What I'd like to be different: - The voice clips just play once and then disappear into the ether. MSN client provides a handy link in the conversation window to play them again. We don't have the code to do that, and when somebody suggested it for file transfers the other day it went down very badly indeed. It makes more sense here since the voice clip is obviously part of the conversation, and would probably not be that difficult since you'd just put a hook into the URL handler code to handle a protocol like voiceclip:// internally in pidgin instead of going to a browser. I fear the wrath of various developers if it was attempted however, and I'm pretty short of time to implement stuff right now.

  Changed 3 years ago by notak

Now tested voice clips (they work, except that I still can't find anything that'll play them in Linux) and winks (they work), so only handwritten from chats remaining

  Changed 3 years ago by lschiere

  • type changed from enhancement to patch

  Changed 3 years ago by rlaager

  • description modified (diff)

notak: Could you attach a copy of a voice clip sound file here and I'll see if I can figure out what it is and make it play under Linux?

Your voiceclip URL hack is interesting. I did the same for a plugin I wrote one time. ;)

  Changed 3 years ago by notak

I suspect the only voice clip I have to hand is of a rather personal nature ;-) - I'll see if I can get hold of another one. In the meantime, it's a .wav container with audio format of 0x28E. Mplayer doesn't want to know, and if I remember rightly I have had a go at changing the audio format in the header at least to raw without any success. Thanks for reformatting the description btw.

Changed 3 years ago by notak

Blows up doing a gfree in got_voice_clip

Changed 3 years ago by notak

Voice clip as generated by MSN

  Changed 3 years ago by notak

Managed to get a voice clip for your trying-to-figure-out pleasure.

Unfortunately it coredumped whilst doing a g_free afterwards. I can't see anything wrong with the code, and it worked just fine yesterday - the only difference I can see being that I was running the debug window rather than debugging to console yesterday. Any chance someone could have a quick glance at got_voice_clip to see if I'm doing something stupid?

follow-up: ↓ 8   Changed 3 years ago by rlaager

khc: You talked about maybe merging some patches. With this, I'm a little concerned (having not looked at the patch, though) about undo complexity (for something so rarely used) in the wink stuff.

I'm also not a fan of the idea of voice clips that just play. We should present a message with a link and have a special URL that triggers it to be played. This would also solve the replayability issue.

in reply to: ↑ 7   Changed 3 years ago by notak

I reckon propagating from im.pidgin.pidgin now will make undo on new patches easier (although nexus.c is the only conflicted file). The functionality (and therefore almost all of the code) can be completely disabled by removing the options in msn.c, so disabling rather than removal is an easy option. The only sligtly contentious area of the patch is the libmspack requirement for winks, and since that's ifdeffed the configure scripts can easily be changed to have it disabled by default rather than just when configure can't find it.

I agree about the voice clips though - I'll try to get to coding the URL stuff soon. If you want a voice-free patch in the meantime commenting out the 'voice-clips' option in msn.c would make it invisible unless you manually hacked the accounts.xml.

  Changed 3 years ago by khc

  • owner set to khc
  • status changed from new to assigned

okay, I was never CC'ed on this so I missed the discussion.

What is libmspack? apt-cache search mspack is not returning any result on ubuntu.

I spent too much time playing with monotone tonight, hopefully I will get to this tomorrow night.

follow-up: ↓ 11   Changed 3 years ago by rlaager

Yes, you're doing: g_free(f); where f is a FILE *. That's wrong. You just fclose() them when you're done, which you're already doing in that case.

in reply to: ↑ 10   Changed 3 years ago by notak

Yes, you're doing: g_free(f); where f is a FILE *.

Excellent, I'll fix that then.

What is libmspack? apt-cache search mspack is not returning any result on ubuntu.

http://www.kyz.uklinux.net/libmspack/ It's statically linked within cabextract and wine so it doesn't get deployed in most dists as a separate target. The only way I can figure to do this without the awkward dep is:

  • include the library in our tree (waste of space - it's bigger than it needs to be)
  • hack the required code straight in (It's only ~200 lines, but I think that would contravene their license)
  • exec cabextract if that's there, which is probably uglier than the dep.

My feeling is that it's easy to package for windows, reasonably easy for people who are building themselves to satisfy the dep if they want it, and the #ifdefs mean that binary builds will just not contain the code at all (unless any of the distros get bored and decide to package mspack I guess). Don't know how that plays with anybody else

On the subject of voiceclip: uris. I guess it could be done in half a dozen lines (check for URI starting voiceclip:, grab following filename, purple_sound_play_file it). Is it likely to be acceptable to just put this directly into purple_handle_uri before the call to ui_ops?

  Changed 3 years ago by rlaager

Well, I'd do it as some sort of audio:// URL. I'm not sure if you should catch it in libpurple or in the UI.

  Changed 3 years ago by ms

Regarding the codec; it is the MSN Messenger Audio Codec (Siren 7/G722.1).

ACM Format 0x028e MSN Messenger Audio codec msacm.siren 0x00200000 quartz.dll --> sirenacm.dll

Full details here: http://wiki.multimedia.cx/index.php?title=Siren

Decode/Encoding Code: https://svn.sourceforge.net/svnroot/amsn/trunk/amsn/utils/tcl_siren/src/

Infact, changing the WAVE format to 0x0112 makes the voice_clip atleast not play as pure noise (except the noise you get due to the fileformat differences described in the wiki).

Changed 3 years ago by notak

patch against msnp14 branch. (v3 - displays voice clip as an audio:// uri and plays those back when clicked)

  Changed 3 years ago by notak

patched changed so that a link is incuded in the convo window with a URI of the form audio://PATH_TO_TMP_FILE. purple_notify_uri then handles links of type 'audio://' by playing them using purple_play_sound, and carries on passing any other links up to the UI handler as before.

  Changed 3 years ago by rlaager

IMPORTANT NOTE: Since there's been talk of merging the MSNP14 branch soon, let's not commit this directly there, but to a branch of that branch instead.

  Changed 3 years ago by khc

I've committed this patch to the branch im.pidgin.cpw.khc.msnp14.ticket393. There are several issues that need to be fixed before I would merge it in the regular msnp14 branch.

1) your change in libpurple/protocols/msn/slpcall.c has a lot of magical stuff. I can't find documentation on msnpiki about the handwriting stuff so I can't check if there's a clearer way to write it 2) do we really need a protocol option for audio clips? people who don't want to hear it can just not click on the link anyway. Will the audio files be big enough for this to be a concern? 3) similar to 1), in msn_handwritten_msg() what is this +7 and -7?

  Changed 2 years ago by rlaager

notak: Have you taken a look at this recently? If these outstanding issues can be corrected, we can look at getting this merged...

  Changed 2 years ago by seanegan

  • component changed from libpurple to MSN

follow-up: ↓ 21   Changed 2 years ago by notak

Tried to get this resurrected the other day when I finally updated to the latest P14 code, and I couldn't even get it to comile :-(. If I can't figure that much out I'm tempted to remove the winks anyway. As a side observation, these are delivered dreadfully slowly with direct connections:
The audio stuff is basically quite cleanly implemented. My main issue is that in linux it isn't going to be too useful until the relevant codec gets included into normal codec packs. Until then I think the option is useful, since it will reject the attempt to send an audio clip at the other end, and stop the sender getting the impression that the receiver has received the message, so I'm quite keen on retaining the protocol option.
The handwritten stuff is unfortunately pretty horribly hacked together by MSN - implementing them in multi-user chats is fairly simple as it just involves implementing the multi-part messaging code. Implementing them in chats however involves dealing with the fact that they are sent as slp messages with no attached msnobj. The message itself is sent in UTF16, in spite of the data being base64 encoded, and is separated from the header information by a \0. Personally I'd say it's a toss up deciding whether they wrote it that way to obfuscate the data, or they just really are that stupid, but whatever, the code required to decode is going to look pretty moronic. the +7 by the way is just to lose the text 'BASE64:' from the start of the data. I guess that much could be commented.

If you are happy enough for me to separate out the winks and leave them for now, I'll try to put in a couple of comments to explain the magic code - hopefully tomorrow, depending on whether I can get the dev environment set up

  Changed 2 years ago by jinn

This is very interesting. What can I do to add the features? Is there a small guide that could help me building these patches? With these patches merged Pidgin would definitively be more interesting.

The most interesting feature would be offline messaging for me. Is there a patch for this? And I dont want the emulation plugin..

in reply to: ↑ 19   Changed 2 years ago by compengi

Replying to notak:

The audio stuff is basically quite cleanly implemented. My main issue is that in linux it isn't going to be too useful until the relevant codec gets included into normal codec packs. Until then I think the option is useful, since it will reject the attempt to send an audio clip at the other end, and stop the sender getting the impression that the receiver has received the message, so I'm quite keen on retaining the protocol option.

notak, what i think is that if distro developers would see that a special codec is important/wanted for/by any well known application like Pidgin then they would begin to include it in there default codec packs, but since it's not well used or asked for to be used, they won't include it. I can give you an example of that, if you work or used to work on Ubuntu before Feisty release, you used to download each media codec, java, flash etc.. alone, but when developers saw that people need those packages they joined them in one single package called ubuntu-restricted-extras. So in my opinion i think you should go ahead and merge your audio stuff in Pidgin.

follow-up: ↓ 23   Changed 2 years ago by galt

I just applied the handwritten messages part of the patch to 2.3.1's MSNP14 source and it still works. It spouts some warnings, crashed once, and for some reason overrides my "Always hide new conversations" setting (I'm surprised that a change limited to a protocol plugin can cause that).

But other than that, it works. I could probably make a clean and updated patch only for handwritten messages if that would help getting this committed.

in reply to: ↑ 22   Changed 2 years ago by notak

I've got no objections to that - there's no chance of me working on this patch anytime soon.

  Changed 2 years ago by galt

OK, I got it to compile without warnings and still work as before. It needed explicit casts and sorting out some const modifiers, and a few other changes.

I'm going to test this a bit, see if I can reproduce a crash (and fix it?) then post the patch here.

About handwritten messages not respecting new-conversation-hiding settings: that's true for all custom emoticons. I'll open a new ticket for that issue.

  Changed 2 years ago by galt

Ticket #4551 opened for the hiding issue mentioned above.

Changed 2 years ago by galt

The patch for 2.3.1 to enable receiving handwritten messages.

Changed 2 years ago by galt

Took out a line about voice clips.

follow-up: ↓ 27   Changed 2 years ago by QuLogic

Alright, so I've looked at the separate patch for handwritten messages. It's looking obvious that it won't work with MSNP14 since msn_message_parse_payload has been changed, and it is used in more places now that will need to be updated. There are also a couple things I don't quite get, as well.

For example, GHashTable *multipartmessages; I assume it is supposed to index Message-ID's to their initial MsnMessage. However, shouldn't this be a member of a MsnSwitchboard or something similar? Presumably it's random, but I see no reason why the Message-ID could not be the same between different conversations, or over different accounts, and using one table sounds like we'll just being confusing messages together. Also, I don't see you removing anything from this table.

I don't really think msn_message_parse_payload should be returning a new MsnMessage. If it returns NULL, then the original msg passed in as the first parameter is not destroyed, and it's a bit messy to fix up. Perhaps just re-writing it as msn_message_new_from_payload or some similar change would work better.

Also, do you have an example of some data? That parsing stuff in slpcall.c looks a bit more complicated than it needs to be.

Finally, I think datacast_table in switchboard.c can probably be removed (for the split patch).

in reply to: ↑ 26   Changed 2 years ago by galt

Replying to QuLogic:

Alright, so I've looked at the separate patch for handwritten messages. It's looking obvious that it won't work with MSNP14 since msn_message_parse_payload has been changed, and it is used in more places now that will need to be updated. There are also a couple things I don't quite get, as well.

This patch is against MSNP14. As long as there wasn't a conflicting change to trunk since 2.3.1, it should be OK.

I think datacast_table in switchboard.c can probably be removed (for the split patch).

Right.

As to all the MSN magic stuff, I haven't touched that. I just split, merged and cleaned up the original patch from notak. For now, I don't actually know much about the MSNP :-)

Hopefully someone who knows more can address the issues you raised.

  Changed 2 years ago by notak

It's been a while, but the following springs to mind:

If the multipart message stuff is being a pain, you could just rip it out and dn't support handwritten messages in group chats for now (in 1-1 im sessions the hw message is always sent as the bastardized slp message)

Without looking at the code I guess it probably would make sense for the hashtable to be in switchboard, not least because it would allow cleanup of incomplete messages. I *think* complete messages are removed from the hashtable currently when they are complete and the function returns them. If not then they probably should be.

msn_message_parse_payload was only used in a couple of places when the code was written. not sure why this would change, but a simple grep would answer the question.

I don't have any data snippets - when I was developing this patch I had easy access to a windows machine so I was just sending the messages on the fly.

follow-up: ↓ 30   Changed 2 years ago by galt

It seems to me that the msg passed in is the same msg that's being returned, and is either destroyed, or gets saved and later destroyed when the multipart message is complete, which seems reasonable.

About removal from the table, there is one line in the patch where messages do get removed.

in reply to: ↑ 29   Changed 2 years ago by QuLogic

Replying to galt:

It seems to me that the msg passed in is the same msg that's being returned, and is either destroyed, or gets saved and later destroyed when the multipart message is complete, which seems reasonable.

No, there are a few places where NULL is returned, but the supplied message is only destroyed when the initial message is found.

Replying to galt:

About removal from the table, there is one line in the patch where messages do get removed.

Ah, yes, I missed that. But of course, since the table is static, nothing is cleaned up when the account is closed, disconnected, etc.

  Changed 2 years ago by QuLogic

Aha, I knew I'd seen some documentation somewhere.

Siebe has some Ink stuff on his site. Also, there's a bit about the P4P messages, and a slightly different write-up on Ink in the Word documents on the sidebar.

  Changed 2 years ago by galt

Replying to QuLogic:

Replying to galt:

It seems to me that the msg passed in is the same msg that's being returned, and is either destroyed, or gets saved and later destroyed when the multipart message is complete, which seems reasonable.

No, there are a few places where NULL is returned, but the supplied message is only destroyed when the initial message is found.

I think the g_return_val_if stuff should return the msg rather than a NULL, so that it gets destroyed. The other 2 places where NULL is returned are really for multipart messages that we don't want to destroy just yet, if I get the code right.

Ah, yes, I missed that. But of course, since the table is static, nothing is cleaned up when the account is closed, disconnected, etc.

So if we move the table into MsnSwitchBoard? and pass it in as an argument, would that solve it? There are only a few calls to msn_message_parse_payload, and all callers have a switchboard from which to pass in a table, with the exception of the one in oim.c - but that's just posting an offline message to the conversation.

  Changed 2 years ago by rlaager

What's the status of the im.pidgin.cpw.khc.msnp14.ticket393 branch? Have the most recent changes been committed there?

  Changed 21 months ago by salinasv

cc me

  Changed 21 months ago by rlaager

I re-read the comments here. Has there been any progress lately?

  Changed 20 months ago by salinasv

  • milestone set to Patches Needing Improvement

Milestone-fu

  Changed 19 months ago by Dimmuxx

cc

  Changed 18 months ago by galt

I still use this patch and it still works, except for a conflict with the new custom-smiley limit of 96 pixels. Once I disable that limit in the code, the patch still works nicely in 2.5.0.

  Changed 17 months ago by felipec

Handwritten messages in one of the most requested features of msn-pecan: http://code.google.com/p/msn-pecan/issues/detail?id=49

I would gladly integrate this into msn-pecan, however, some cleanup is needed.

Anyone interested?

  Changed 17 months ago by felipec

Devid Filoni rebased the patch for msn-pecan so I'll include it for the next release.

notak, galt: would you like to give me your names for the credit?

  Changed 17 months ago by galt

felipec: galt == Gal Topper. Glad to hear you're including it.

  Changed 17 months ago by rlaager

felipec/galt: If you have a link to the patch/commit (or can post it here), we're definitely interested in including this in libpurple's MSN plugin.

  Changed 17 months ago by notak

notak = Chris Stafford. cheers

  Changed 17 months ago by galt

felipec, rlaager: Please keep in mind that, as I noted earlier, Pidgin 2.5.x limits custom smilies to 96x96 pixels, which is too small for most handwritten messages.

  Changed 17 months ago by felipec

Yes, a regression from 2.4.x in my opinion.

If this gets integrated into libpurple you would get a bug report for that, and the component is not msn, but libpurple.

follow-up: ↓ 48   Changed 17 months ago by rlaager

As you're clearly familiar with this, does either of you have a reference to the commit that changed this, or the code in question? I'm sure we can find an appropriate solution, regardless of whether we commit *this* change to libpurple (which I would still like to, but I'm saying they're unrelated).

in reply to: ↑ 47   Changed 17 months ago by salinasv

Replying to rlaager:

As you're clearly familiar with this, does either of you have a reference to the commit that changed this, or the code in question? I'm sure we can find an appropriate solution, regardless of whether we commit *this* change to libpurple (which I would still like to, but I'm saying they're unrelated).

This was #5231. Then the code was moved to gtkimhtml.c I have not read carefuly this code. I guess we can add a field in the GtkIMHtmlSmiley struct that indicates if its a custom smiley or a handwritten stuff. Then conditionally resize the smiley.

I'm FULLY against big emoticon in my gtkconv.

  Changed 17 months ago by galt

rlaager: pidgin/gtkimhtml.c:gtk_custom_smiley_size_prepared

Personally, I don't like the lossiness in resizing the received icons. On the other hand, displaying very large icons or handwritten messages as-is is a bit annoying as well (but they go away when the conversation window is closed, so it doesn't bother me too much).

  Changed 17 months ago by felipec

rlaager: Add an API for handwritten messages then.

  Changed 13 months ago by qulogic@…

(In [a2f4d295f1d985858eacada121adb414f6cc924b]):
Re-combine large (multi-part) messages on MSN since we seem to say that we do support receiving chunked messages.

References #393, though not the same as the patch in the ticket.

  Changed 7 months ago by qulogic@…

(In [51d199ba5dd8a81f6276f19421c8f7e0158dcb98]):
Add support for receiving handwritten (Ink) messages from MSN buddies. Based on patch from notak and galt, but using imgstore instead of a custom smiley (like AIM DirectIM), and with better error checking.

References #393.

  Changed 7 months ago by qulogic@…

(In [a8422436d38c6888ffb8da715cd67333fe40ebe6]):
Add support for receiving winks and audio clips on MSN. The resulting file is linked from the conversation window with msn-wink:// and audio:// links for further processing by the UI. It's up to the UI to interpret this in whichever way it pleases.

References #393.

  Changed 7 months ago by qulogic@…

(In [f5644cc4891cdcc46fafcec5127d00e48aa48e22]):
Add support in Pidgin for playing back audio:// links. Also include a "Save File" menu item to save a permanent copy.

Playback uses purple_sound_play_file, which uses a playbin. This will not correctly play back the Siren codec until Bug 588218 is fixed, but that looks like it will be soon.

Also, if anyone has a better idea for GtkIMHtml->PidginConversation? than g_object_[gs]_data, feel free to fix it.

References #393.

Note: See TracTickets for help on using tickets.