From 4c086b938112203336fcda2c34483229e8d24a7e Mon Sep 17 00:00:00 2001 From: Tom Wallroth Date: Fri, 30 Sep 2022 09:46:08 +0200 Subject: [PATCH 1/3] reworked tests, added mp4 fields --- tinytag/tests/test_all.py | 86 +++++++++++++++++++++++++-------------- tinytag/tinytag.py | 19 ++++++--- 2 files changed, 68 insertions(+), 37 deletions(-) diff --git a/tinytag/tests/test_all.py b/tinytag/tests/test_all.py index 0916ce7..540a40f 100644 --- a/tinytag/tests/test_all.py +++ b/tinytag/tests/test_all.py @@ -11,6 +11,7 @@ from __future__ import unicode_literals import io +import operator import os import re import shutil @@ -197,21 +198,21 @@ # OGG ('samples/empty.ogg', - {'extra': {}, 'duration': 3.684716553287982, '_max_samplenum': 162496, - '_tags_parsed': False, 'filesize': 4328, 'audio_offset': 0, 'bitrate': 112.0, + {'extra': {}, 'duration': 3.684716553287982, + 'filesize': 4328, 'audio_offset': 0, 'bitrate': 112.0, 'samplerate': 44100}), ('samples/multipagecomment.ogg', - {'extra': {}, 'duration': 3.684716553287982, '_max_samplenum': 162496, - '_tags_parsed': False, 'filesize': 135694, 'audio_offset': 0, 'bitrate': 112, + {'extra': {}, 'duration': 3.684716553287982, + 'filesize': 135694, 'audio_offset': 0, 'bitrate': 112, 'samplerate': 44100}), ('samples/multipage-setup.ogg', {'extra': {}, 'genre': 'JRock', 'duration': 4.128798185941043, 'album': 'Timeless', 'year': '2006', 'title': 'Burst', 'artist': 'UVERworld', 'track': '7', - '_tags_parsed': False, 'filesize': 76983, 'audio_offset': 0, 'bitrate': 160.0, + 'filesize': 76983, 'audio_offset': 0, 'bitrate': 160.0, 'samplerate': 44100}), ('samples/test.ogg', {'extra': {}, 'duration': 1.0, 'album': 'the boss', 'year': '2006', - 'title': 'the boss', 'artist': 'james brown', 'track': '1', '_tags_parsed': False, + 'title': 'the boss', 'artist': 'james brown', 'track': '1', 'filesize': 7467, 'audio_offset': 0, 'bitrate': 160.0, 'samplerate': 44100, 'comment': 'hello!'}), ('samples/corrupt_metadata.ogg', @@ -343,7 +344,7 @@ 'genre': 'Pop', 'year': '2011', 'title': 'Nothing', 'album': 'Only Our Hearts To Lose', 'track_total': '11', 'track': '11', 'artist': 'Marian', 'filesize': 61432}), ('samples/test2.m4a', - {'extra': {}, 'bitrate': 256.0, 'track': '1', + {'extra': {'copyright': '℗ 1992 Ace Records'}, 'bitrate': 256.0, 'track': '1', 'albumartist': "Millie Jackson - Get It Out 'cha System - 1978", 'duration': 167.78739229024944, 'filesize': 223365, 'channels': 2, 'year': '1978', 'artist': 'Millie Jackson', 'track_total': '9', 'disc_total': '1', 'genre': 'R&B/Soul', @@ -359,7 +360,8 @@ 'albumartist': 'Major Lazer', 'channels': 2, 'bitrate': 125.584, 'comment': '? 2016 Mad Decent'}), ('samples/alac_file.m4a', - {'extra': {}, 'artist': 'Howard Shelley', 'composer': 'Clementi, Muzio (1752-1832)', + {'extra': {'copyright': '© Hyperion Records Ltd, London', 'lyrics': 'Album notes:'}, + 'artist': 'Howard Shelley', 'composer': 'Clementi, Muzio (1752-1832)', 'filesize': 20000, 'title': 'Clementi: Piano Sonata in D major, Op 25 No 6 - Movement 2: Un poco andante', 'album': 'Clementi: The Complete Piano Sonatas, Vol. 4', 'year': '2009', 'track': '14', @@ -414,42 +416,64 @@ if match: expected_values[fieldname] = _type(match[0]) if expected_values: - expected_values['__do_not_require_all_values'] = True + expected_values['_do_not_require_all_values'] = True testfiles[os.path.join('custom_samples', filename)] = expected_values else: # if there are no expected values, just try parsing the file testfiles[os.path.join('custom_samples', filename)] = {} +def almost_equal_float(val1, val2): + # allow duration to be off by 100 ms and a maximum of 1% + if val1 == val2: + return True + if abs(val1 - val2) < 0.100: + if val2 and min(val1, val2) / max(val1, val2) > 0.99: + return True + return False + + +def startswith(val1, val2): + return val1.startswith(val2) + + +def compare(results, expected, filename, prev_path=None): + assert isinstance(results, dict) + missing_keys = set(expected.keys()) - set(results) + assert not missing_keys, 'Missing data in fixture \n%s' % str(missing_keys) + + for key, result_val in results.items(): + path = prev_path + '.' + key if prev_path else key + try: + expected_val = expected[key] + except KeyError: + assert False, 'Missing field "%s" with value "%s" in fixture in "%s"!' % (key, result_val, filename) + # recurse if the result and expected values are a dict: + if isinstance(result_val, dict) and isinstance(expected_val, dict): + compare(result_val, expected_val, filename, prev_path=key) + else: + fmt_string = 'field "%s": got %s (%s) expected %s (%s) in %s!' + fmt_values = (key, repr(result_val), type(result_val), repr(expected_val), type(expected_val), filename) + op = operator.eq + if path == 'duration': # allow duration to be off by 100 ms and a maximum of 1% + op = almost_equal_float + if path == 'extra.lyrics': # lets not copy *all* the lyrics inside the fixture + op = startswith + assert op(result_val, expected_val), fmt_string % fmt_values + + @pytest.mark.parametrize("testfile,expected", [ pytest.param(testfile, expected) for testfile, expected in testfiles.items() ]) def test_file_reading(testfile, expected): filename = os.path.join(testfolder, testfile) tag = TinyTag.get(filename) + results = { + key: val for key, val in tag.__dict__.items() + if not key.startswith('_') and val is not None + } + compare(results, expected, filename) - for key, expected_val in expected.items(): - if key.startswith('__'): - continue - result = getattr(tag, key) - fmt_string = 'field "%s": got %s (%s) expected %s (%s)!' - fmt_values = (key, repr(result), type(result), repr(expected_val), type(expected_val)) - if key == 'duration' and result is not None and expected_val is not None: - # allow duration to be off by 100 ms and a maximum of 1% - if abs(result - expected_val) < 0.100: - if expected_val and min(result, expected_val) / max(result, expected_val) > 0.99: - continue - assert result == expected_val, fmt_string % fmt_values - # for custom samples, allow not specifying all values - if expected.get('_do_not_require_all_values'): - return - undefined_in_fixture = {} - for key, val in tag.__dict__.items(): - if key.startswith('_') or val is None: - continue - if key not in expected: - undefined_in_fixture[key] = val - assert not undefined_in_fixture, 'Missing data in fixture \n%s' % str(undefined_in_fixture) # # def test_generator(): # for testfile, expected in testfiles.items(): diff --git a/tinytag/tinytag.py b/tinytag/tinytag.py index 738d502..be6b4e5 100644 --- a/tinytag/tinytag.py +++ b/tinytag/tinytag.py @@ -401,18 +401,25 @@ def debug_atom(cls, data): # callables return {fieldname: value} which is updates the TinyTag. META_DATA_TREE = {b'moov': {b'udta': {b'meta': {b'ilst': { # see: http://atomicparsley.sourceforge.net/mpeg-4files.html - b'\xa9alb': {b'data': Parser.make_data_atom_parser('album')}, + # and: https://metacpan.org/dist/Image-ExifTool/source/lib/Image/ExifTool/QuickTime.pm#L3093 b'\xa9ART': {b'data': Parser.make_data_atom_parser('artist')}, - b'aART': {b'data': Parser.make_data_atom_parser('albumartist')}, - # b'cpil': {b'data': Parser.make_data_atom_parser('compilation')}, + b'\xa9alb': {b'data': Parser.make_data_atom_parser('album')}, b'\xa9cmt': {b'data': Parser.make_data_atom_parser('comment')}, - b'disk': {b'data': Parser.make_number_parser('disc', 'disc_total')}, - b'\xa9wrt': {b'data': Parser.make_data_atom_parser('composer')}, + # b'cpil': {b'data': Parser.make_data_atom_parser('extra.compilation')}, # need test-data for this b'\xa9day': {b'data': Parser.make_data_atom_parser('year')}, + # b'\xa9des': {b'data': Parser.make_data_atom_parser('description')}, # need test-data for this b'\xa9gen': {b'data': Parser.make_data_atom_parser('genre')}, - b'gnre': {b'data': Parser.parse_id3v1_genre}, + b'\xa9lyr': {b'data': Parser.make_data_atom_parser('extra.lyrics')}, + b'\xa9mvn': {b'data': Parser.make_data_atom_parser('movement')}, b'\xa9nam': {b'data': Parser.make_data_atom_parser('title')}, + b'\xa9wrt': {b'data': Parser.make_data_atom_parser('composer')}, + b'aART': {b'data': Parser.make_data_atom_parser('albumartist')}, + b'cprt': {b'data': Parser.make_data_atom_parser('extra.copyright')}, + # b'desc': {b'data': Parser.make_data_atom_parser('extra.description')}, # need test-data for this + b'disk': {b'data': Parser.make_number_parser('disc', 'disc_total')}, + b'gnre': {b'data': Parser.parse_id3v1_genre}, b'trkn': {b'data': Parser.make_number_parser('track', 'track_total')}, + # b'tmpo': {b'data': Parser.make_data_atom_parser('extra.bmp')}, # need test-data for this }}}}} # see: https://developer.apple.com/library/mac/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html From cd56688084b99981669885ec5707dd4afd0ce648 Mon Sep 17 00:00:00 2001 From: Tom Wallroth Date: Fri, 30 Sep 2022 10:13:29 +0200 Subject: [PATCH 2/3] refactored some more test code --- tinytag/tests/test_all.py | 79 +++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/tinytag/tests/test_all.py b/tinytag/tests/test_all.py index 540a40f..9425593 100644 --- a/tinytag/tests/test_all.py +++ b/tinytag/tests/test_all.py @@ -394,33 +394,38 @@ testfolder = os.path.join(os.path.dirname(__file__)) -# load custom samples -custom_samples_folder = os.path.join(testfolder, 'custom_samples') -pattern_field_name_type = [ - (r'sr=(\d+)', 'samplerate', int), - (r'dn=(\d+)', 'disc', str), - (r'dt=(\d+)', 'disc_total', str), - (r'd=(\d+.?\d*)', 'duration', float), - (r'b=(\d+)', 'bitrate', int), - (r'c=(\d)', 'channels', int), - (r'genre="([^"]+)"', 'genre', str), -] -for filename in os.listdir(custom_samples_folder): - if filename == 'instructions.txt': - continue - if os.path.isdir(os.path.join(custom_samples_folder, filename)): - continue - expected_values = {} - for pattern, fieldname, _type in pattern_field_name_type: - match = re.findall(pattern, filename) - if match: - expected_values[fieldname] = _type(match[0]) - if expected_values: - expected_values['_do_not_require_all_values'] = True - testfiles[os.path.join('custom_samples', filename)] = expected_values - else: - # if there are no expected values, just try parsing the file - testfiles[os.path.join('custom_samples', filename)] = {} +def load_custom_samples(): + retval = {} + custom_samples_folder = os.path.join(testfolder, 'custom_samples') + pattern_field_name_type = [ + (r'sr=(\d+)', 'samplerate', int), + (r'dn=(\d+)', 'disc', str), + (r'dt=(\d+)', 'disc_total', str), + (r'd=(\d+.?\d*)', 'duration', float), + (r'b=(\d+)', 'bitrate', int), + (r'c=(\d)', 'channels', int), + (r'genre="([^"]+)"', 'genre', str), + ] + for filename in os.listdir(custom_samples_folder): + if filename == 'instructions.txt': + continue + if os.path.isdir(os.path.join(custom_samples_folder, filename)): + continue + expected_values = {} + for pattern, fieldname, _type in pattern_field_name_type: + match = re.findall(pattern, filename) + if match: + expected_values[fieldname] = _type(match[0]) + if expected_values: + expected_values['_do_not_require_all_values'] = True + retval[os.path.join('custom_samples', filename)] = expected_values + else: + # if there are no expected values, just try parsing the file + retval[os.path.join('custom_samples', filename)] = {} + return retval + + +testfiles.update(load_custom_samples()) def almost_equal_float(val1, val2): @@ -437,7 +442,11 @@ def startswith(val1, val2): return val1.startswith(val2) -def compare(results, expected, filename, prev_path=None): +def error_fmt(value): + return '%s (%s)' % (repr(value), type(value)) + + +def compare(results, expected, file, prev_path=None): assert isinstance(results, dict) missing_keys = set(expected.keys()) - set(results) assert not missing_keys, 'Missing data in fixture \n%s' % str(missing_keys) @@ -447,13 +456,14 @@ def compare(results, expected, filename, prev_path=None): try: expected_val = expected[key] except KeyError: - assert False, 'Missing field "%s" with value "%s" in fixture in "%s"!' % (key, result_val, filename) + assert False, 'Missing field "%s": "%s" in fixture "%s"!' % ( + key, error_fmt(result_val), file) # recurse if the result and expected values are a dict: if isinstance(result_val, dict) and isinstance(expected_val, dict): - compare(result_val, expected_val, filename, prev_path=key) + compare(result_val, expected_val, file, prev_path=key) else: - fmt_string = 'field "%s": got %s (%s) expected %s (%s) in %s!' - fmt_values = (key, repr(result_val), type(result_val), repr(expected_val), type(expected_val), filename) + fmt_string = 'field "%s": got %s expected %s in %s!' + fmt_values = (key, error_fmt(result_val), error_fmt(expected_val), file) op = operator.eq if path == 'duration': # allow duration to be off by 100 ms and a maximum of 1% op = almost_equal_float @@ -474,11 +484,6 @@ def test_file_reading(testfile, expected): } compare(results, expected, filename) -# -# def test_generator(): -# for testfile, expected in testfiles.items(): -# yield get_info, testfile, expected - def test_pathlib_compatibility(): try: From a34ec36d37a09e96844782d5352f12cdb83aad6b Mon Sep 17 00:00:00 2001 From: Tom Wallroth Date: Fri, 30 Sep 2022 10:44:14 +0200 Subject: [PATCH 3/3] flake8'd --- tinytag/tests/test_all.py | 1 + tinytag/tinytag.py | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tinytag/tests/test_all.py b/tinytag/tests/test_all.py index 9425593..4f12906 100644 --- a/tinytag/tests/test_all.py +++ b/tinytag/tests/test_all.py @@ -394,6 +394,7 @@ testfolder = os.path.join(os.path.dirname(__file__)) + def load_custom_samples(): retval = {} custom_samples_folder = os.path.join(testfolder, 'custom_samples') diff --git a/tinytag/tinytag.py b/tinytag/tinytag.py index be6b4e5..4150480 100644 --- a/tinytag/tinytag.py +++ b/tinytag/tinytag.py @@ -405,9 +405,11 @@ def debug_atom(cls, data): b'\xa9ART': {b'data': Parser.make_data_atom_parser('artist')}, b'\xa9alb': {b'data': Parser.make_data_atom_parser('album')}, b'\xa9cmt': {b'data': Parser.make_data_atom_parser('comment')}, - # b'cpil': {b'data': Parser.make_data_atom_parser('extra.compilation')}, # need test-data for this + # need test-data for this + # b'cpil': {b'data': Parser.make_data_atom_parser('extra.compilation')}, b'\xa9day': {b'data': Parser.make_data_atom_parser('year')}, - # b'\xa9des': {b'data': Parser.make_data_atom_parser('description')}, # need test-data for this + # need test-data for this + # b'\xa9des': {b'data': Parser.make_data_atom_parser('description')}, b'\xa9gen': {b'data': Parser.make_data_atom_parser('genre')}, b'\xa9lyr': {b'data': Parser.make_data_atom_parser('extra.lyrics')}, b'\xa9mvn': {b'data': Parser.make_data_atom_parser('movement')}, @@ -415,11 +417,13 @@ def debug_atom(cls, data): b'\xa9wrt': {b'data': Parser.make_data_atom_parser('composer')}, b'aART': {b'data': Parser.make_data_atom_parser('albumartist')}, b'cprt': {b'data': Parser.make_data_atom_parser('extra.copyright')}, - # b'desc': {b'data': Parser.make_data_atom_parser('extra.description')}, # need test-data for this + # need test-data for this + # b'desc': {b'data': Parser.make_data_atom_parser('extra.description')}, b'disk': {b'data': Parser.make_number_parser('disc', 'disc_total')}, b'gnre': {b'data': Parser.parse_id3v1_genre}, b'trkn': {b'data': Parser.make_number_parser('track', 'track_total')}, - # b'tmpo': {b'data': Parser.make_data_atom_parser('extra.bmp')}, # need test-data for this + # need test-data for this + # b'tmpo': {b'data': Parser.make_data_atom_parser('extra.bmp')}, }}}}} # see: https://developer.apple.com/library/mac/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html