From 76f202106b7c9fb9be2994e8715a9f4fb54991b0 Mon Sep 17 00:00:00 2001 From: Albert Santoni Date: Fri, 30 Oct 2015 17:12:13 -0400 Subject: [PATCH 1/5] Defensive coding against Silan bugs and bump to Mutagen 1.31 --- .../airtime_analyzer/cuepoint_analyzer.py | 33 ++++++++++++++++--- .../airtime_analyzer/metadata_analyzer.py | 7 ++-- python_apps/airtime_analyzer/setup.py | 2 +- .../tests/metadata_analyzer_tests.py | 12 +++---- 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py index 8a99626c6..bf124ac18 100644 --- a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py +++ b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py @@ -28,12 +28,35 @@ class CuePointAnalyzer(Analyzer): try: results_json = subprocess.check_output(command, stderr=subprocess.STDOUT, close_fds=True) silan_results = json.loads(results_json) - metadata['length_seconds'] = float(silan_results['file duration']) + + # Defensive coding against Silan wildly miscalculating the cue in and out times: + silan_length_seconds = float(silan_results['file duration']) + silan_cuein = format(silan_results['sound'][0][0], 'f') + silan_cueout = format(silan_results['sound'][0][1], 'f') + + # Sanity check the results against any existing metadata passed to us (presumably extracted by Mutagen): + if 'length_seconds' in metadata: + # Silan has a rare bug where it can massively overestimate the length or cue out time sometimes. + if (silan_length_seconds - metadata['length_seconds'] > 3) or (float(silan_cueout) > metadata['length_seconds']): + # Don't trust anything silan says then... + raise Exception("Silan cue out {0} or length {1} differs too much from the mutagen length {2}." + .format(silan_cueout, silan_length_seconds, metadata['length_seconds'])) + # Don't allow silan to trim more than the greater of 3 seconds or 5% off the start of a track + if float(silan_cuein) > max(silan_length_seconds*0.05, 3): + raise Exception("Silan cue in time {0} too big, ignoring.".format(silan_cuein)) + + + ''' XXX: I've commented out the track_length stuff below because Mutagen seems more accurate than silan + as of Mutagen version 1.31. We are always going to use Mutagen's length now because Silan's + length can be off by a few seconds reasonably often. + ''' + # Conver the length into a formatted time string - track_length = datetime.timedelta(seconds=metadata['length_seconds']) - metadata["length"] = str(track_length) - metadata['cuein'] = format(silan_results['sound'][0][0], 'f') - metadata['cueout'] = format(silan_results['sound'][0][1], 'f') + #metadata['length_seconds'] = silan_length_seconds # + #track_length = datetime.timedelta(seconds=metadata['length_seconds']) + #metadata["length"] = str(track_length) + metadata['cuein'] = silan_cuein + metadata['cueout'] = silan_cueout except OSError as e: # silan was not found logging.warn("Failed to run: %s - %s. %s" % (command[0], e.strerror, "Do you have silan installed?")) diff --git a/python_apps/airtime_analyzer/airtime_analyzer/metadata_analyzer.py b/python_apps/airtime_analyzer/airtime_analyzer/metadata_analyzer.py index 58fc1d56e..61a4cbe25 100644 --- a/python_apps/airtime_analyzer/airtime_analyzer/metadata_analyzer.py +++ b/python_apps/airtime_analyzer/airtime_analyzer/metadata_analyzer.py @@ -67,8 +67,11 @@ class MetadataAnalyzer(Analyzer): track_length = datetime.timedelta(seconds=info.length) metadata["length"] = str(track_length) #time.strftime("%H:%M:%S.%f", track_length) # Other fields for Airtime - metadata["cueout"] = metadata["length"] - + metadata["cueout"] = metadata["length"] + + # Set a default cue in time in seconds + metadata["cuein"] = 0.0; + if hasattr(info, "bitrate"): metadata["bit_rate"] = info.bitrate diff --git a/python_apps/airtime_analyzer/setup.py b/python_apps/airtime_analyzer/setup.py index 47e231123..b2c643c48 100644 --- a/python_apps/airtime_analyzer/setup.py +++ b/python_apps/airtime_analyzer/setup.py @@ -27,7 +27,7 @@ setup(name='airtime_analyzer', packages=['airtime_analyzer'], scripts=['bin/airtime_analyzer'], install_requires=[ - 'mutagen', + 'mutagen=1.31', # The Mutagen guys change stuff all the time that break our unit tests. Watch out for this. 'pika', 'daemon', 'python-magic', diff --git a/python_apps/airtime_analyzer/tests/metadata_analyzer_tests.py b/python_apps/airtime_analyzer/tests/metadata_analyzer_tests.py index ee108b362..8a2d59967 100644 --- a/python_apps/airtime_analyzer/tests/metadata_analyzer_tests.py +++ b/python_apps/airtime_analyzer/tests/metadata_analyzer_tests.py @@ -24,7 +24,7 @@ def test_mp3_mono(): metadata = MetadataAnalyzer.analyze(u'tests/test_data/44100Hz-16bit-mono.mp3', dict()) check_default_metadata(metadata) assert metadata['channels'] == 1 - assert metadata['bit_rate'] == 64000 + assert metadata['bit_rate'] == 64876 assert abs(metadata['length_seconds'] - 3.9) < 0.1 assert metadata['mime'] == 'audio/mp3' # Not unicode because MIMEs aren't. assert metadata['track_total'] == u'10' # MP3s can have a track_total @@ -34,7 +34,7 @@ def test_mp3_jointstereo(): metadata = MetadataAnalyzer.analyze(u'tests/test_data/44100Hz-16bit-jointstereo.mp3', dict()) check_default_metadata(metadata) assert metadata['channels'] == 2 - assert metadata['bit_rate'] == 128000 + assert metadata['bit_rate'] == 129757 assert abs(metadata['length_seconds'] - 3.9) < 0.1 assert metadata['mime'] == 'audio/mp3' assert metadata['track_total'] == u'10' # MP3s can have a track_total @@ -43,7 +43,7 @@ def test_mp3_simplestereo(): metadata = MetadataAnalyzer.analyze(u'tests/test_data/44100Hz-16bit-simplestereo.mp3', dict()) check_default_metadata(metadata) assert metadata['channels'] == 2 - assert metadata['bit_rate'] == 128000 + assert metadata['bit_rate'] == 129757 assert abs(metadata['length_seconds'] - 3.9) < 0.1 assert metadata['mime'] == 'audio/mp3' assert metadata['track_total'] == u'10' # MP3s can have a track_total @@ -52,7 +52,7 @@ def test_mp3_dualmono(): metadata = MetadataAnalyzer.analyze(u'tests/test_data/44100Hz-16bit-dualmono.mp3', dict()) check_default_metadata(metadata) assert metadata['channels'] == 2 - assert metadata['bit_rate'] == 128000 + assert metadata['bit_rate'] == 129757 assert abs(metadata['length_seconds'] - 3.9) < 0.1 assert metadata['mime'] == 'audio/mp3' assert metadata['track_total'] == u'10' # MP3s can have a track_total @@ -109,7 +109,7 @@ def test_mp3_utf8(): assert metadata['genre'] == u'Я Б Г Д Ж Й' assert metadata['track_number'] == u'1' assert metadata['channels'] == 2 - assert metadata['bit_rate'] == 128000 + assert metadata['bit_rate'] == 129757 assert abs(metadata['length_seconds'] - 3.9) < 0.1 assert metadata['mime'] == 'audio/mp3' assert metadata['track_total'] == u'10' # MP3s can have a track_total @@ -153,7 +153,7 @@ def test_mp3_bad_channels(): metadata = MetadataAnalyzer.analyze(filename, dict()) check_default_metadata(metadata) assert metadata['channels'] == 1 - assert metadata['bit_rate'] == 64000 + assert metadata['bit_rate'] == 64876 assert abs(metadata['length_seconds'] - 3.9) < 0.1 assert metadata['mime'] == 'audio/mp3' # Not unicode because MIMEs aren't. assert metadata['track_total'] == u'10' # MP3s can have a track_total From fc51e6321461d4f2dd06e9ff447e44349c19bbd6 Mon Sep 17 00:00:00 2001 From: Albert Santoni Date: Fri, 30 Oct 2015 17:13:12 -0400 Subject: [PATCH 2/5] Fix typo in airtime_analyzer setup.py --- python_apps/airtime_analyzer/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_apps/airtime_analyzer/setup.py b/python_apps/airtime_analyzer/setup.py index b2c643c48..c9f33f45a 100644 --- a/python_apps/airtime_analyzer/setup.py +++ b/python_apps/airtime_analyzer/setup.py @@ -27,7 +27,7 @@ setup(name='airtime_analyzer', packages=['airtime_analyzer'], scripts=['bin/airtime_analyzer'], install_requires=[ - 'mutagen=1.31', # The Mutagen guys change stuff all the time that break our unit tests. Watch out for this. + 'mutagen==1.31', # The Mutagen guys change stuff all the time that break our unit tests. Watch out for this. 'pika', 'daemon', 'python-magic', From 7be548f30ea39166b827df91c8e69c0c6e2b2a53 Mon Sep 17 00:00:00 2001 From: Albert Santoni Date: Fri, 30 Oct 2015 17:15:33 -0400 Subject: [PATCH 3/5] Fixed the airtime_analyzer unit tests --- .../airtime_analyzer/cuepoint_analyzer.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py index bf124ac18..3c55d75e7 100644 --- a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py +++ b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py @@ -44,6 +44,13 @@ class CuePointAnalyzer(Analyzer): # Don't allow silan to trim more than the greater of 3 seconds or 5% off the start of a track if float(silan_cuein) > max(silan_length_seconds*0.05, 3): raise Exception("Silan cue in time {0} too big, ignoring.".format(silan_cuein)) + else: + # Only use the Silan track length in the worst case, where Mutagen didn't give us one for some reason. + # (This is mostly to make the unit tests still pass.) + # Convert the length into a formatted time string. + metadata['length_seconds'] = silan_length_seconds # + track_length = datetime.timedelta(seconds=metadata['length_seconds']) + metadata["length"] = str(track_length) ''' XXX: I've commented out the track_length stuff below because Mutagen seems more accurate than silan @@ -51,10 +58,6 @@ class CuePointAnalyzer(Analyzer): length can be off by a few seconds reasonably often. ''' - # Conver the length into a formatted time string - #metadata['length_seconds'] = silan_length_seconds # - #track_length = datetime.timedelta(seconds=metadata['length_seconds']) - #metadata["length"] = str(track_length) metadata['cuein'] = silan_cuein metadata['cueout'] = silan_cueout From b49bb2e2624fc0a1019db1fa05ca13f6373a56ab Mon Sep 17 00:00:00 2001 From: Albert Santoni Date: Fri, 30 Oct 2015 17:54:24 -0400 Subject: [PATCH 4/5] Loosen up Mutagen vs. Silan cue out length threshold a bit --- .../airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py index 3c55d75e7..e51b61ce2 100644 --- a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py +++ b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py @@ -37,7 +37,7 @@ class CuePointAnalyzer(Analyzer): # Sanity check the results against any existing metadata passed to us (presumably extracted by Mutagen): if 'length_seconds' in metadata: # Silan has a rare bug where it can massively overestimate the length or cue out time sometimes. - if (silan_length_seconds - metadata['length_seconds'] > 3) or (float(silan_cueout) > metadata['length_seconds']): + if (silan_length_seconds - metadata['length_seconds'] > 3) or (float(silan_cueout) - metadata['length_seconds'] > 2): # Don't trust anything silan says then... raise Exception("Silan cue out {0} or length {1} differs too much from the mutagen length {2}." .format(silan_cueout, silan_length_seconds, metadata['length_seconds'])) From c85944785bdff3abca8789cfd8a70a0966fab449 Mon Sep 17 00:00:00 2001 From: Albert Santoni Date: Fri, 30 Oct 2015 18:09:56 -0400 Subject: [PATCH 5/5] Comments --- .../airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py index e51b61ce2..47a517b59 100644 --- a/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py +++ b/python_apps/airtime_analyzer/airtime_analyzer/cuepoint_analyzer.py @@ -39,7 +39,7 @@ class CuePointAnalyzer(Analyzer): # Silan has a rare bug where it can massively overestimate the length or cue out time sometimes. if (silan_length_seconds - metadata['length_seconds'] > 3) or (float(silan_cueout) - metadata['length_seconds'] > 2): # Don't trust anything silan says then... - raise Exception("Silan cue out {0} or length {1} differs too much from the mutagen length {2}." + raise Exception("Silan cue out {0} or length {1} differs too much from the Mutagen length {2}. Ignoring Silan values." .format(silan_cueout, silan_length_seconds, metadata['length_seconds'])) # Don't allow silan to trim more than the greater of 3 seconds or 5% off the start of a track if float(silan_cuein) > max(silan_length_seconds*0.05, 3):