NickvisionApps / Tagger

Tag your music
https://flathub.org/apps/details/org.nickvision.tagger
MIT License
232 stars 14 forks source link

High CPU Usage With M4A Fingerprint Calculation #264

Closed SpicyWasab closed 1 year ago

SpicyWasab commented 1 year ago

(Okay, I admit that this title was a joke.)

Recently I noticed that my computer starts overheating for no reason. Sometimes, after a while the fan kicks in really hard and my computer gets really hot. When I say really it's, like, it suddenly starts to get really noisy.

It happened again a few minutes ago, so I opened Mission Center, and I may have found the culprit :eyes:

Capture d’écran du 2023-08-17 15-34-18

I remember that I've opened Tagger briefly to edit some tracks, and closed it directly after using the shortcut ctrl + Q before continuing doing some things (using GNOME Builder, Edge and Workbench, along with other tiny apps like the Icon Library, Typography, etc.) When I took the screen, only Mission Center was opened though.

Now that I think about, I'm really unsure if the fan issue started since I use Tagger, or only since I use the last beta version :thinking: Edit : still unsure if it's related to the beta version, but when I think about some times when it got really noisy, I think it was always when I used tagger, sometimes quite a lot.

I'm gonna do some tests to see. I didn't saw any other issue about that, so maybe it's just me or under really particular conditions :thinking:.

But if it's really Tagger, wow draining nearly 30% of the battery in about 20 minutes to edit some metadatas is really a lot lol. (it's not even exagerated x))

Anyways, I'm still in vacations but I'm gonna post a reply if I have some other infos or precise steps to reproduce. If you have any request feel free to ask ! (I'm using Fedora Silverblue on a Microsoft Surface Laptop Go 2)

SpicyWasab commented 1 year ago

I don't think I have precise steps to reproduce yet, but what is fpcalc ? *Nevermind I did a research and the name is pretty straightforward : fingerprint calc.

Here's a quick guess : when opening a folder, Tagger tries to calculate the audio fingerprint of each files, and for some reason keeps calculating it even when the main window is closed. (?)

SpicyWasab commented 1 year ago

Okay I'm gonna write this real quick since I now have only 5%, but I guess I know what the issue is (I suspected it in fact).

It's related to me now using .m4a. Apparently fpcalc struggles calculating MPEG-4 AAC file format's fingerprint.

https://bugs.archlinux.org/task/75375

Hope this helps.

SpicyWasab commented 1 year ago

Now on my phone (before my computer explodes lol).

And I guess I know why the cpu usage was so high. I think it's because Tagger tries to calculate the fingerprint only when the view for one particular file is opened. So here's a guess : for each view opened, tagger spawns another fpcalc -> the cpu usage adds up. Maybe when batch editing files it starts calculating for each files. So editing one pr two .m4a files would result to some "CPU leecher", and batch editing .m4a files would act as some sort of "CPU bomb". (if that makes sense).

I'm not sure of all that because I didn't really investigated a lot, but it seems to be the issue.

And again, didn't have the time to check with the regular version, but I don't think you made any changes to the fingerprint calculation since the last version.

nlogozzo commented 1 year ago

Both the current stable and current beta/master will behave the same way as we haven't changed any fingerprint calculation code recently.

With that being said, Tagger lazily evaluates Fingerprint. This means that it will only try to evaluate the fingerprint when you try to use it. So selecting an individual file or trying to run Download Metadata from MusicBrainz will cause the calculation to occur. But, once Tagger has a fingerprint for the file, it won't try to calculate it again. Now as you correctly assumed, the calculation only happens when individual files are selected, not multiple.

Now there is an issue where is the calculation is taking a long time, and then you select another file and then select the original one again, yes another fpcalc instance will spawn up, but I can write some code to prevent that.

It's related to me now using .m4a. Apparently fpcalc struggles calculating MPEG-4 AAC file format's fingerprint. https://bugs.archlinux.org/task/75375

But this chromaprint/fpcalc bug seems to be the overarching issue, in which we can do nothing about until it's fixed. (If it's ever fixed because chromaprint hasn't had an update in a while)

As I said though, I can probably try to mitigate the CPU usage.

Could you email me the M4A file so I have something to test?

nlogozzo commented 1 year ago

I'll also make it where all the fpcalc instances stop on close

nlogozzo commented 1 year ago

(I'm using Fedora Silverblue on a Microsoft Surface Laptop Go 2)

Complete side note...how is fedora on the laptop go 2?? I was looking into one actually...

SpicyWasab commented 1 year ago

in which we can do nothing about until it's fixed. (If it's ever fixed because chromaprint hasn't had an update in a while)

I may have an easier idea : I have the feeling that the calculation never ends, at least on my files. If it's really the case (if the calculation never ends), maybe just not try to calculate the fingerprint if it's an .m4a file (or alternatively if the calculation is too long) ?

The decision is up to you since it quite feels like a workaround, but ... if it doesn't work on these type of files, causes way too much CPU usage, and drains a lot of battery, it seems pretty useless to try to calculate it on .m4a.

In fact, I opened Tagger and started an fpcalc instance on a pretty short song more than 5 minutes ago, and it didn't finished yet. But my computer heats up really quick :/

(and yup, I'll send a comment to notify for the email :))

SpicyWasab commented 1 year ago

I'll also make it where all the fpcalc instances stop on close

I completely agree on that x)

SpicyWasab commented 1 year ago

Complete side note...how is fedora on the laptop go 2?? I was looking into one actually...

Oh :eyes:

About the computer on itself : Quite expensive, unless you have money to spend or it's on sales. I bought mine on sales actually. Honestly, it's my first real new computer, so I don't have a lot of experience and references to compare with. (before I had a really old thinkpad, and after that some shitty computer the city gave to our school. I mean, it was reallyyyy lightweight and tiny which is cool, but 4Gigs of ram, 64 EMMC storage and an intel celeron to run some 30Gigs windows with a lot of school-unused bloatware in 2023 is quite a crime IMO).

Okay that was really off-topic, sorry The computer on itself is pretty cool, but only if you're really looking into a tiny computer which is not very powerful but still enjoyable to code and browse on.

The 3:2 format is pretty cool. In fact, while this computer is 12.4", my previous school 13" laptop does have a wider screen, but the SLG2's one is still taller. While the screen resolution isn't great on the paper, the screen looks good. The pixels are sometimes really visible on some specific colors though, like on some sky-blue buttons. The fan does kick in. Especially on Windows, which is ... a shame since it's litteraly a microsoft laptop ? The keyboard is really enjoyable to use.

I guess you already did some researchs, but IMO it's definitely not for everyone, and if you can go to a store to get hands on the laptop and see the format, you should.

About Linux on this device (since it was basically your question in the first place lol) : Linux support on surface devices is pretty advanced (here's the feature matrix, look for "SLG2" : https://github.com/linux-surface/linux-surface/wiki/Supported-Devices-and-Features#surface-laptops). No need to use the custom kernel on Fedora for this laptop. It's really useless. Battery is good if you pay attention to what apps are actually opened and using the CPU.

But it's not perfect. Almost, but not perfect. I'll start from the less annoying :

  1. The fingerprint doesn't work, which is ... really not bothering. Fingerprints on linux aren't great at all anyways.
  2. Sometimes while waking up the laptop, the touchpad is completely unresponsive. It's no big deal, at first I thought I had to reboot the computer, but in fact I just need to swipe the whole touchpad with the side of my hand. I'm unsure what is the cause, but it's not annoying since I figured it out.
  3. Touchscreen is great, but sometimes buggy. It's really not often, just that sometimes if I use the touchscreen to move a window that is not an Adwaita window, I have to use the touchpad to move it. Like it started the "move window" action but doesn't actually moves it ; when I use the touchpad the cursor appears to be in the "move window" mode and actually moves the window. Maybe not Surface-related at all and a bug of GNOME.
  4. Everything boot-menu related is unresponsive for like 1 second when you start the computer. I guess it's probably related to the fact that most surface devices are tablets, so the UEFI loads an On Screen Keyboard. It's just an icon on the bottom left of the screen to display it, the actual keyboard still works.

Those aren't dealbreakers, I'm just being exhaustive. I think this one could be the one dealbreaker to you :

For some reason, you have to start your computer twice. Okay, that sounds weird, so let me explain : For some reason I didn't figure out, when GRUB starts id displays your menu entries. Whatever menu entry you choose will cause the computer to restart. The second time it displays GRUB, when you choose an entry, it will boot the selected installation. It's not a random but, it's really everytime, like, you know it will happen. You get used to it, but it gets quite annoying when some day you have to reboot your computer often in the same hour to move from windows to linux, or to do some updates.

There is a workaround, which is ... trying other bootloaders. I used REFIND for a while, it was pretty hard to figure out how to setup it (shimx64, here's a reddit post I participated in on the Surface Linux reddit, I tried to explain how I got it working link). It was working pretty good, but I didn't reinstalled Refind when I moved to Silverblue since Silverblue is pretty glued to GRUB.

Last thing : if you're doing a Windows dual-boot ... first of all I really advise you to choose the 256Go option, but disable bitlocker on Windows. It's there by default, and will lock your computer everytime you do something UEFI related, like booting an USB key or making some linux updates. If it's locked and you have a microsoft account, you just have to go to your account settings on the website, find the key, and type it. Not hard, just annoying since it's a very long key.

Yet, I listed everything wrong I could think of so that you don't have much surprise if you buy one, but I do think linux works great on this laptop, and even way better than Windows. (especially the battery)

SpicyWasab commented 1 year ago
nlogozzo commented 1 year ago

The decision is up to you since it quite feels like a workaround, but ... if it doesn't work on these type of files, causes way too much CPU usage, and drains a lot of battery, it seems pretty useless to try to calculate it on .m4a. In fact, I opened Tagger and started an fpcalc instance on a pretty short song more than 5 minutes ago, and it didn't finished yet. But my computer heats up really quick :/

Maybe I could have it timeout after 2 minutes or so...I'm going to test if it's ALL m4a files or just some....if it's ALL then i'll just disable the calculation...if not, then timeout.

nlogozzo commented 1 year ago

Thank you so much for all the info! I'm going to do some more looking around and see what I end up with ahahah :)

nlogozzo commented 1 year ago

Thank you so much for all the info! I'm going to do some more looking around and see what I end up with ahahah :)

I found this cheap, used, i7 15in surface laptop 3...which may sound old but the battery only has 20 charge cycles so it's basically brand new...might snag that

SpicyWasab commented 1 year ago

if it's ALL then i'll just disable the calculation...if not, then timeout.

Sounds good c:

Thank you so much for all the info! You're welcome ^^

I found this cheap, used, i7 15in surface laptop 3 Guess you already saw that, but here's the linux compatibility (SLG3) : https://github.com/linux-surface/linux-surface/wiki/Supported-Devices-and-Features#feature-matrix

Seems good overall, but apparently you can't use the touchscreen without the linux-surface custom kernel, and the "sensors" won't work. (unsure what "sensors" means. The camera should work out of the box, but the "sensors" won't ever work :thinking:)

(it's cheap ? like really cheap ? cheaper than a SLG2 ? how did you manage to find it ? :eyes:)

(btw I just sent you one .m4a file. I know some .m4a files aren't AAC, maybe the issue is related to the AAC codec ?)

nlogozzo commented 1 year ago

Seems good overall, but apparently you can't use the touchscreen without the linux-surface custom kernel, and the "sensors" won't work. (unsure what "sensors" means. The camera should work out of the box, but the "sensors" won't ever work 🤔)

I will probably install the surface kernel anyway...seems easy, just another package to install

it's cheap ? like really cheap ? cheaper than a SLG2 ? how did you manage to find it ? 

399 usd. Ebay haha

btw I just sent you one .m4a file. I know some .m4a files aren't AAC, maybe the issue is related to the AAC codec ?)

I'll investigate. Thanks :)

SpicyWasab commented 1 year ago

Wow, that's a really good price. Kinda hurts since my SLG2 costed nearly twice the price. x)

One side note : if you plan on using Silverblue (I doubt but anyways) installing the surface kernel doesn't seem to be easy. Like you can but it requires some workaround I think.

nlogozzo commented 1 year ago

One side note : if you plan on using Silverblue (I doubt but anyways) installing the surface kernel doesn't seem to be easy. Like you can but it requires some workaround I think.

Yeah I just stick with fedora workstation 38 haha

nlogozzo commented 1 year ago

So i tried the file you gave me, and it generated the fingerprint no problem: image

Let me try a flatpak build and see...

nlogozzo commented 1 year ago

image

ok, on flatpak it loads infinitely...looks like a ffmpeg issue...going to build the latest ffmpeg manually for flatpak instead of using the extension

nlogozzo commented 1 year ago

going to build the latest ffmpeg manually for flatpak instead of using the extension

that didn't work :(

nlogozzo commented 1 year ago

Please try the beta and let me know how it goes: https://github.com/NickvisionApps/Tagger/releases/tag/2023.8.3-beta1 :)

SpicyWasab commented 1 year ago

and it generated the fingerprint no problem

Good to know. Is the CPU usage high ?

that didn't work :(

Oow :/

Please try the beta and let me know how it goes

I didn't do intensive tests as I was about to sleep, but ... nope. I mean I think it's different than before. When I select a file, it tries to calculate the fingerprint. When I select another one, and go back to the previous one, the UI looks like the fingerprint was calculated, except it's blank. There's the copy button and everything, just no value. That's just an UI thing, the fpcalc process is still running in the background and using a lot of ram. So again, selecting multiple files just makes the CPU explode :/

Not the purpose of this issue but I got hands on the lyrics feature as well. Definitely better, still one little (easily fixable) issue I'm gonna describe in #225 :)

nlogozzo commented 1 year ago

Good to know. Is the CPU usage high ?

No it generates almost instantly just like other fingerprints....it seems to be an issue with the ffmpeg we are using for flatpak, but I can't seem to fix it for now...

the UI looks like the fingerprint was calculated, except it's blank.

that's because it didn't finish yet...I have to make it still say Calculating..., I know where that bug is...

That's just an UI thing, the fpcalc process is still running in the background and using a lot of ram. So again, selecting multiple files just makes the CPU explode :/

Yes but it's still just one fpcalc process per file (that we can't do anything about) but for the m4a file it won't spawn multiple. And when you close Tagger it should all end...

That's really all that I can do until we can get ffmpeg fixed...there should be a timeout that after 10 seconds that process ends for the m4a, but I guess that's not working, I will look into it

nlogozzo commented 1 year ago

Ok fixed all the UI stuff...just waiting for an updated packaged of our backend and I'll release a new version for you to try

SpicyWasab commented 1 year ago

Sorry for the late reply, for some reason I completely forgot about it ^^' I just retried because I think that when I did my last test I was too tired and about to sleep, so I kinda missed the point.

Yup, Tagger now kills all the fpcalcs on close. Which is a great step forward.

Yes but it's still just one fpcalc process per file (that we can't do anything about) (yup, but it's still really abnormaly CPU intensive and lasts forever xD)

And when you close Tagger it should all end... Thanks for the reminder, sorry I really missed the point last time :')

I'm not that familiar with flatpak. Why is flatpak's FFMPEG different ?

edit : did you buy a new computer since last time ?

fsobolev commented 1 year ago

I'm not that familiar with flatpak. Why is flatpak's FFMPEG different ?

FFMPEG is very complex and have a lot of flags for compilation to enable specific codecs, options, features etc. What FFMPEG can do is defined at compile time. So for example, in Parabolic we build FFMPEG manually in Flatpak because the build provided by Freedesktop SDK doesn't contain everything we need (even tho it's called ffmpeg-full, hah). So different FFMPEG builds do not necessary work the same for programs that utilize it.

SpicyWasab commented 1 year ago

Okay, thanks for the answer !

even tho it's called ffmpeg-full

Uh, alright x)

nlogozzo commented 1 year ago

Good news tho!! @fsobolev found the fix to get the calculation of the file working on flathub version of the app too :)

SpicyWasab commented 1 year ago

Oh wow, glad to hear this !

nlogozzo commented 1 year ago

https://github.com/NickvisionApps/Tagger/releases/tag/2023.8.3 includes the fix!