Opened 12 years ago

Closed 11 years ago

#3045 closed Bug (fixed)

Size & speed units don't match desktop policies, and also is implemented independently in each app

Reported by: hernejj Owned by:
Priority: Low Milestone: 2.10
Component: Transmission Version: 1.91
Severity: Minor Keywords: patch-needed
Cc: benjamin.drung@…

Description

transmission lists bandwidth measurements in KB/s. Is that in base-2 or base-10? It should instead use one of the following:

kB (base-10 SI standard) KiB (base -2 IEC standard)

Places to look for more information on these standards: http://en.wikipedia.org/wiki/Binary_prefix https://wiki.ubuntu.com/UnitsPolicy

I've notived four places where this happens: 1) Download status bar indicator 2) Upload status bar indicator 3) Preferences -> Speed -> Download Limit 4) Preferences -> Speed -> Upload Limit

This bug was originally reported on Ubuntu's bug tracker by user Benjamin Drung. Here is the link: https://bugs.launchpad.net/ubuntu/+source/transmission/+bug/538504

Attachments (5)

units.diff (21.2 KB) - added by charles 11 years ago.
experimental patch
units.r2.diff (38.1 KB) - added by charles 11 years ago.
Add the qt and command-line apps. Use the correct default values for Snow Leopard and Ubuntu.
units.r3.diff (162.2 KB) - added by charles 11 years ago.
units.r4.diff (167.6 KB) - added by charles 11 years ago.
make r3 more backwards-friendly to settings.json
units.r5.diff (202.5 KB) - added by charles 11 years ago.
updated diff

Download all attachments as: .zip

Change History (28)

comment:1 Changed 12 years ago by bdrung

  • Cc benjamin.drung@… added

comment:2 Changed 12 years ago by charles

  • Component changed from Transmission to GTK+ Client
  • Resolution set to invalid
  • Status changed from new to closed

Transmission uses terminology consistent with g_format_size_for_display().

If that function's units are incorrect, this needs to be addressed upstream with glib, not in Transmission.

http://library.gnome.org/devel/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-format-size-for-display

comment:3 follow-up: Changed 12 years ago by bdrung

  • Resolution invalid deleted
  • Status changed from closed to reopened

We have fixed g_format_size_for_display() in Ubuntu to show the units in base-10. Only file sizes use g_format_size_for_display(), but the bandwidth uses hard-coded KB/s.

comment:4 Changed 12 years ago by bdrung

Here is a screen shot showing the correct units for file sizes, but the wrong for bandwidth: http://launchpadlibrarian.net/40927427/incorrect_bandwidth_unit.png

The GTK+ Client is not the only affected component (for example, the CLI is affected too).

comment:5 in reply to: ↑ 3 Changed 12 years ago by charles

Replying to bdrung:

We have fixed g_format_size_for_display() in Ubuntu to show the units in base-10. Only file sizes use g_format_size_for_display(), but the bandwidth uses hard-coded KB/s.

No, what you have done is forked g_format_size_for_display(). I agree that the function probably should use base 10, but the reality on the ground is that it now it will use base 10 on some systems and base 2 on other systems.

Transmission's preferences dialog lets users specify speed limits in KiB/s. If Transmission were to display speeds with g_format_size_for_display() + "/s" as you suggest, Transmission will appear to end users to be exceeding the limit.

Conversely, if Transmission fully adopts Ubuntu's Unit Policy, its speeds will be off in the other direction (never reaching the speed limit) on distros using the upstream implementation of g_format_size_for_display().

In *both* cases, Transmission's input and display units will be inconsistent with one another on at least one platform. Every application in this situation would need to pass test input to g_format_size_for_display() in order to see if it's the upstream implementation or the forked implementation, and then tailor its input dialogs accordingly, as well as massaging sizes and speeds entered by the user.

Transmission has, and will continue, to be responsive to feedback from Ubuntu, but this change makes my life harder. Since you are the author of this g_format_size_for_display() fork, I would very much like to hear your suggestions on how I can handle units in a method that is (A) consistent between Transmission's input and output units, (B) consistent with g_format_size_for_display() on both Ubuntu and non-Ubuntu systems, and (C) doesn't force me to invent piles of new code.

https://bugs.launchpad.net/ubuntu/+source/glib2.0/+bug/538783

comment:6 Changed 12 years ago by charles

  • Keywords patch-needed added

comment:7 Changed 12 years ago by bdrung

What do you think about comment 17 [1]? This will fix (A), (B) for the Ubuntu package, and (hopefully) will be doable regarding (C).

[1] https://bugs.launchpad.net/transmission/+bug/538504/comments/17

comment:9 Changed 12 years ago by charles

libT TODO

Public API to convert to bytes-per-second:

  tr_sessionSetSpeedLimit()
  tr_sessionGetSpeedLimit()
  tr_sessionSetAltSpeed()
  tr_sessionGetAltSpeed()
  tr_torrentSetSpeedLimit()
  tr_torrentGetSpeedLimit()

struct fields:

  tr_peer_stat.rateToPeer
  tr_peer_stat.rateToClient
  tr_stat.rawUploadSpeed
  tr_stat.rawDownloadSpeed
  tr_stat.pieceUploadSpeed
  tr_stat.pieceDownloadSpeed

implementation:

  bandwidth.c: hardcodes 1024 in several places
  ratecontrol.c: nearly identical to bandwidth.c
  makemeta.c: bestPieceSize()
  torrent.c: hardcodes 1024 when calculating ETA

RPC:
  same as above.  Also, must update RPC spec.

config keys:

  TR_PREFS_KEY_ALT_SPEED_UP
  TR_PREFS_KEY_ALT_SPEED_DOWN
  TR_PREFS_KEY_DSPEED
  TR_PREFS_KEY_USPEED

  These are currently in KB/s.  In the past this has been interpreted as KiB/s.
  I don't see a clear Right Thing to do here, but perhaps the less bad thing
  is to keep these keys as-is and interpret their values as kB/s.
  This is probably less confusing to end users than changing their 50 KB/s
  to 49 kB/s (or whatever) when they upgrade Transmission.

Not addressed: transmission-daemon, transmission-remote, transmission-qt, transmission-gtk

comment:10 Changed 12 years ago by charles

  • Component changed from GTK+ Client to Transmission

comment:11 Changed 12 years ago by dfee

bandwidth is measured in bits not bytes. should be kbps (http://en.wikipedia.org/wiki/Kilobit) rather than kilobytes/sec.

e.g. my broadband connection is 10Mbps (megabits/sec).

comment:12 Changed 12 years ago by charles

That may be true of ISPs, but not of end-user apps like Transmission. Just as a quick example -- gnome-system-monitor, Deluge, and XChat all use bytes rather than bits.

Changed 11 years ago by charles

experimental patch

comment:13 Changed 11 years ago by charles

  • Milestone changed from None Set to 2.10
  • Summary changed from Upload/Download units of measuement displayed incorrectly to Size & speed units don't match desktop policies, and also is implemented independently in each app

Along with the issue of OS platform preferences (which OS X and Ubuntu both have, and those are probably are largest platforms), there's also the issue that all our different apps roll their own functions to display speed and sizes.

IMO this functionality is a good candidate for the "libtransmissionapp" that we've talked about in the past.

Changed 11 years ago by charles

Add the qt and command-line apps. Use the correct default values for Snow Leopard and Ubuntu.

comment:14 Changed 11 years ago by charles

A baby step for this was committed in r10818 and r10819. This was done to have a consistent size/speed display function for Transmission's applications. As a side-effect, it also makes the size/speed units easy to set at compile time.

The remaining -- and larger -- task is to handle the input fields in Options and the Preferences Dialogs, the internal units used by libtransmission, and the units used by the RPC interface. :/

comment:15 Changed 11 years ago by charles

  • Milestone changed from 2.10 to None Set

comment:16 Changed 11 years ago by charles

  • Milestone changed from None Set to 2.10

I was prepared to punt this indefinitely, but I think I've finally got a decent way of implementing this... very tentatively penciling in a 2.10 milestone

Changed 11 years ago by charles

Changed 11 years ago by charles

make r3 more backwards-friendly to settings.json

Changed 11 years ago by charles

updated diff

comment:17 Changed 11 years ago by charles

committed to trunk in r10931 and r10932. This affects libT, daemon, utils, qt, gtk, and web

comment:18 Changed 11 years ago by charles

r10937 -- apply changes suggested by BMW

comment:19 Changed 11 years ago by Rolcol

I think it's this ticket that messes with the cache size. Transmission was using almost 1GB of memory when I checked it and found out the cache setting was set to ~2 million megabytes.

comment:20 Changed 11 years ago by livings124

r10948 applies for the Mac client

comment:21 Changed 11 years ago by charles

r10955 and r10956 update the formatter functions based on feedback from BMW

Last edited 11 years ago by charles (previous) (diff)

comment:22 Changed 11 years ago by charles

#3440 reports that units are inconsistent between some of the Transmission end-user apps.

comment:23 Changed 11 years ago by livings124

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.