Calculate moving averages in IO Graphs

This issue was migrated from bug 6236 in our old bug tracker.

Original bug information:

Reporter: Albert Rafetseder
Status: RESOLVED FIXED
Product: Wireshark
Component: GTK+ UI
OS: All
Platform: All
Version: unspecified

Attachments:

io_graph_moving_average.diff: Patch to add drop-down list to IO Graphs
io_stat.diff: FIX: Patch to add drop-down list to IO Graphs
io_stat.diff: FIX #‍2: Patch to add drop-down list to IO Graphs
wireshark_smooth.patch: Patch to fix div-by-zero error in the moving average filter

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

Child items 0

No child items are currently assigned. Use child items to break down this issue into smaller parts.

(In reply to comment #0) Sorry for filing this bug without adding a description -- my click finger was faster than my brain. Please find attached a patch (by courtesy of my colleague Akos Lukovics) that adds a dropbox to the IO Graph dialog from which you can choose a number of "filters" (in a Digital Signal Processing sense, not Wireshark Display Filters) for the Y axis config. You can choose to display moving averages over packets/bytes/bits with different window sizes. Calculating the moving average (rather than the average per interval, which the IO Graph dialog already provides) has the benefit that it's less susceptible to show aliasing artifacts for traces whose packet rates approach the current Y axis scale.

(In reply to comment #3) > I don't really like the placement of the filter combobox in the GUI. We don't > have the standard 6 pixel border in the IO Graph because we want the window to > be as small as possible, and the filter expands the window size. And we get a > large unused area below the Graph controls which does not look nice. Well, thanks for considering our patch in the first place. Should we rather make it an "Advanced..." option for the y axis? That would leave the original appearance of the IO Graph window intact, and we can address the naming issue (see below) as well. > We only have one filter, called "M.avg", which does not tell me anything what > it does. We need some clarification in the GUI, and this feature needs to be > documented in the user guide. Granted. I'd like to add it shouldn't be called "filter" to avoid confusion with display filters of the "tcp.flags.syn==1" kind. > It seems like a bug in the calculation of the leftmost values. Try having a > large capture (or set a small tick interval) to get a scrollbar. When > scrolling the graph the graph changes, which makes it non reliable. The > filtered data has to be calculated for the whole data set, not only the > displayed ones. Oops, we'll fix that and post another patch.

(In reply to comment #3) I have attached a new diff with a few changes. > I don't really like the placement of the filter combobox in the GUI. We don't > have the standard 6 pixel border in the IO Graph because we want the window to > be as small as possible, and the filter expands the window size. And we get a > large unused area below the Graph controls which does not look nice. > The feature is now integrated in the Graphs box instead of the Y Axis box, so there should not be any extra unused space. Still, it makes the IO Graph window a bit wider. > We only have one filter, called "M.avg", which does not tell me anything what > it does. We need some clarification in the GUI, and this feature needs to be > documented in the user guide. > The name is now changed to "Smooth" and "Moving avg, XX samples". The feature is not yet documented, as the feature should be finalized first. > It seems like a bug in the calculation of the leftmost values. Try having a > large capture (or set a small tick interval) to get a scrollbar. When > scrolling the graph the graph changes, which makes it non reliable. The > filtered data has to be calculated for the whole data set, not only the > displayed ones. The bug with the scrolling should be fixed now. Also, the algorithm has been changed to Central Moving Average, which means not only past but also future values (if existant) are taken into account. This makes the smoothing somewhat better on the first tapped intervals.

(In reply to comment #9) > > The feature is now integrated in the Graphs box instead of the Y Axis box, so > > there should not be any extra unused space. Still, it makes the IO Graph window > > a bit wider. > > Hmmm, no. This is even worse as it pollutes the otherwise simple dialog. > > Maybe just add another entry in the Y-axis Unit called "Smooth...", which lets > the user select smoothing instead of calc (as in "Advanced")? Another entry in the y axis combobox sounds like a good idea so you get either the standard, or the advanced, or the smoothed variant of the dialog. I'd still keep the new dialog as we have it right now, i.e. with display filter and calc textboxes, and a window size comboboxes: If you drop the calc textbox, you'd have to go with one pre-selected quantity to plot; on the other hand, you couldn't smooth any of the Advanced graphs. Akos, how about another iteration?

(In reply to comment #9) > > Hmmm, no. This is even worse as it pollutes the otherwise simple dialog. > > Maybe just add another entry in the Y-axis Unit called "Smooth...", which lets > the user select smoothing instead of calc (as in "Advanced")? The problem is that it does not fit logically into the idea of units, as it is not an actual unit, but a display feature. If there would be a standalone "Smooth" unit, then the user would have to be presented somewhere with an option to choose the actual units, which would cause redundancy and even more confusion to the user. An option would be to add combined units like "Bits/Tick+Smooth". And if the user would choose one of the combined units then the current dropboxes would be shown. However, this would double the number of available units. A second option would be to add a checkbox "Smooth" to the Y Axis box similar to the "View as time of day" of the X Axis, which would hide/unhide the current drop-boxes. However, the checkbox would add an extra empty line similarly to the first version. This line could be filled, for example, with a sixth Graph; although that is most likely undesirable. I cannot come up with any other options at the moment, so I am open to suggestions.

(In reply to comment #11) > The problem is that it does not fit logically into the idea of units, as it is > not an actual unit, but a display feature. If there would be a standalone > "Smooth" unit, then the user would have to be presented somewhere with an > option to choose the actual units, which would cause redundancy and even more > confusion to the user. Ok, I see your points. If you can pull out the bugfixes from the patch we can apply them while figuring out a GUI solution.

Sorry for not posting this to the description of the patch file. The attached patch fixes a bug in the moving average calculation which could make wireshark crash. The bug can be reproduced by following these steps: 1) start a capture 2) open the io graph 3) create a filter which filters out all packets, e.g. "tcp.port==0" 4) set smooth to some value other than "No filter" 5) crash. The reason for the bug was a division-by-zero error.

Calculate moving averages in IO Graphs

Activity