New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GTK+ module #48

Merged
merged 3 commits into from Jul 7, 2015

Conversation

Projects
None yet
5 participants
@clehner
Contributor

clehner commented Jul 5, 2015

Thank you Alfred and contributors for making a great program with such good feature support and modularity.

This module adds a GTK+ UI to baresip. It uses a status icon with a drop-down menu for application actions, and dialog windows for calls. It is intended to be unobtrusive and easy to use.

Menu and status submenu

Features

  • status icon with drop-down menu
  • submenu of contacts for quick dialing
  • submenus for setting the active account and presence
  • notifications for incoming calls and chats
    An incoming call notification (xfce4-notifyd)
  • a dialog window for placing a call
    The dial window
  • a dialog window for each call
    An established call
  • call window buttons for mute, hold, transfer, and hangup
  • volume meters in the call windows
  • sending DTMF using keypresses in a call window
  • a dialog window for transferring call

Current issues

  • The volume meters are updated using audio filters as in the vumeter module. The audio filters don't have a reference to the call they are associated with, so I am doing a bit of a hack to associate audio filters with call windows. It seems to work but I haven't tested it extensively. To improve the situation I would add a pointer to the audio struct to the audio filter parameter, and perhaps a user arg pointer for registering audio filters.
  • When a call transfer event is received, the UA code creates a new call to the transfer target. This results in a new call window appearing, and the old one stays visible and doesn't show that it is disconnected. A better situation would be the existing call window is used for the new call. This might involve changes to ua.c.
  • I didn't find an API for getting info about the call audio encoder/decoder, or an event for when the encoder/decoder changes. It would be nice to have this so that it can be shown in the call windows. Also, there is a method for cycling the encoder but it would be nice to be able to list the available encoders so as to put them in a drop-down menu for the user to choose from.
  • There are sometimes crashes coming from GTK. I haven't figured out what is causing this but I suspect it is a thread or memory management issue. I also am sometimes seeing crashes coming from libre having to do with memory allocation, which could be from the same issue.
  • Sometimes the application gets stuck when quitting, possibly because of a race condition with ending the GTK event loop.

Future work

  • Allow setting presence (and registration status) for individual accounts
  • Show current and incoming calls in the menu, in case the incoming call notification was dismissed accidentally, or the call window is in another workspace.
  • Add a UI for chat
  • Add a log window
  • Add a redial button for closed calls
  • Show call codec settings, and maybe allow changing the encoder
  • Handle video calls
  • Add a .desktop file so DEs can know about baresip

Questions

  • What do you all think of the overall approach of using a menu and dialog windows instead of a main application window?
  • Should a call window stay visible after the user clicks hangup? (In case the user wants to see the total call duration, or have the opportunity to redial).
  • Is it redundant to have a submenu of contacts when the dial dialog also has a menu of contacts?
  • Would a re-invite button for calls be useful?
  • What other things would you like to see in this GUI?
@juha-h

This comment has been minimized.

Show comment
Hide comment
@juha-h

juha-h Jul 5, 2015

Collaborator
Collaborator

juha-h commented Jul 5, 2015

@czarkoff

This comment has been minimized.

Show comment
Hide comment
@czarkoff

czarkoff Jul 6, 2015

Contributor

Regarding questions:

  1. Menus work fine when you can reach them by calling program's executable with an argument. Your UI assumes there is a system tray, which may be absent.
  2. I'd rather dismiss the call UI after (1) call is ended and (2) focus stays out of that window for eg. 10 seconds. This may actually be configurable.
  3. It is rather redundant to have contacts in dial dialog. I'd rather rename "Dial contact" menu item to "Dial" and attach "Dial..." button under name "Enter URI" on top of that menu, followed by separator. I'd also repurpose dropdown menu from "Dial" dialog to show a list of recent URIs sorted by last call date.
  4. Don't clutter UI.
Contributor

czarkoff commented Jul 6, 2015

Regarding questions:

  1. Menus work fine when you can reach them by calling program's executable with an argument. Your UI assumes there is a system tray, which may be absent.
  2. I'd rather dismiss the call UI after (1) call is ended and (2) focus stays out of that window for eg. 10 seconds. This may actually be configurable.
  3. It is rather redundant to have contacts in dial dialog. I'd rather rename "Dial contact" menu item to "Dial" and attach "Dial..." button under name "Enter URI" on top of that menu, followed by separator. I'd also repurpose dropdown menu from "Dial" dialog to show a list of recent URIs sorted by last call date.
  4. Don't clutter UI.
@clehner

This comment has been minimized.

Show comment
Hide comment
@clehner

clehner Jul 6, 2015

Contributor

On Sun, 05 Jul 2015 17:01:09 -0700
"Dmitrij D. Czarkoff" notifications@github.com wrote:

  1. Menus work fine when you can reach them by calling program's
    executable with an argument. Your UI assumes there is a system tray,
    which may be absent.

I see. One option would be for the module to add a text UI command to pop up the menu under the cursor, and then this could be accessed from the terminal, or by using the cons module and a netcat wrapper, or with the httpd module and curl. Triggering the action using a command line argument would be nice but modules currently don't have access to argv. This could perhaps be a separate executable that allows for controlling a remote baresip using CLI arguments.

  1. I'd rather dismiss the call UI after (1) call is ended and (2) focus stays out of that window for eg. 10 seconds. This may actually be configurable.

That's reasonable. Based on the "principle of least surprise" I would be wary of having the window close automatically based on heuristics, but in the case of an ended call it may be okay. Linphone closes a call's pane a few seconds after the call ends.

  1. It is rather redundant to have contacts in dial dialog. I'd rather rename "Dial contact" menu item to "Dial" and attach "Dial..." button under name "Enter URI" on top of that menu, followed by separator. I'd also repurpose dropdown menu from "Dial" dialog to show a list of recent URIs sorted by last call date.

That sounds good. A list of recent URIs could be implemented in the module.

Contributor

clehner commented Jul 6, 2015

On Sun, 05 Jul 2015 17:01:09 -0700
"Dmitrij D. Czarkoff" notifications@github.com wrote:

  1. Menus work fine when you can reach them by calling program's
    executable with an argument. Your UI assumes there is a system tray,
    which may be absent.

I see. One option would be for the module to add a text UI command to pop up the menu under the cursor, and then this could be accessed from the terminal, or by using the cons module and a netcat wrapper, or with the httpd module and curl. Triggering the action using a command line argument would be nice but modules currently don't have access to argv. This could perhaps be a separate executable that allows for controlling a remote baresip using CLI arguments.

  1. I'd rather dismiss the call UI after (1) call is ended and (2) focus stays out of that window for eg. 10 seconds. This may actually be configurable.

That's reasonable. Based on the "principle of least surprise" I would be wary of having the window close automatically based on heuristics, but in the case of an ended call it may be okay. Linphone closes a call's pane a few seconds after the call ends.

  1. It is rather redundant to have contacts in dial dialog. I'd rather rename "Dial contact" menu item to "Dial" and attach "Dial..." button under name "Enter URI" on top of that menu, followed by separator. I'd also repurpose dropdown menu from "Dial" dialog to show a list of recent URIs sorted by last call date.

That sounds good. A list of recent URIs could be implemented in the module.

@czarkoff

This comment has been minimized.

Show comment
Hide comment
@czarkoff

czarkoff Jul 6, 2015

Contributor

I don't agree with TUI command to activate GUI element. TUI and GUI should be independent in my opinion. I'd rather open a socket of fifo and accept commands there. Or just rename module to "gtkicon" and be done with that.

Contributor

czarkoff commented Jul 6, 2015

I don't agree with TUI command to activate GUI element. TUI and GUI should be independent in my opinion. I'd rather open a socket of fifo and accept commands there. Or just rename module to "gtkicon" and be done with that.

@clehner

This comment has been minimized.

Show comment
Hide comment
@clehner

clehner Jul 6, 2015

Contributor

I don't agree with TUI command to activate GUI element. TUI and GUI should be independent in my opinion. I'd rather open a socket of fifo and accept commands there. Or just rename module to "gtkicon" and be done with that.

At first I was calling the module gtk_menu, but then I changed it to just gtk, thinking that it is enough trouble to get GTK+ working here that if someone wants a slightly different GTK+ UI, it could be added on to this one and have parts (de-)activated using config settings. Of course, if someone wants to create a drastically different GTK+ UI, then sure, one could be renamed.

If this module used a fifo, what commands would it accept?

  1. pop up the menu
  2. open the dial window
  3. open the about window
  4. answer a call
  5. reject a call
  6. hangup/mute/transfer/hold/resume an active call
  7. set presence
  8. set active account
  9. quit

4,5,6,7,8,9 can be done using the text UI. (although 6 doesn't necessarily update the module UI, that could be changed by modifying core) 3 is not that useful. 2 might be useful. 1 I've added a text command for. So there is not much here that would benefit from having another interface for, because it can mostly already be activated using the text UI. Of course another text UI module for a fifo interface might be useful.

Another thing:

I read in the GNOME developer pages that GTK+'s Win32 backend doesn't support making GTK+/GDK calls from multiple threads using the gdk_threads_enter()/gdk_threads_leave() lock as this module is now doing. If there are any Windows users here who are interested in this module and can confirm that it doesn't work, I would consider rewriting parts of it to use asynchronous queues instead of gdk_threads_enter()/gdk_threads_leave().

Contributor

clehner commented Jul 6, 2015

I don't agree with TUI command to activate GUI element. TUI and GUI should be independent in my opinion. I'd rather open a socket of fifo and accept commands there. Or just rename module to "gtkicon" and be done with that.

At first I was calling the module gtk_menu, but then I changed it to just gtk, thinking that it is enough trouble to get GTK+ working here that if someone wants a slightly different GTK+ UI, it could be added on to this one and have parts (de-)activated using config settings. Of course, if someone wants to create a drastically different GTK+ UI, then sure, one could be renamed.

If this module used a fifo, what commands would it accept?

  1. pop up the menu
  2. open the dial window
  3. open the about window
  4. answer a call
  5. reject a call
  6. hangup/mute/transfer/hold/resume an active call
  7. set presence
  8. set active account
  9. quit

4,5,6,7,8,9 can be done using the text UI. (although 6 doesn't necessarily update the module UI, that could be changed by modifying core) 3 is not that useful. 2 might be useful. 1 I've added a text command for. So there is not much here that would benefit from having another interface for, because it can mostly already be activated using the text UI. Of course another text UI module for a fifo interface might be useful.

Another thing:

I read in the GNOME developer pages that GTK+'s Win32 backend doesn't support making GTK+/GDK calls from multiple threads using the gdk_threads_enter()/gdk_threads_leave() lock as this module is now doing. If there are any Windows users here who are interested in this module and can confirm that it doesn't work, I would consider rewriting parts of it to use asynchronous queues instead of gdk_threads_enter()/gdk_threads_leave().

@czarkoff

This comment has been minimized.

Show comment
Hide comment
@czarkoff

czarkoff Jul 7, 2015

Contributor

Dependency on TUI makes the merit of this module questionable: if user has to use TUI anyway, why would he want to get troubled with GUI at all?

Contributor

czarkoff commented Jul 7, 2015

Dependency on TUI makes the merit of this module questionable: if user has to use TUI anyway, why would he want to get troubled with GUI at all?

@clehner

This comment has been minimized.

Show comment
Hide comment
@clehner

clehner Jul 7, 2015

Contributor

There is no dependency on TUI here.

I've posted this module and opened a pull request here in the hopes that others may find it useful. If you do not find it useful I kindly ask you to let it alone.

Contributor

clehner commented Jul 7, 2015

There is no dependency on TUI here.

I've posted this module and opened a pull request here in the hopes that others may find it useful. If you do not find it useful I kindly ask you to let it alone.

@czarkoff

This comment has been minimized.

Show comment
Hide comment
@czarkoff

czarkoff Jul 7, 2015

Contributor

Oh, you got me wrong. I was trying to suggest an improvement. I am all for accepting your pull request. (Albeit I didn't thoroughly review it yet.)

Contributor

czarkoff commented Jul 7, 2015

Oh, you got me wrong. I was trying to suggest an improvement. I am all for accepting your pull request. (Albeit I didn't thoroughly review it yet.)

@clehner

This comment has been minimized.

Show comment
Hide comment
@clehner

clehner Jul 7, 2015

Contributor

Ok, I see. At this point I don't know if it should be accepted because I am getting frequent segfaults that I can't find the source of. I am working on fixing this though.

Contributor

clehner commented Jul 7, 2015

Ok, I see. At this point I don't know if it should be accepted because I am getting frequent segfaults that I can't find the source of. I am working on fixing this though.

@alfredh alfredh merged commit c96f478 into alfredh:master Jul 7, 2015

@alfredh

This comment has been minimized.

Show comment
Hide comment
@alfredh

alfredh Jul 7, 2015

Owner

thank you Charles for this nice and clean patch, I have merge it into master with 1 additional
commit for some minor cleanups.

many people have asked for a GUI and you are the first person to submit some actual code
for it :)

I suggest we use the new module "gtk" as a starting point, and can iterate from here.

some input:

  • if gtk.so module is loaded, a graphical window should be presented. there should not be any
    need to press any key ('G'). This is because if people are loading the gtk.so module then
    they know what they are doing and do not need the menu.so/stdio.so modules.
  • global functions (non-static) inside modules/gtk should be prefixed with something
    unique within baresip (e.g. gtk_xxx) to avoid symbol clashing. and this can be tested
    when you compile baresip static (make STATIC=1)
  • quick testing on OSX is working, using GTK from Macports/Homebrew.

Keep up the good work Charles! Keep them patches coming :)

Owner

alfredh commented Jul 7, 2015

thank you Charles for this nice and clean patch, I have merge it into master with 1 additional
commit for some minor cleanups.

many people have asked for a GUI and you are the first person to submit some actual code
for it :)

I suggest we use the new module "gtk" as a starting point, and can iterate from here.

some input:

  • if gtk.so module is loaded, a graphical window should be presented. there should not be any
    need to press any key ('G'). This is because if people are loading the gtk.so module then
    they know what they are doing and do not need the menu.so/stdio.so modules.
  • global functions (non-static) inside modules/gtk should be prefixed with something
    unique within baresip (e.g. gtk_xxx) to avoid symbol clashing. and this can be tested
    when you compile baresip static (make STATIC=1)
  • quick testing on OSX is working, using GTK from Macports/Homebrew.

Keep up the good work Charles! Keep them patches coming :)

@clehner

This comment has been minimized.

Show comment
Hide comment
@clehner

clehner Jul 8, 2015

Contributor

Thanks for merging. That's cool that it works on OSX. Does the status icon show up in the menu bar?

I am able to build it with make STATIC=1. I'll work on getting the functions a common prefix.

I made some changes, in a gtk-safer branch, to only call GTK+/GDK functions in the GTK+ thread, in the hope that it would eliminate some bugs. I'm still getting some segfaults though. The changes might be useful for getting the module working on Win32 (or with GTK3, where the gdk thread locks are deprecated - but so are status icons) . The code is more complicated though because of more message passing indirection. So there are still some bugs in any case. I suspect one problem may be calling mem_deref being called in the GTK+ thread causing some corruption with baresip's datastructures. Or it could be some mistake I didn't notice. I'll do some more experimenting to try to figure it out.

I was able to resolve one bug, which was that quitting from GTK+ was causing a segfault. This is resolved by loading the module with "module", not "module_app". When the app requests to close, all modules are unloaded and then it seems that execution returns into the gtk module's unloaded code. I suspect this could be worked around by calling ua_stop_all from a re timer, but I didn't try that because I'm not sure what the advantage of using module_app would be (or why the same bug doesn't happen using "module").

What would you envision being in a window that would appear when the module is loaded? Would this be a preferences window for editing config and accounts? Or a contact list window?

Contributor

clehner commented Jul 8, 2015

Thanks for merging. That's cool that it works on OSX. Does the status icon show up in the menu bar?

I am able to build it with make STATIC=1. I'll work on getting the functions a common prefix.

I made some changes, in a gtk-safer branch, to only call GTK+/GDK functions in the GTK+ thread, in the hope that it would eliminate some bugs. I'm still getting some segfaults though. The changes might be useful for getting the module working on Win32 (or with GTK3, where the gdk thread locks are deprecated - but so are status icons) . The code is more complicated though because of more message passing indirection. So there are still some bugs in any case. I suspect one problem may be calling mem_deref being called in the GTK+ thread causing some corruption with baresip's datastructures. Or it could be some mistake I didn't notice. I'll do some more experimenting to try to figure it out.

I was able to resolve one bug, which was that quitting from GTK+ was causing a segfault. This is resolved by loading the module with "module", not "module_app". When the app requests to close, all modules are unloaded and then it seems that execution returns into the gtk module's unloaded code. I suspect this could be worked around by calling ua_stop_all from a re timer, but I didn't try that because I'm not sure what the advantage of using module_app would be (or why the same bug doesn't happen using "module").

What would you envision being in a window that would appear when the module is loaded? Would this be a preferences window for editing config and accounts? Or a contact list window?

@ishmaelen ishmaelen referenced this pull request Dec 8, 2016

Closed

GStreamer 1.4.5 #171

@r3code

This comment has been minimized.

Show comment
Hide comment
@r3code

r3code Apr 13, 2018

@clehner How should I build this module and how to start baresip with it? I see it in the repo but it won't compile when I run make && make install.

r3code commented Apr 13, 2018

@clehner How should I build this module and how to start baresip with it? I see it in the repo but it won't compile when I run make && make install.

@r3code

This comment has been minimized.

Show comment
Hide comment
@r3code

r3code Apr 13, 2018

Reply to myself - Just download and build libgtk-2 then rebuild baresip by make && make install

r3code commented Apr 13, 2018

Reply to myself - Just download and build libgtk-2 then rebuild baresip by make && make install

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment