Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Overviews in library #13644

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

ninomp
Copy link
Contributor

@ninomp ninomp commented Sep 11, 2024

@ronso0 This is my work so far. This is obviously still a work in progress, but I am sharing this so that you can take a look and so that we can discuss our next steps. After we make a plan, I will split this into several smaller commits and/or PRs.

Here is a summary of changes:

  • Move overview waveform drawing from WOverview into a new component called OverviewRenderThread (currently not used as a dedicated overview render thread)
  • Introduction of OverviewCache (used by both WOverview and OverviewDelegate)
  • Introduction of Overview column (mostly a copy of Cover art column)
  • Refactor of Overview waveform render settings and removal of [Waveform], WaveformOverviewType CO

Note: I wasn't able to render overviews in a dedicated thread. For some reason I cannot get a database connection inside of that thread.

@ronso0
Copy link
Member

ronso0 commented Sep 11, 2024

Alright, thanks!

Just a quick comparison of #13638 / this
image image

@ronso0
Copy link
Member

ronso0 commented Sep 11, 2024

You're currently not using QPixmapCache?

@Swiftb0y
Copy link
Member

in the left screenshot it looks like the waveforms where drawn at higher resolution and then scaled down with anti-aliasing while in the right one they seem to have been drawn at screen resolution without AA?

@ronso0
Copy link
Member

ronso0 commented Sep 11, 2024

I couldn't resist to look into the issues, so here are some commits that fix the deck overview and the scaling in OverviewDelegate:
a09a037
73edc2f

Sometimes the overview delegate only shows the first few chunks of the generated image (when it's being analyzed after loading to deck).

@ronso0
Copy link
Member

ronso0 commented Sep 11, 2024

Update immediatly when type is changed
99d13c3

@ninomp
Copy link
Contributor Author

ninomp commented Sep 11, 2024

You're currently not using QPixmapCache?

Well, the idea was to have two layers of cache: One that will cache full size QImages and the other one that will cache scaled down QPixmaps with QPixmapCache. I first wanted to get the first one working properly, and only then introduce the second one.

Also, thank you for the fixes. I'll incorporate them.

@Swiftb0y
Copy link
Member

Note that there are also other generic caches in Qt. QCache and QContigousCache, the latter specifically designed for caching model content (though I'm not sure how good that jives with sorting).

@ninomp
Copy link
Contributor Author

ninomp commented Sep 11, 2024

Note that there are also other generic caches in Qt. QCache and QContigousCache, the latter specifically designed for caching model content (though I'm not sure how good that jives with sorting).

I am using QCache for the full size QImages cache.

@ronso0
Copy link
Member

ronso0 commented Sep 12, 2024

Well, the idea was to have two layers of cache: One that will cache full size QImages and the other one that will cache scaled down QPixmaps with QPixmapCache. I first wanted to get the first one working properly, and only then introduce the second one.

Uff, caching the raw images is costly.
I was curious how big the cache gets so I added some logging.
With the current (item) limit of 50 and after loading some overviews of live sets (1-2h) Mixxx uses ~7 GB of RAM for the QImage cache :
I fear this will cause issues on machines with little RAM, like RPi and alike.

Though this only happens during the session where a track is being analyzed.
In later sessions all track's images are ~3820 kB
What could be the reason?

This is the logging commit (yes, it's a bit excessive and slows down the GUI :\ )
f79ad80

@ywwg
Copy link
Member

ywwg commented Sep 17, 2024

Are these LRU caches that we can cap so they don't grow without limit?

@ninomp
Copy link
Contributor Author

ninomp commented Sep 17, 2024

@ronso0 I tried your commit and it seems like there is a memory leak, but I was not able to find it.

@ywwg Yes, QCache is a LRU cache and currently it is capped at 50 items.

@ronso0
Copy link
Member

ronso0 commented Sep 18, 2024

Compared to QPixmapCache, only QCache items can be capped, but not the size.

To me it didn't look like a memory leak, it's really just the cached images that are huge. (same for the QImages created by WOverview btw)
There must a difference between rendering the image with the initial analysis data and with the reloaded (compressed?) data later on.
I'd try re-rendering when the analysis has finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants