Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

MBS-5765: Correctly render a duration in seconds as hours, minutes, seconds #15

Closed
wants to merge 2 commits into from

3 participants

Oliver Charles Ian McEwen Kuno Woudt
Oliver Charles
Owner

Time::Duration->normalize normalizes a large duration into years, months, and
days - rather than just the needed hours, minutes, seconds. Rather than manually
re-fudging the calculation to account for that overflow, I've just implemented
partioning of seconds in hours, minutes and seconds manually.

Oliver Charles MBS-5765: Correctly render a duration in seconds as hours, minutes, s…
…econds

Time::Duration->normalize normalizes a large duration into years, months, and
days - rather than just the needed hours, minutes, seconds. Rather than manually
re-fudging the calculation to account for that overflow, I've just implemented
partioning of seconds in hours, minutes and seconds manually.
63289a1
Ian McEwen ianmcorvidae commented on the diff
lib/MusicBrainz/Server/Track.pm
@@ -23,12 +23,18 @@ sub FormatTrackLength
23 23
     return $ms unless looks_like_number($ms);
24 24
     return "$ms ms" if $ms < 1000;
25 25
 
26  
-    my $seconds = $ms / 1000.0 + 0.5;
5
Ian McEwen Owner

If you're getting rid of this you need to add 500 to the $ms number to keep this consistent.

Oliver Charles Owner
ocharles added a note

Ok, it was unclear why that was even there - 7fcda88 isn't much help on why it's there either.

Ian McEwen Owner

Ah, okay. The reason is that using floor() (as we do, here and in the JS version of this function both) means that something like 3700ms would get rounded to 3 seconds, when it's actually closer to 4 seconds. So, by adding 500ms (or 0.5s) to the number, floor() ends up working like you'd expect with millisecond values between 500 and 999 above a given thousand.

Oliver Charles Owner
ocharles added a note

Oh, so maybe it would be better to use round, which seems to be what we want?

Kuno Woudt Owner
warpr added a note

It does seem that round is what we want :)
Though if you change that here I would prefer you also change the implementation of the javascript equivalent (in common/MB/utility.js).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Ian McEwen
Owner

Looks good to me, so long as tests still pass. :ship: if so!

Oliver Charles ocharles closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Jan 22, 2013
Oliver Charles MBS-5765: Correctly render a duration in seconds as hours, minutes, s…
…econds

Time::Duration->normalize normalizes a large duration into years, months, and
days - rather than just the needed hours, minutes, seconds. Rather than manually
re-fudging the calculation to account for that overflow, I've just implemented
partioning of seconds in hours, minutes and seconds manually.
63289a1
Jan 29, 2013
Oliver Charles MBS-5765: Use the same implementation of FormatTrackLength (Perl) in …
…formatTrackLength (JS)
c28ab68
This page is out of date. Refresh to see the latest.
18  lib/MusicBrainz/Server/Track.pm
... ...
@@ -1,7 +1,7 @@
1 1
 package MusicBrainz::Server::Track;
2 2
 use strict;
3 3
 use Carp 'confess';
4  
-use aliased 'DateTime::Format::Duration';
  4
+use POSIX qw( floor );
5 5
 use Scalar::Util qw( looks_like_number );
6 6
 
7 7
 use Sub::Exporter -setup => {
@@ -23,12 +23,18 @@ sub FormatTrackLength
23 23
     return $ms unless looks_like_number($ms);
24 24
     return "$ms ms" if $ms < 1000;
25 25
 
26  
-    my $seconds = $ms / 1000.0 + 0.5;
  26
+    my $one_second = 1000.0;
  27
+    my $one_minute = $one_second * 60;
  28
+    my $one_hour = $one_minute * 60;
27 29
 
28  
-    my %data = Duration->new->normalise (seconds => $seconds);
29  
-    return $data{hours} > 0 ?
30  
-        sprintf ("%d:%02d:%02d", $data{hours}, $data{minutes}, $data{seconds}) :
31  
-        sprintf ("%d:%02d", $data{minutes}, $data{seconds});
  30
+    my ($hours, $minutes, $seconds);
  31
+    ($hours, $ms) = (floor($ms / $one_hour), $ms % $one_hour);
  32
+    ($minutes, $ms) = (floor($ms / $one_minute), $ms % $one_minute);
  33
+    $seconds = round($ms / $one_second);
  34
+
  35
+    return $hours > 0 ?
  36
+        sprintf ("%d:%02d:%02d", $hours, $minutes, $seconds) :
  37
+        sprintf ("%d:%02d", $minutes, $seconds);
32 38
 }
33 39
 
34 40
 sub FormatXSDTrackLength
34  root/static/scripts/common/MB/utility.js
@@ -197,29 +197,29 @@ MB.utility.formatTrackLength = function (duration)
197 197
         return duration + ' ms';
198 198
     }
199 199
 
200  
-    var seconds = 1000;
201  
-    var minutes = 60 * seconds;
202  
-    var hours = 60 * minutes;
  200
+    var one_second = 1000.0;
  201
+    var one_minute = 60 * one_second;
  202
+    var one_hour = 60 * one_minute;
203 203
 
204  
-    var hours_str = '';
205  
-    duration = duration + 500;
  204
+    var hours = Math.floor(duration / one_hour);
  205
+    duration = duration % one_hour;
206 206
 
207  
-    if (duration > 1 * hours)
208  
-    {
209  
-        hours_str = Math.floor (duration / hours) + ':';
210  
-        duration = Math.floor (duration % hours);
211  
-    }
  207
+    var minutes = Math.floor(duration / one_minute);
  208
+    duration = duration % one_minute;
212 209
 
213  
-    /* pad minutes with zeroes of the hours string is non-empty. */
214  
-    var minutes_str = hours_str === '' ?
215  
-        Math.floor (duration / minutes) + ':' :
216  
-        ('00' + Math.floor (duration / minutes)).slice (-2) + ':';
  210
+    var seconds = Math.round(duration / one_second);
217 211
 
218  
-    duration = Math.floor (duration % minutes);
  212
+    var ret = '';
  213
+    ret = ('00' + seconds).slice(-2);
219 214
 
220  
-    var seconds_str = ('00' + Math.floor (duration / seconds)).slice (-2);
  215
+    if (hours > 0) {
  216
+        ret = hours + ':' + ('00' + minutes).slice(-2) + ':' + ret;
  217
+    }
  218
+    else {
  219
+        ret = minutes + ':' + ret;
  220
+    }
221 221
 
222  
-    return hours_str + minutes_str + seconds_str;
  222
+    return ret;
223 223
 };
224 224
 
225 225
 
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.