1. 29 Mar, 2021 1 commit
  2. 26 Mar, 2021 1 commit
    • Michael Pyne's avatar
      tag_scan: Fix painful rescan of music metadata on startup. · a65a4e8a
      Michael Pyne authored
      For the longest time, JuK has suffered from a problem where its intended
      behavior to load music metadata from a cached database, instead of
      re-scanning every individual track on startup, was not working right.
      
      There has been debugging lines in JuK since all the way back to 2013
      trying to trace what area of startup sequence was taking up all the
      time, but to no avail in helping me fix the bug.
      
      The Problem
      ===========
      
      Recently I took a different approach, of adding a debug-only crash
      whenever we would load a music track tag the "slow" way, and long story
      short there were two bugs that each would cause slowdown:
      
      1. Playlists aside from the CollectionList would cause every music track
         in that playlist to be re-scanned. What this means is that every
         though the music in the CollectionList would be loaded quickly, if
         you had that same music track in a separate Playlist, that music
         track would reload the same tags from disk rather than copying from
         the existing CollectionList item.  This was especially bad for users
         of the old "tree mode" view, since every individual artist *and*
         album were rendered as individual playlists, which would therefore
         each re-scan the music over and over again.
      2. JuK supports a "folder scan" feature, and in fact really wants the
         user to have at least one folder assigned. Any music identified in
         this folder is added to the CollectionList automatically on startup
         and, you guessed it, causes the music track information to be loaded
         from disk, even if the music was already in the CollectionList! :(
      
      The net effect is that most music would be re-scanned on startup unless
      you were a user who used CollectionList exclusively, and had most of
      your music not visible to the folder scanner.
      
      The Solution
      ============
      
      Due to how painful this problem has been, I had ended up adding a
      threaded solution for the folder scan process. This didn't help make
      things any faster but at least the GUI wasn't frozen. But now that the
      threading code is present I judged it would be easier and safer to make
      the central object holding track metadata (CollectionList's m_itemsDict)
      available in thread-safe fashion.
      
      This then permitted me to check for whether a track has already been
      loaded when performing folder scan, and to check whether a track has
      already been loaded when creating a new (non-CollectionList) Playlist.
      In either event if the track already exists, then we copy the FileHandle
      rather than create a new one.
      
      The combination speeds up loading significantly, taking anywhere from
      60% to 70% off of the total time to load on my system, with mostly a
      CollectionList under folder scan and few additional playlists. In this
      configuration I go from about 5.4 seconds to 1.5 seconds with cold
      caches. The difference should be even more stark on systems where disk
      I/O is expensive, or where there are a great number of tracks in
      playlists outside of the CollectionList.
      
      I consider this a bugfix (and there are even multiple bug reports) so I
      will backport shortly.
      
      CHANGELOG:Reduce startup time by 60-70% or more.
      CCBUG:428772
      CCBUG:317666
      FIXED-IN:21.04
      
      (cherry picked from commit d6b28a9b)
      a65a4e8a
  3. 25 Mar, 2021 9 commits
    • Michael Pyne's avatar
      dynamicplaylist: Ensure subclasses handle virtual calls in dtor. · 0027986c
      Michael Pyne authored
      LGTM points out that DynamicPlaylist's destructor calls a function which
      indirectly leads to a virtual call (updateItems) in a potential subclass
      (SearchPlaylist). By C++ rules, SearchPlaylist has ceased to be a
      SearchPlaylist by the time DynamicPlaylist::~DynamicPlaylist() begins,
      so the virtual call would never actually be dispatched to
      SearchPlaylist.
      
      Fix by having SearchPlaylist do its own updating on destruction.
      
      (cherry picked from commit 36581b2b)
      0027986c
    • Michael Pyne's avatar
      nowplaying: Improve clickable link search in Now Playing bar. · 6282bbb5
      Michael Pyne authored
      * The "album" link now searches for any matching tracks of the same album,
        even with different artists.
      * The search applies to the playlist that is selected, even if you're
        playing from a different playlist.
      * The playing item is reset to the Collection List when the "return to
        playlist" link is selected rather than pulling from the internal
        search playlist (which is likely to be destroyed by a subsequent
        search).
      
      (cherry picked from commit a9f85d8d)
      6282bbb5
    • Michael Pyne's avatar
      playlist: Play whole album when playback started in album-random mode. · 2ff06766
      Michael Pyne authored
      Previously, the track the user double-clicked would be played, then
      album random playback would start with the next track and succeeding
      tracks after.
      
      (cherry picked from commit 1c698b3a)
      2ff06766
    • Michael Pyne's avatar
      Reenable album random play in the user interface. · 04b631c3
      Michael Pyne authored
      (cherry picked from commit ced3ffb6)
      04b631c3
    • Michael Pyne's avatar
      playlist: Reimplement random and album random sequencing. · e8e5ef92
      Michael Pyne authored
      (cherry picked from commit b1097f4e)
      e8e5ef92
    • Michael Pyne's avatar
      Improve track sequencing by removing the track sequencing classes. · b46844f6
      Michael Pyne authored
      This removes one of my first contributions to JuK :(
      
      But it's worth it because the extra code is not worth the complexity,
      seeing as how the job is really pretty simple in the first place, even
      with album random play and randomized playback.
      
      I believe this also fixes some bugs, including some longstanding ones.
      Bug 417551 (being unable to drag and drop into Play Queue) had some
      related work in a recent commit but was still broken until now.
      
      BUG:100356
      BUG:166711
      BUG:302250
      BUG:303901
      BUG:336637
      BUG:353259
      BUG:404157
      BUG:417551
      FIXED-IN:21.04
      
      (cherry picked from commit 6aef3be8)
      b46844f6
    • Michael Pyne's avatar
      Remove dead DCOP-related interface. · b718c542
      Michael Pyne authored
      (cherry picked from commit 482db116)
      b718c542
    • Michael Pyne's avatar
      systemtray: Cleanups and modernization. Also a timer bugfix. · b64b3761
      Michael Pyne authored
      The bug: When using the track popup announcement, a timer kicks off to
      tell the popup to fade out. A second timer is used to incrementally fade
      the popup a bit every few milliseconds until the popup can finally be
      hidden. However the first timer was never stopped, and would kick off
      the fadeout sequence again, over and over, only hidden to view since the
      popup is no longer visible.
      
      This might be related to the old bug 165899 (which I tried to fix a
      different way). I believe this happened in the Qt3/4 transition since
      the code seems to assume the first timer was a "single shot" timer, but
      it could have been wrong forever.
      
      CCBUG:165899
      
      (cherry picked from commit 62561ad0)
      b64b3761
    • Michael Pyne's avatar
      Merge commit 'd4bf5912' into release/21.04 · 1e83df7e
      Michael Pyne authored
      This pulls in a batch of bugfixes applied to master into the 21.04
      release branch. Further bugfixes will be cherry picked.
      
      Conflicts:
      	CMakeLists.txt
      1e83df7e
  4. 18 Mar, 2021 5 commits
    • Michael Pyne's avatar
      history: Enforce specific date format that sorts right. · d4bf5912
      Michael Pyne authored
      The QTreeWidget we're using sorts by display text by default. This makes
      the date column in the history playlist a bit useless to sort by in the
      normal format.
      
      The ISO date format is a bit too ugly though, so I manually implement
      something more akin to RFC 3339. To properly fix this we should
      implement a specific role in each item that can be sorted by the
      underlying model. If the underlying model is a QStandardItemModel this
      might already be easy but I don't have time now to investigate further.
      d4bf5912
    • Michael Pyne's avatar
      Address undefined behavior during shutdown. · fce79085
      Michael Pyne authored
      Event filters strike again, along with our old friend, sibling ordering
      dependencies not captured by the Qt memory hierarchy. Again, noted by
      ubsan.
      fce79085
    • Michael Pyne's avatar
      playlist: Avoid resizing columns until after we've generated weights. · 66ddbb3b
      Michael Pyne authored
      This avoids warnings about undefined use of NaN and similar, when we try
      to resize the columns too early (before there are any tracks to use to
      inform average column widths).
      66ddbb3b
    • Michael Pyne's avatar
      Fix undefined behavior in PlaylistItem. · b67785f9
      Michael Pyne authored
      The PlaylistItem ctor meant to be called from its subclass's constructor
      (CollectionListItem) copied the this pointer too early. When
      PlaylistItem is being constructed, by C++ rules the PlaylistItem is not
      a CollectionListItem yet. This was appropriately flagged by ubsan.
      
      I fixed this by ensuring the only user of this subclass fixes up the
      pointer as soon as it can, and making it a private constructor so it's
      not used elsewhere by mistake.
      b67785f9
    • Michael Pyne's avatar
      Fix use of undefined behavior in status label. · 10110d76
      Michael Pyne authored
      The event filter doesn't check that the received event is a QMouseEvent
      but it is possible (and actually happened) to receive other events from
      Qt.
      
      But in this case we don't even need an event filter, we're just handling
      mouse events and Qt provides entirely usable methods for that already.
      10110d76
  5. 13 Mar, 2021 2 commits
  6. 24 Feb, 2021 1 commit
  7. 23 Feb, 2021 1 commit
    • Michael Pyne's avatar
      clazy: Fix most remaining warnings. · 288c69eb
      Michael Pyne authored
      I'm not going to worry about non-POD statics in an application, and I
      can't fix the Q_PROPERTY warnings in Phonon. Other than that, all
      default clazy warnings should be gone now.
      
      Of note is the tagtransactionmanager.cpp setFile call, which is not just
      a warning but an actual bug (good job clazy!).
      288c69eb
  8. 22 Feb, 2021 5 commits
  9. 13 Feb, 2021 2 commits
    • Michael Pyne's avatar
      test: Fix tagguessertest lookup error with ctest. · 3a3bc5e5
      Michael Pyne authored
      For whatever reason, running plain ctest or even "ninja test" on a
      testing-enabled does not succeed in finding tagguessertest to run it,
      probably because this test is located in the build directory's /bin
      subfolder and the test metadata is incorrect.
      
      However I was able to solve this just by using the existing ECM macro to
      add a single test (https://api.kde.org/ecm/module/ECMAddTests.html)
      directly, so that's what I did. ctest now runs and passes both tests
      fine again.
      3a3bc5e5
    • Michael Pyne's avatar
      cmake: Remove needless checks for Taglib format support. · 082914b0
      Michael Pyne authored
      The recent commit 9a54d6e7 to use
      FindTaglib from ECM breaks detection of Ogg Opus support in Taglib for
      some reason. This causes crashes when playing Ogg Opus files because
      their relevant Taglib::File subclass is not created from our MediaFiles
      factory.
      
      However the check is needless. Taglib support for ASF and MP4 files has
      been standard since 2012 or so, and Ogg Opus support was made standard
      around the same time frame. So rather than check for these as optional
      features, just assume that supported versions of Taglib support these
      formats.
      082914b0
  10. 11 Feb, 2021 1 commit
    • Michael Pyne's avatar
      Merge branch 'neoninteger/juk-fix-rename-folders' · 8956f56d
      Michael Pyne authored
      Merging a change from Callum Parsey to fix a bug with the file renamer
      where it was, in essence, trying to treat the resulting file basename as
      if it were another directory in the path, and failing as a result.
      
      This comes extremely late, as I somehow missed this merge request
      hanging out for JuK until after an unrelated one came in recently. But
      no excuses, that's my fault. My apologies to the submitter.
      
      I didn't see a corresponding bug to close. This fix should land in
      version 20.12.3 and later.
      8956f56d
  11. 10 Feb, 2021 1 commit
  12. 06 Feb, 2021 3 commits
  13. 29 Jan, 2021 1 commit
  14. 02 Jan, 2021 1 commit
  15. 01 Jan, 2021 1 commit
  16. 02 Dec, 2020 1 commit
  17. 28 Nov, 2020 1 commit
  18. 08 Nov, 2020 1 commit
  19. 31 Oct, 2020 1 commit
  20. 03 Oct, 2020 1 commit