Open celynw opened 4 years ago
I'd like to add that FLAC & MP3 files work just fine for me. I noticed this issue with .m4a
and stopped the whole scan. Any way we can debug further without trying to find memory errors within the source?
Phew. Good question (and I’m happy to see someone uses it on a Windows machine!). Thanks for the build log, it looks okay but I wonder why it uses only version 56.38.100 of libavutil
. I think (but I’m not sure) it should also have a version 58.xx?!
You are saying it only happens with m4a
? Any possibility you could send me a (short) m4a
file that produces the bug? My test suite contains m4a-aac, m4a-alac and m4a-nero files, and they all work out ok here.
If you do a loudgain -v
, what does it show?
In the past we found that updating FFMpeg also helped in some cases (loudgain uses part of the FFMpeg libraries).
I just noticed the m4a segfault on Debian 11, which offers Loudgain version "0.6.8+ds-1+b1" as a package.
$ loudgain -v
loudgain 0.6.8 - using:
libebur128 1.2.5
libavformat 58.45.100
libswresample 3.7.100
These are downloaded files, I'll see if I can encode a random m4a file that reproduces the problem and upload it somewhere.
@TylerVigario @deutrino The reason is because the current code stores a reference to a local object from the taglib function TagLib::MP4::Tag::itemListMap()
, but that object is destroyed when the function completes. The calling function later tries to access the destroyed object, which is UB and typically results in a segfault. A contributing factor here is the lack of a const
qualifier on the function return type, which would have prevented this bug at compile time. That's why the taglib devs deprecated itemListMap
in favor of itemMap
.
This same issue also affects WMA/ASF.
To fix the bug, simply convert the reference to a local object. This effectively copies the object to the caller and prevents the issues seen above. See the below patch:
diff --git a/src/tag.cc b/src/tag.cc
index 1c149f3..bd39aeb 100644
--- a/src/tag.cc
+++ b/src/tag.cc
@@ -512,7 +512,7 @@ void tag_remove_mp4(TagLib::MP4::Tag *tag) {
for(TagLib::MP4::ItemMap::Iterator item = items.begin();
item != items.end(); ++item)
#else
- TagLib::MP4::ItemListMap &items = tag->itemListMap();
+ TagLib::MP4::ItemListMap items = tag->itemListMap();
for(TagLib::MP4::ItemListMap::Iterator item = items.begin();
item != items.end(); ++item)
@@ -591,7 +591,7 @@ bool tag_clear_mp4(scan_result *scan) {
void tag_remove_asf(TagLib::ASF::Tag *tag) {
TagLib::String desc;
- TagLib::ASF::AttributeListMap &items = tag->attributeListMap();
+ TagLib::ASF::AttributeListMap items = tag->attributeListMap();
for(TagLib::ASF::AttributeListMap::Iterator item = items.begin();
item != items.end(); ++item)
@hughmcmaster I recommend applying the above patch to your Debian package to help @deutrino and others with this issue. This is a somewhat critical bug since it completely breaks M4A support with taglib <1.12 (current Stable) and WMA support for all versions. loudgain seems to have been abandoned, so it's unlikely this will be fixed upstream anytime soon, if ever.
@TylerVigario @deutrino The reason is because the current code stores a reference to a local object from the taglib function
TagLib::MP4::Tag::itemListMap()
, but that object is destroyed when the function completes.
@complexlogic's patch works for me
First of all, really pleased I found this, looks like some great work. Unfortunately, I'm not sure I have enough information to help the report. I'm running Windows, but with WSL running Ubuntu 18.04.3. Followed the instructions, very clear and familiar. No problems.
But running
loudgain
in any way on a file segfaults:It's the same with any filetype:
If running through
rgbpm
, it returns:For verbosity's sake, I'm including the build output:
Is there any way I can debug this?