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

Display tracks number next to Auto DJ and every playlist in Playlists and Crates, bug#991938 #8

Closed
wants to merge 16 commits into from

Conversation

weixindut
Copy link

The current implementation is by changing the name of each playlist and crate in the database, which is a straight forward way but with low efficiency and some constraints because of using some reg-exs. A better way is to add a name_displayed column in the table playlists and crates.

@kain88-de
Copy link
Member

Hi

I just skimmed a bit over your code. I don't think we need a new sqltable column. Just make a second query for all the songs in the Playlist and check for it's size.

http://qt-project.org/doc/qt-4.8/qsqlquery.html#size

@weixindut
Copy link
Author

Hi @kain88-de
The new sqltable column I used is used for caching the shown name, which can avoid using reg-ex with some constrains and finally improve the query performance. In fact, only at the last two commits 6d6a50d and ea83b95 I used the new column instead of a reg-ex to reimplement the function. You can have a look at the codes before the last two commits. I'm sorry I am not very clear about the meaning of just making a second query for all the songs and checking for it's size. How to avoid a new sqltable column through a second query?

@rryan
Copy link
Member

rryan commented Jun 17, 2013

I agree we should not change the database schema. Instead you can use a nested query and a join:

select Playlists.name, count_table.count from Playlists INNER JOIN 
  (select playlist_id, count(*) as count from PlaylistTracks 
  GROUP BY playlist_id) AS count_table 
  ON count_table.playlist_id = Playlists.id;

@weixindut
Copy link
Author

Thanks @rryan. I am sorry for having ignored the performance of my sql statements. I will modify them later. And If both you and @kain88-de think it's better not to change the database schema, I will do a rollback.

@rryan
Copy link
Member

rryan commented Jun 18, 2013

@weixindut no worries! Also, the performance of your patch was fine. The nested query approach I posted is slightly slower when creating the list of playlist names but it eliminates the need to stick the track count in the title (and add getCrate/PlaylistByNameDisplayed which is a little awkward).

@kain88-de
Copy link
Member

@weixindut do you still want to work on this?

@rryan
Copy link
Member

rryan commented Jan 3, 2014

Hi @weixindut -- I'm guessing you don't have time to work on this. Let us know if you want to finish it up at some point. Thanks!

@rryan rryan closed this Jan 3, 2014
@daschuer daschuer mentioned this pull request Dec 28, 2014
daschuer referenced this pull request in daschuer/mixxx Oct 15, 2015
Improves shoutcast by moving to _raw sending (drop syncing code from libshout) and let Mixxx do the timing.
daschuer pushed a commit that referenced this pull request May 25, 2017
@daschuer daschuer mentioned this pull request Aug 13, 2017
19 tasks
@daschuer daschuer mentioned this pull request Jun 3, 2018
@daschuer daschuer mentioned this pull request Jun 11, 2018
11 tasks
daschuer pushed a commit that referenced this pull request Jul 14, 2018
daschuer pushed a commit that referenced this pull request Jun 12, 2020
replace invalid characters from Git diff
Holzhaus referenced this pull request in Holzhaus/mixxx Aug 22, 2020
LateNight: Fix keylock icon colors
ywwg pushed a commit that referenced this pull request Mar 7, 2022
…h sync

When loading a track that is not yet present in the library (and thus
doesn't have any BPM because it hasn't been analyzed yet) while another
deck is playing and both decks have sync enabled, a debug assertion is
triggered:

    DEBUG ASSERT: "isValid()" in function double mixxx::Bpm::value() const at src/track/bpm.h:53
    Aborted (core dumped)

The backtrace looks as follows:

    #0  0x00007f175c87234c in __pthread_kill_implementation () at /usr/lib/libc.so.6
    #1  0x00007f175c8254b8 in raise () at /usr/lib/libc.so.6
    #2  0x00007f175c80f534 in abort () at /usr/lib/libc.so.6
    #3  0x00007f175cf05ee4 in qt_assert(char const*, char const*, int) () at /usr/lib/libQt5Core.so.5
    #4  0x000055deb2e67e1c in mixxx::(anonymous namespace)::handleMessage(QtMsgType, QMessageLogContext const&, QString const&) (type=<optimized out>, context=<optimized out>, input=<optimized out>) at src/util/logging.cpp:355
    #5  0x00007f175cf47128 in  () at /usr/lib/libQt5Core.so.5
    #6  0x00007f175cf3fd8a in  () at /usr/lib/libQt5Core.so.5
    #7  0x00007f175cf06526 in QMessageLogger::critical(char const*, ...) const () at /usr/lib/libQt5Core.so.5
    #8  0x000055deb2e5c720 in mixxx_debug_assert(char const*, char const*, int, char const*) (assertion=assertion@entry=0x55deb39bd0db "isValid()", file=file@entry=0x55deb39bbf30 "src/track/bpm.h", line=line@entry=53, function=function@entry=0x55deb39bbf08 "double mixxx::Bpm::value() const") at gsrc/util/assert.h:9
    #9  0x000055deb2ee7e7e in mixxx_debug_assert_return_true(char const*, char const*, int, char const*) (function=0x55deb39bbf08 "double mixxx::Bpm::value() const", line=53, file=0x55deb39bbf30 "src/track/bpm.h", assertion=0x55deb39bd0db "isValid()") at gsrc/util/assert.h:18
    #10 mixxx::Bpm::value() const (this=<synthetic pointer>) at src/track/bpm.h:53
    #11 mixxx::operator*(mixxx::Bpm, double) (multiple=1, bpm=...) at src/track/bpm.h:160
    #12 SyncControl::setLocalBpm(mixxx::Bpm) (this=<optimized out>, localBpm=...) at src/engine/sync/synccontrol.cpp:567
    #13 0x000055deb34c7ba3 in EngineBuffer::postProcess(int) (this=0x55deb56eb060, iBufferSize=2048) at src/engine/enginebuffer.cpp:1318
    #14 0x000055deb3139023 in EngineMaster::processChannels(int) (this=0x55deb5449440, iBufferSize=<optimized out>) at src/engine/enginemaster.cpp:383
    #15 0x000055deb31394f7 in EngineMaster::process(int) (this=0x55deb5449440, iBufferSize=iBufferSize@entry=2048) at src/engine/enginemaster.cpp:410
    #16 0x000055deb2f91d0b in SoundManager::onDeviceOutputCallback(long) (this=<optimized out>, iFramesPerBuffer=iFramesPerBuffer@entry=1024) at src/soundio/soundmanager.cpp:596
    #17 0x000055deb32dd794 in SoundDevicePortAudio::callbackProcessClkRef(long, float*, float const*, PaStreamCallbackTimeInfo const*, unsigned long) (this=0x55deb553e6b0, framesPerBuffer=1024, out=<optimized out>, in=<optimized out>, timeInfo=<optimized out>, statusFlags=<optimized out>) at src/soundio/sounddeviceportaudio.cpp:965

This happens because `newLocalBpm` is invalid when `localBpm` is
invalid. Trying to do sync decks while no tempo information is available
does not make sense, so we only synchronize decks if the local BPM is
available.
JoergAtGithub pushed a commit that referenced this pull request Jan 3, 2024
m0dB referenced this pull request in m0dB/mixxx Jan 21, 2024
m0dB referenced this pull request in m0dB/mixxx Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants