query-handbrake-log: Reading time & speed on logs created in Windows doesn't work in OS X

[lambdan]

I switch between Windows and OS X frequently, which means I do transcodes in both operating systems, and I noticed this bug when trying to analyze log files using query-handbrake-log afterwards.

In OS X:

djss-Mac-Pro:friends djs$ query-handbrake-log s friends\ s06e22.mkv.log 
25.886818 fps friends s06e22.mkv
djss-Mac-Pro:friends djs$ query-handbrake-log s friends\ s06e23.mkv.log 
/usr/local/bin/query-handbrake-log: fps not found: /Volumes/Green/mac_encodes/a/friends/friends s06e23.mkv.log

If I'm Windows it can read logs created in both OS X and Windows.

Checking the bitrate (b) works. It's just speed (s) and time (t) that is affected, because it needs the fps which it can't find?
I'm attaching two logs, one for OS X and one for Windows.
Archive.zip

[lisamelton]

@lambdan Thanks for testing this case and opening this issue. I can easily reproduce the problem with the files you supplied, too.

Your diagnosis and the error message is correct. The query-handbrake-log tool is failing on OS X because it can't find fps in the .log file generated on Windows. And I suspect that's because the regular expression I'm using to find it is somehow getting confused by the CRLF-style line endings on Windows.

Unfortunately, these line-ending bugs are trickier to fix in a cross-platform way than you would think. Stay tuned...

[lisamelton]

@lambdan OK, I have a patch which fixes the bug:

diff --git a/bin/query-handbrake-log b/bin/query-handbrake-log
index e8b8ba0..20f3985 100755
--- a/bin/query-handbrake-log
+++ b/bin/query-handbrake-log
@@ -145,7 +145,7 @@ HERE
               if fps_line.nil?
                 fps_line = line
               elsif fps_line_2.nil?
-                if fps_line =~ / ([0.]+) fps$/
+                if fps_line =~ / ([0.]+) fps\r?$/
                   fps_line = line
                 else
                   fps_line_2 = line
@@ -173,7 +173,7 @@ HERE
           else
             Console.debug fps_line

-            if fps_line =~ / ([0-9.]+) fps$/
+            if fps_line =~ / ([0-9.]+) fps\r?$/
               fps = $1
             else
               fail "fps not found: #{log}"
@@ -182,7 +182,7 @@ HERE
             unless fps_line_2.nil?
               Console.debug fps_line_2

-              if fps_line_2 =~ / ([0-9.]+) fps$/
+              if fps_line_2 =~ / ([0-9.]+) fps\r?$/
                 fps_2 = $1
                 fps = (1 / ((1 / fps.to_f) + (1 / fps_2.to_f))).round(6).to_s
               else

This works on OS X but, unfortunately, I don't have access to my Windows machine at the moment. Can you try this on OS X yourself but, especially, can you try this on Windows to make sure it doesn't break anything there?

Thanks!

[lambdan]

@donmelton Seems to be working:

I also tried it on OS X and it did work there aswell.

[lisamelton]

@lambdan Excellent! Thanks for testing that. Especially so quickly.

OK, I'll do my full regression test later this afternoon and queue this up for a 0.9.1 release. However, 0.9.1 probably won't happen until the end of the week since I just did the 0.9.0 release last night and I want to give other people time to test it in case I have any other regressions.

[lisamelton]

@lambdan I've decided to skip a 0.9.1 version because this will go in the same release as #68 which is a new feature. And that merits a bump to 0.10.0 instead. So this will probably go out sometime today.

[lisamelton]

@lambdan OK, version 0.10.0 was just released with this bug fix. Just gem update video_transcoding to get it. Thanks, again!

[lisamelton]

@lambdan D'oh! Thanks for closing that. I forgot. :)