[Lars Roland <lars.roland@xxxxxxx>]

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