Martin Mathieson schrieb:
(1) The single 'tap reset' function that is passed to
register_tap_listener() doesn't distinguish between:
- resetting because new packets are coming
- resetting because the same packets are going to scanned again.
reset functions should be called only for the second reason. If it is
called whenever new packets are coming, it is a bug. I had to change the
original voip call analysis tap to make it use "reset" and "draw"
functions correctly.
Due to lack of time I haven't had a look at the rtp analysis tap, but I
suspect it to have the same bugs as the original VoIP tap, as the latter
used it as a basis.
By the first reason, I really meant new packets as in:
- a new capture file being loaded, or
- a new actual capture being started, or maybe even
- the current list of packets being closed
i.e. packets that could be ignored by an up-to-date tap. This would be
complicated by allowing filters to be applied to the tap of course, so
perhaps this isn't really an option.
(2) Change Voip Calls to use fixed rather than dynamic memory, so that
if
the same packets are re-tapped the same details go back into the same
memory
addresses and the Graph Analysis window will continue to refer to valid
data.
Well, it uses fixed memory (at least the structure to hold basic tap
information is static, while the "per call" data has to be dynamic). I
plan to change it to use dynamic memory to allow calling it more than
once, e.g. with different tap packet filter expressions. However this
feature will take some time to develop, as it will require some
additional "pointer magic".
So what exactly should be changed to use fixed instead of dynamic memory.
Whenever voip_calls_init_tap() is called, it frees up the list of
graph_analysis_item_t's held in the graph_analysis_info_t.
voip_calls_init_tap() gets called by the menu entry
Statistics->"VoIP Calls..." or by the command line option "-z
voip,calls". There shouln't be any other way to call it.
It shouldn't get called, when you add another tap like
"Statistics->RTP->Show All Streams..."
Anyhow, The data for the graph window gets already invalidated when the
taps are resetted. The best solution would be, to keep the graph window
up to date with voip_calls_dlg_draw(). This will require some
investigation and probably a code cleanup, too.
dialog_graph_draw() works out which items are in view each time and copies
data from graph_analysis_info_t structures into an array of display_items_t
structures. When the retapping has happened, mostly the items are not drawn
because the graph_analysis_info_t 'display' fields are reset to FALSE.
Perhaps just setting the display fields to a correct value within
voip_calls_dlg_draw() might be enough to fix this.
Hmm, strange things happen with your ethereal, is it up to date?
Currently the graph analysis window gets updated only when you press the
"Graph" button in the Call Analysis window. It fetches the selected data
at "click" time and I see no problem with that, as the Call Analysis
window is always up to date and even recognizes loading of new capture
files.
Can you provide a scheme, how you trigger the bugs you are reporting, so
I can reproduce them?
My ethereal is up to date, the only diffs I have I'm pretty sure are not
affecting this.
You can hopefully see this behaviour in the following way:
- Load a file with at least one VoIP call, including RTP (an H.323 call with
more rows than the default window size would be ideal)
- Open up 'Voip Calls' and display the graph for a call.
- With that still open, choose 'Statistics | RTP | Show all streams...'
- Now try clicking the up and down scroll buttons on the Graph Analysis
window...
I see. I'll think about an efficient way to keep the graph window up to
date.
Best regards,
Lars
