From 5586f17a4fb56a024afe5422ffa72296b01a8592 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 3 Jul 2020 11:15:44 +0200 Subject: [PATCH 1/4] PICARD-1867: Fix guess_format fallback on file load error Properly handle trying an alternative file format on file load errors by injecting the guessed file format into the file load queue. Avoids issues with the loaded file object being the wrong type and having two file objects causing the pending files to be displayed wrongly. --- picard/file.py | 24 ++++++++++++++---------- picard/tagger.py | 4 ++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/picard/file.py b/picard/file.py index 9e8bccad0..529827e0b 100644 --- a/picard/file.py +++ b/picard/file.py @@ -168,16 +168,7 @@ class File(QtCore.QObject, Item): if self.tagger.stopping: log.debug("File not loaded because %s is stopping: %r", PICARD_APP_NAME, self.filename) return None - try: - # Try loading based on extension first - return self._load(filename) - except Exception: - from picard.formats.util import guess_format - # If it fails, force format guessing and try loading again - file_format = guess_format(filename) - if not file_format or type(file_format) == type(self): - raise - return file_format._load(filename) + return self._load(filename) def _load(self, filename): """Load metadata from the file.""" @@ -189,6 +180,19 @@ class File(QtCore.QObject, Item): if error is not None: self.error = str(error) self.state = self.ERROR + + # If loading failed, force format guessing and try loading again + from picard.formats.util import guess_format + alternative_file = guess_format(self.base_filename) + if alternative_file: + # Do not retry reloading exactly the same file format + if type(alternative_file) != type(self): # pylint: disable=unidiomatic-typecheck + log.debug('Loading %r failed, retrying as %r' % (self, alternative_file)) + self.remove() + self.tagger.add_file(alternative_file, callback) + return + else: + alternative_file.remove() # cleanup unused File object from picard.formats import supported_extensions file_name, file_extension = os.path.splitext(self.base_filename) if file_extension not in supported_extensions(): diff --git a/picard/tagger.py b/picard/tagger.py index 7afbb50d2..3de0d2ff0 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -527,6 +527,10 @@ class Tagger(QtWidgets.QApplication): file.load(partial(self._file_loaded, target=target)) QtCore.QCoreApplication.processEvents() + def add_file(self, file_, callback): + self._pending_files_count += 1 + file_.load(callback) + def _scan_dir(self, folders, recursive, ignore_hidden): files = [] local_folders = list(folders) From 656ac2d097c4d4c5d9a01e7ffcd7ec751afbfd9b Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 3 Jul 2020 15:49:08 +0200 Subject: [PATCH 2/4] PICARD-1867: Handle errors in guess_format --- picard/file.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/picard/file.py b/picard/file.py index 529827e0b..63f3823af 100644 --- a/picard/file.py +++ b/picard/file.py @@ -183,7 +183,12 @@ class File(QtCore.QObject, Item): # If loading failed, force format guessing and try loading again from picard.formats.util import guess_format - alternative_file = guess_format(self.base_filename) + try: + alternative_file = guess_format(self.filename) + except (FileNotFoundError, OSError): + log.error("Guessing format of %s failed", self.filename, exc_info=True) + alternative_file = None + if alternative_file: # Do not retry reloading exactly the same file format if type(alternative_file) != type(self): # pylint: disable=unidiomatic-typecheck From df3c4c941d186fd62d93f7b008369169be2050cf Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 3 Jul 2020 16:00:44 +0200 Subject: [PATCH 3/4] PICARD-1867: Do not increase pending files counter when trying alternative format --- picard/file.py | 2 +- picard/tagger.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/picard/file.py b/picard/file.py index 63f3823af..60266128e 100644 --- a/picard/file.py +++ b/picard/file.py @@ -194,7 +194,7 @@ class File(QtCore.QObject, Item): if type(alternative_file) != type(self): # pylint: disable=unidiomatic-typecheck log.debug('Loading %r failed, retrying as %r' % (self, alternative_file)) self.remove() - self.tagger.add_file(alternative_file, callback) + alternative_file.load(callback) return else: alternative_file.remove() # cleanup unused File object diff --git a/picard/tagger.py b/picard/tagger.py index 3de0d2ff0..7afbb50d2 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -527,10 +527,6 @@ class Tagger(QtWidgets.QApplication): file.load(partial(self._file_loaded, target=target)) QtCore.QCoreApplication.processEvents() - def add_file(self, file_, callback): - self._pending_files_count += 1 - file_.load(callback) - def _scan_dir(self, folders, recursive, ignore_hidden): files = [] local_folders = list(folders) From c303343892105804ff672e3ac6a10a6a1d15c9da Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Fri, 3 Jul 2020 16:08:32 +0200 Subject: [PATCH 4/4] Fixed file loading pending counter not decrementing If a file was detected as an supported file type, but could not be loaded and had an unsupported file extension, tagger._pending_files_count did not get decremented. This results in table sorting staying disabled. --- picard/file.py | 2 +- picard/tagger.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/picard/file.py b/picard/file.py index 60266128e..fa04dcc44 100644 --- a/picard/file.py +++ b/picard/file.py @@ -201,8 +201,8 @@ class File(QtCore.QObject, Item): from picard.formats import supported_extensions file_name, file_extension = os.path.splitext(self.base_filename) if file_extension not in supported_extensions(): - self.remove() log.error('Unsupported media file %r wrongly loaded. Removing ...', self) + callback(self, remove_file=True) return else: self.error = None diff --git a/picard/tagger.py b/picard/tagger.py index 7afbb50d2..c238aee64 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -421,11 +421,15 @@ class Tagger(QtWidgets.QApplication): return 1 return super().event(event) - def _file_loaded(self, file, target=None): + def _file_loaded(self, file, target=None, remove_file=False): self._pending_files_count -= 1 if self._pending_files_count == 0: self.window.set_sorting(True) + if remove_file: + file.remove() + return + if file is None: return