Bug 429803 - Import Audio File Metadata does not work since version 3.3
Summary: Import Audio File Metadata does not work since version 3.3
Status: RESOLVED FIXED
Alias: None
Product: tellico
Classification: Applications
Component: general (show other bugs)
Version: 3.3
Platform: Manjaro Linux
: NOR normal
Target Milestone: ---
Assignee: Robby Stephenson
URL:
Keywords:
: 437807 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-11-29 11:33 UTC by jolu4711
Modified: 2021-06-15 01:19 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 3.3.5


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jolu4711 2020-11-29 11:33:24 UTC
Import Audio File Metadata does not work since version 3.3.
This applies to 3.3.1, 3.3.2 and 3.3.3, could not test 3.3.4 as it is not jet in the Manjaro repos. But the changelog of 3.3.4 does not mention a fix regarding this item.
Last working version was 3.2.3

STEPS TO REPRODUCE
1. Open Tellico, click on File/New/New Music Collection
2. Click on "Import Audio File Metadata"
3. Select a folder containing mp3 files with metadata, click "Choose"
4. Click "Import"

OBSERVED RESULT
There is no new item in the collection.

EXPECTED RESULT
New item in the collection.

SOFTWARE/OS VERSIONS
Linux 4.19.160
XFCE 4.14
Qt Version: 5.15.2
Comment 1 jolu4711 2020-11-30 10:22:58 UTC
Does not work with Tellico 3.3.4 on a fresh installation of openSUSE Tumbleweed as well.
KDE Frameworks 5.76.0
Qt 5.15.2
Linux 5.9.10-1-default
Comment 2 Robby Stephenson 2020-12-01 15:39:23 UTC
Git commit aef00862c70f7321564059fda457fb26b287e649 by Robby Stephenson.
Committed on 01/12/2020 at 15:39.
Pushed by rstephenson into branch 'master'.

Fix reading album artist when ALBUMARTIST audio tag does not exist

Prevent trying to read first entry in an empty string list by checking
list length. Add unit test. Also ensure albumkey is appropriate for
either case.
FIXED-IN: 3.3.5

M  +4    -0    ChangeLog
M  +22   -2    src/tests/audiofiletest.cpp
M  +1    -0    src/tests/audiofiletest.h
A  +-    --    src/tests/data/test.mp3
M  +5    -5    src/translators/audiofileimporter.cpp

https://invent.kde.org/office/tellico/commit/aef00862c70f7321564059fda457fb26b287e649
Comment 3 Robby Stephenson 2020-12-01 15:39:36 UTC
Git commit c79c6a87be760784a2f79cca3113419cdde60302 by Robby Stephenson.
Committed on 01/12/2020 at 15:39.
Pushed by rstephenson into branch '3.3'.

Fix reading album artist when ALBUMARTIST audio tag does not exist

Prevent trying to read first entry in an empty string list by checking
list length. Add unit test. Also ensure albumkey is appropriate for
either case.
FIXED-IN: 3.3.5

M  +4    -0    ChangeLog
M  +22   -2    src/tests/audiofiletest.cpp
M  +1    -0    src/tests/audiofiletest.h
A  +-    --    src/tests/data/test.mp3
M  +5    -5    src/translators/audiofileimporter.cpp

https://invent.kde.org/office/tellico/commit/c79c6a87be760784a2f79cca3113419cdde60302
Comment 4 jolu4711 2020-12-04 08:25:38 UTC
Thanks for your fast fix, but it doesn't work for me.
My mp3 files do have an ALBUMARTIST audio tag.
I tried some debugging, but I'm not familiar with KDE/Qt developement and have less time.
When importing the folder it seems the collection returned by AudioFileImporter::collection() does not contain any data. 
The for loop in line 190 of audiofileimporter.cpp does not add anything to the string list (directoryFiles), on the first loop it exits with the continue statement.
Comment 5 Robby Stephenson 2020-12-08 02:04:08 UTC
Git commit 2ed98d560608290ad9eb0341dc38dd8044456212 by Robby Stephenson.
Committed on 08/12/2020 at 02:01.
Pushed by rstephenson into branch 'master'.

Ensure audio .directory file is always read with relative icon file path

Adjust unit test to add additional audio file directory which includes
a .directory pointing to an icon for the album. Add additional has to
map directory name to the album key used for the album entry.

M  +16   -2    src/tests/audiofiletest.cpp
M  +1    -0    src/tests/audiofiletest.h
A  +1    -0    src/tests/data/audio/.directory
R  +-    --    src/tests/data/audio/test.mp3 [from: src/tests/data/test.mp3 - 100% similarity]
M  +11   -6    src/translators/audiofileimporter.cpp

https://invent.kde.org/office/tellico/commit/2ed98d560608290ad9eb0341dc38dd8044456212
Comment 6 Robby Stephenson 2020-12-08 02:10:32 UTC
(In reply to jolu4711 from comment #4)
> When importing the folder it seems the collection returned by
> AudioFileImporter::collection() does not contain any data. 
> The for loop in line 190 of audiofileimporter.cpp does not add anything to
> the string list (directoryFiles), on the first loop it exits with the
> continue statement.

That loop is just looking at the .directory files for an album icon. It shouldn't affect which files are read for importing the metadata. (Though your comment did spur me to expand the unit test and correct a different bug when reading relative icon paths).

Does the directory in question contain only mp3 files? And they are all from the same album?
Comment 7 jolu4711 2020-12-08 08:31:29 UTC
Yes, all mp3 files are from the same album and there are no other files in that folder.
The bug originally came up for me with 3.3.1 on my FLAC music collection, so the bug is not related to mp3 files only. The structure of that collection is:
One folder for each album, containing the music files and a cover.jpg which I import manually as a link.

I switched to mp3 album/files just for testing with a new collection.
When the for loop in line 191 exits on the first run the program never reaches a breakpoint on line 260, so I think it has to do with this this bug. The for loop is closed in line 388, so in my opinion it is not just for scanning .directory files.
Comment 8 Robby Stephenson 2020-12-09 02:08:44 UTC
Git commit 7fdebb51634222cf419e1f412483c9d2556de79e by Robby Stephenson.
Committed on 09/12/2020 at 02:08.
Pushed by rstephenson into branch '3.3'.

Use QFileInfo to determine when importing an audio file directory

Rather than checking for an empty filename to determine when importing
a directory, use QFileInfo which accounts for scenario when the file
chooser passes a directory without a trailing slash. Adjust unit test
accordingly.

M  +4    -0    ChangeLog
M  +2    -0    src/tests/audiofiletest.cpp
M  +4    -4    src/translators/audiofileimporter.cpp

https://invent.kde.org/office/tellico/commit/7fdebb51634222cf419e1f412483c9d2556de79e
Comment 9 Robby Stephenson 2020-12-09 02:09:13 UTC
Thanks for the follow-up. I believe I've found the issue now.
Comment 10 jolu4711 2020-12-09 07:12:04 UTC
It works as intended, from my point of view this bug can be closed now.
Many thanks for your help!
Comment 11 Robby Stephenson 2021-06-15 01:19:41 UTC
*** Bug 437807 has been marked as a duplicate of this bug. ***