transmission measures bandwidth in KB/s instead of kB/s

Bug #538504 reported by Benjamin Drung
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Transmission
Fix Released
Unknown
transmission (Ubuntu)
Fix Released
Low
Unassigned

Bug Description

Binary package hint: transmission

transmission measures bandwidth in KB/s. Is it in base-2 or base-10? It should use kB/s in base-10 instead following the units policy [1].

[1] https://wiki.ubuntu.com/UnitsPolicy

Tags: units-policy

Related branches

Revision history for this message
Jason J. Herne (hernejj) wrote :

Transmission does indeed use units names KB (Which is indeed against the Ubuntu Units Policy). 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

I'm attaching a screen shot that shows the four places highlighted in Red.

Daniel T Chen (crimsun)
Changed in transmission (Ubuntu):
importance: Undecided → Low
status: New → Confirmed
Daniel T Chen (crimsun)
Changed in transmission (Ubuntu):
status: Confirmed → Triaged
Changed in transmission:
status: Unknown → New
Revision history for this message
Charles Kerr (charlesk) wrote :

    *

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

affects: transmission (Ubuntu) → glib2.0 (Ubuntu)
Changed in glib2.0 (Ubuntu):
status: Triaged → New
Revision history for this message
Benjamin Drung (bdrung) wrote :

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.

Changed in glib2.0 (Ubuntu):
status: New → Triaged
affects: glib2.0 (Ubuntu) → transmission (Ubuntu)
Revision history for this message
Krzysztof Klimonda (kklimonda) wrote :

it should use kB/s instead, right?

Revision history for this message
Benjamin Drung (bdrung) wrote :

Yes, it should be kB/s (small k) and it should be measured in base-10.

Revision history for this message
Krzysztof Klimonda (kklimonda) wrote :

This change is going to require UI Freeze Exception and help from translators. Subscribing release-team. Should I also subscribe translations team and documentation? What does "notification to (those teams) mean in this context?

summary: - transmission measures bandwidth in KB/s instead of kB/s
+ [UIFe] transmission measures bandwidth in KB/s instead of kB/s
Revision history for this message
Scott Kitterman (kitterman) wrote : Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

Is this going to be accepted upstream? I think it would be awkward for Transmission to display differently on Ubuntu than other distros.

Revision history for this message
Benjamin Drung (bdrung) wrote :

We will differ because of our units policy [1] anyway. Upstream could use g_format_size_for_display() and append "/s", then it would be consistent.

[1] https://wiki.ubuntu.com/UnitsPolicy

Revision history for this message
Scott Kitterman (kitterman) wrote : Re: [Bug 538504] Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

This seems like a nit to me and not worth a freeze exception.

Revision history for this message
Charles Kerr (charlesk) wrote : Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

g_format_size_for_display() + "/s" is sufficient for displaying output.

When prompting the user for input though, things are more complicated. For example if user enters in a speed cap of 100 KB/s in Transmission 1.75 in Ubuntu 9.10, and then upgrades to Transmission 1.93 on Ubuntu 10.04, should the preference dialog read that the speed is capped at 1.024 kB? Or if that's overkill and we just round it off at whole numbers, do we silently update the cap in the user's settings from 1024 bytes to 1000 bytes?

Revision history for this message
Charles Kerr (charlesk) wrote :

> Is this going to be accepted upstream? I think it would be awkward for Transmission to display differently on Ubuntu than other distros.

This is a very good question. I think the key is going to be, are other distros going to follow this policy. Using different units on different platforms is untenable, but changing all of transmission-gtk's users preferences to change subtly when upgrading to 1.93 is undesirable.

How are other applications in similar situations addressing this issue?

Revision history for this message
Charles Kerr (charlesk) wrote :

> This seems like a nit to me and not worth a freeze exception.

Strongly agree. Displaying speeds is fairly easy to do, but prompting the user for an input speed will require code changes which aren't clear to me yet. In general the risk for doing this for 10.04 seems much higher than the reward.

Revision history for this message
Charles Kerr (charlesk) wrote :

Reverting from "Triaged" for a second time.

Triaged implies there's a clear fix. Don't mark this bug as triaged unless you know what The Right Thing to do is with users' settings that were entered in base 2 in older versions of Transmission.

Changed in transmission (Ubuntu):
status: Triaged → Confirmed
Revision history for this message
Charles Kerr (charlesk) wrote :

I've opened a ticket regarding Ubuntu's downstream changes to g_format_size_for_display():

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

Revision history for this message
Benjamin Drung (bdrung) wrote :

I think this bug is worth a freeze exception. We try to increase the consistency of the desktop in Ubuntu. Leaving some parts unpatched will lead to more user confusion than the policy will reduce it - especial when one application use base-10 and base-2 both with SI prefixes.

Changing the input unit from KiB to kB will decrease the value by 2.3 % if you do not adjust the value. In my opinion a conversion of the user's settings from 1024 to 1000 is not required. What do you think?

The Technical Board decided on the units policy. This will lead to inconsistency between Ubuntu and other distributions, but will increase the consistency in Ubuntu. I hope that other distributions will follow Ubuntu or at least stop using SI prefixes for base-2.

Using g_format_size_for_display() + "/s" will lead to a consistent application on the platform - independent from the used binary.

Revision history for this message
Martin Pitt (pitti) wrote : Re: [Bug 538504] Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

Benjamin Drung [2010-03-14 15:00 -0000]:
> We will differ because of our units policy [1] anyway. Upstream could
> use g_format_size_for_display() and append "/s", then it would be
> consistent.

Please don't. It's not quite clear yet whether we'll keep the
g_format_size_for_display() patch or rather fix only nautilus
(upstream is still arguing whether our patch is an ABI break).

Network speed has never _ever_ been measured in base-2 units, so this
fix seems entirely appropriate to me.

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

Revision history for this message
Benjamin Drung (bdrung) wrote : Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

I'm now realizing the problem of g_format_size_for_display() + "/s". I have a new approach to that.

What upstream can do: Use bytes internally and write/use some helper functions for inputting/displaying them:
* bandwidth_unit(): Return a string containing the bandwidth unit (e.g. "KB/s")
* input_bandwidth_to_byte(size): Converts the input value to bytes (e.g. size * 1024)

Then we can write a small patch for Ubuntu that modifies these two functions:
* bandwidth_unit(): Return "kB/s"
* input_bandwidth_to_byte(size): Return size * 1000

Anything that I forgot?

@Martin: If we drop the g_format_size_for_display() patch, we have to patch all applications that use this function to comply with our policy.

Revision history for this message
Charles Kerr (charlesk) wrote :

Transmission has a long history of working closely with Ubuntu -- for two examples, see the upstream integration of libappindicator, and see how many Transmission tickets I've triaged in launchpad.

However, the libappindicator patch was completed a month ago. Why was this ticket filed after the feature freeze, after the UI freeze, after the beta 1 freeze? Why should Transmission introduce new code -- code which behaves differently on different systems, no less, so that'll be a joy to validate -- during the freeze of an LTS release?

That said, one of Transmission's main goals is to work seamlessly on all platforms. If this really is widely accepted Ubuntu policy -- and by that I mean not that two people wrote a UnitPolicy wiki page, but rather that a wider group of peers have seen and understood the problems generated by forking g_format_size_for_display(), etc. and still want to go through with this -- then I'll happily consider a patch for inclusion.

These are the technical constraints I would have to follow for such a patch, so I request that you follow the same constraints:

  1. The units *must* be consistent between Transmission's input fields and display output
  2. The units *must* be consistent between Transmission and the platform it's running on

These have been discussed upthread, and also in <https://bugs.launchpad.net/ubuntu/+source/glib2.0/+bug/538783>. Without these two, there's no point in patching. Now for the possible pitfalls:

  3. It *should* not break backwards compatibility with settings.json's base 2 fields. If it does so, the other applications that use settings.json (transmission-daemon, transmision-cli, transmission-qt) will need to be reviewed for ripple effects.
  4. It *must* not break the behavior of RPC/JSON's base 2 fields. There are too many third-party apps to review.
  5. It *should* not change libtransmission's interface wrt settings.json fields passed directly into tr_sessionInit(), tr_sessionSetSpeedLimit(), etc. If libtransmission's API and/or its handling of configuration key/value pairs is changed, then other applications which use libtransmission directly, particularly the Mac OS X client, will need to be reviewed for ripple effects.
 6. It *must* not add new library dependencies (such as, for example, transmission-daemon linking against glib to pick up g_format_size_for_display()) that would hamstring Transmission's viability on routers and embedded systems.
 7. It must be tested and not introduce new bugs.

If you're up for that, please be my guest.

Revision history for this message
eris23 (jdkatz23) wrote :

There should be a preferences option to select KiB or kB. That just leaves the problem of deciding the default. Maybe suggest this to upstream?

Revision history for this message
Charles Kerr (charlesk) wrote :

And what should the default be...? ;)

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Adding a preference for something because we can't decide on what to do is virtually always never the right answer, especially for something like this.

Revision history for this message
Benjamin Drung (bdrung) wrote :

The bug was filed so late in the cycle, because the policy was recently approved.

I agree with point one and two.

3. We could introduce new setting, which store the values in bytes. For example, download-limit-bytes could replace download-limit.
4. I don't get this point. RPC/JSON? third-party apps? A little more explanation is required.
5. Instead of changing tr_sessionSetSpeedLimit(), a new function should be introduces to change the speed in bytes (for example tr_sessionSetSpeedLimitBytes). The old functions will then still work.
6. That's doable.
7. Of course.

Having a preferences option to select KiB or kB would be nice, but then g_format_size_for_display() will be useless. The default should be changeable with a configure option, because Ubuntu has to default to base-10 (refer to units policy).

Revision history for this message
Scott Kitterman (kitterman) wrote : Re: [Bug 538504] Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

Sounds like lucid+1 material to me.

Revision history for this message
Charles Kerr (charlesk) wrote : Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

> 3. We could introduce new setting, which store the values in bytes. For example, download-limit-bytes could replace download-limit.

This is a pretty good idea.

> 4. I don't get this point. RPC/JSON? third-party apps? A little more explanation is required.

There are a lot of third-party applications that can act as "remote controls" for Transmission... there's a Web GUI, two Windows GUIs, a Java GUI, ones for iPhones and Android phones, and on, and on. The important part here is that we need to not change the JSON-formatted RPC API they use to talk to Transmission.

> 5. Instead of changing tr_sessionSetSpeedLimit(), a new function should be introduces to change the speed in bytes (for example tr_sessionSetSpeedLimitBytes). The old functions will then still work.

This is a good idea. I don't see any downside to this. :)

Revision history for this message
Charles Kerr (charlesk) wrote :

> This is a good idea. I don't see any downside to this. :)

Rereading my text in my inbox this morning, I realized I should clarify this: I like Benjamin's suggestions for what the patch should look like, but the patch is still unnecessary risk during an LTS freeze. My preference would be that this gets done, and gets done after Lucid.

Revision history for this message
Benjamin Drung (bdrung) wrote :

If the required changes to transmission are too huge for lucid, we have to change the strings from "KB/s" to "KiB/s".

Revision history for this message
Scott Kitterman (kitterman) wrote : Re: [Bug 538504] Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

No. We don't have to do anything. I don't think nits like this are worth breaking a freeze for.

Revision history for this message
Charles Kerr (charlesk) wrote : Re: [UIFe] transmission measures bandwidth in KB/s instead of kB/s

> If the required changes to transmission are too huge for lucid,

It's not really about the size of the change, it's about the unnecessary risk during an LTS freeze. (IMO.) If a serious bug is found in Transmission in the middle of April, I guarantee you'll find me here in Launchpad trying to push the fix into Lucid. :)

> [then] we have to change the strings from "KB/s" to "KiB/s".

Even this change -- which is easy, safe, and (IMO) good -- would also make Transmission less compatible with most distributions. That's why I didn't do it six months ago, despite my personal preference to use IEC notation.

And that's exactly why it's better to do this upstream. GNOME apps should not have to "take sides" in this tempest-in-a-teapot, nor should they each have to invent their own units helper functions. After the last years' worth of flamewars, you surely understand how many people feel strongly about this on both sides. It would be advantageous to have an upstream glib API to handle this issue to everyone's satisfaction.

Revision history for this message
Benjamin Drung (bdrung) wrote :

Let's stall this bug until we have a solution or at least a roadmap for glib (bug #369525).

To ease future modifications, transmission should work internally in bytes (or bits). According to the responses, this should target lucid+1.

Revision history for this message
Charles Kerr (charlesk) wrote :

Lest anyone think all I can do on this issue is complain, I've attached a (very) rough draft units implementation for the GTK+ version of Transmission in the upstream ticket, along with a todo list of changes that will need to be made to libtransmission. :)

Revision history for this message
Steve Langasek (vorlon) wrote :

UIFe has been rejected by the release team, so unsubscribing ubuntu-release and dropping "UIFe" from the bug title since this is now Lucid+1.

summary: - [UIFe] transmission measures bandwidth in KB/s instead of kB/s
+ transmission measures bandwidth in KB/s instead of kB/s
Changed in transmission:
status: New → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package transmission - 2.11-0ubuntu1

---------------
transmission (2.11-0ubuntu1) natty; urgency=low

  * new upstream release (LP: #663396)
  * Merged 2.10-2 from Debian experimental, remaining changes:
    - debian/patches/series:
      - keep 0102-lpi.patch and 0103-add_X_Ubuntu_Gettext_Domain.patch
    - debian/rules:
      - enable support for libcanberra-gtk and libgconf
      - create translation templates
      - enable dh_autoreconf support
    - debian/control:
      - build-depend on libcanberra-gtk-dev, libgconf2-dev,
        liblaunchpad-integration-dev, libappindicator-dev,
        dh-autoreconf
      - update maintainer to "Ubuntu Developers"
  * debian/patches/series:
    - 0101-fix_dso_linking.patch: Fix FTBFS created by changes made to the
      natty toolchain.
  * Closes bugs:
    - disk read/write scheduling (LP: #567181)
    - torrent properties window does not fit netbook screen (LP: #634557)
    - transmission measures bandwidth in KB/s instead of kB/s (LP: #538504)
 -- Krzysztof Klimonda <email address hidden> Tue, 19 Oct 2010 20:30:31 +0200

Changed in transmission (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.