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.

Sign up for GitHub

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

Jump to bottom

Use HSV renderer for track's waveform overview #15

Closed
wants to merge 1 commit into from
Closed

Use HSV renderer for track's waveform overview #15

wants to merge 1 commit into from

Conversation

xorik
Copy link
Contributor

@xorik xorik commented Jun 18, 2013

@daschuer
Copy link
Member

Hi xorik,

Thank you for your update.
While this new waveform looks nice, the user should have an option in preferences to choose between the different waveform overview styles.
This system should be that generic, that we later can choose a key / mood / beat view as well.

@xorik
Copy link
Contributor Author

xorik commented Jun 18, 2013

Do you add this option in the near future?

@kain88-de
Copy link
Member

I'll also agree with Daniel. If we add new renderers also for the overview it should be possible to change them later and add new ones as we also do for the normal waveforms

@daschuer
Copy link
Member

Do you add this option in the near future?

I have currently a slightly different focus. But it is very welcome, if you could spend additional time and make the new overview optional. You will receive all support you need.
I would love to merge your branch if this work is done.

The best solution is to let the user change the overview at runtime. But this is of cause the most work.
Maybe you can start with an intermediates solution where the overview style is selected by the skin or changed after Mixxx restart.

@xorik
Copy link
Contributor Author

xorik commented Jun 18, 2013

I'm glad to hear it, but I'm very novice in Qt and don't know, how to change interface, add new option into preferences, and make mixxx to save it...

@daschuer
Copy link
Member

Thats no problem, if you are willing to learn and if are able to spend some extra time for this ...
I think you already know mixxx-dev and our IRC channel!

@rryan
Copy link
Member

rryan commented Jun 18, 2013

I agree with @daschuer and @kain88-de -- it needs to be configurable a-la the waveform preferences. Perhaps it should even be the same selection as the waveform preference -- if you pick the HSV renderer, you get the HSV overview. If not -- you get the regular overview. That would prevent having to change the preferences window.

@xorik
Copy link
Contributor Author

xorik commented Jun 18, 2013

Perhaps it should even be the same selection as the waveform preference -- if you pick the HSV renderer, you get the HSV overview. If not -- you get the regular overview.

Good idea!
Can you help me? How can I find, which waveform renderer is used, inside this function:
WOverview::drawNextPixmapPart()

@kain88-de
Copy link
Member

I don't like to have the HSV Waveform and HSV overview together. For me the old overview has some information I can't spot easily in the HSV view. So being able to activate them separetely would be nice for me. I'm ok if we don't show that option to the novice user but it should be possible to do in the mixxx.cfg .

@esbrandt
Copy link
Contributor

Another aspect is the CPU utilization. For example on a stock Macbook Pro 2011, the HSV renderer is nearly unusable because it consumes about 25% more CPU then the standard renderer. While the HSV view is pretty nifty and can be of good use, making it optional is better then making it the new default. And it should be easily accessible in the waveform preferences. Messing with *.cfg files is no option.

@kain88-de
Copy link
Member

I think this is a OS problem, because I experience it different on Linux. I benchmarked both overviews this morning and could measure any difference in performance ( AMD X4 640 at 800 Mhz). Also on my Laptop I get see that the HSV waveform is smoother then the GL ones and this Laptop has a rather crappy low voltage CPU from 2010 and a INTEL GMA4500.

If we also add the options to change the overview waveform, we should move all waveform options to a new perference tab. The interface tab is already crowded as it is

@rryan
Copy link
Member

rryan commented Jun 18, 2013

If we also add the options to change the overview waveform, we should move all waveform options to a new perference tab. The interface tab is already crowded as it is.

I think we can all agree on that! A dedicated preference section for the waveform would be great.

@xorik
Copy link
Contributor Author

xorik commented Jun 19, 2013

Ok, I'll try to rewrite this part, to make this optional, and then resend pull request

@xorik xorik closed this Jun 19, 2013
@ywwg
Copy link
Member

ywwg commented Jun 19, 2013

You don't need to close the whole request, just make your changes and repush and the pull request will be automatically updated

@xorik
Copy link
Contributor Author

xorik commented Jun 20, 2013

You don't need to close the whole request, just make your changes and repush and the pull request will be automatically updated

I know it, but I want totally rewrite the code, and I don't need this commit :)

@ywwg ywwg mentioned this pull request Dec 20, 2013
Merged
@uklotzde uklotzde mentioned this pull request Jul 12, 2014
Merged
@daschuer daschuer mentioned this pull request Dec 28, 2014
Merged
@daschuer daschuer mentioned this pull request Jan 8, 2015
Merged
@daschuer daschuer mentioned this pull request Aug 13, 2017
Merged
19 tasks
ronso0 referenced this pull request in ronso0/mixxx Nov 29, 2017
@Be-ing
Merge pull request #15 from ronso0/deere_2.1_layout-fixes
10768df 
Deere 2.1 layout & color fixes
@daschuer daschuer mentioned this pull request Jun 11, 2018
Closed
11 tasks
@uklotzde uklotzde mentioned this pull request Jan 16, 2020
Merged
Holzhaus referenced this pull request in Holzhaus/mixxx Apr 18, 2020
@ferranpujolcamins
Merge pull request #15 from Holzhaus/bulk-controller-fix
97d5623 
controllers/bulk: Fix BulkController constructor
ninomp referenced this pull request in ninomp/mixxx May 16, 2020
@rryan
Merge pull request #15 from daschuer/qopenglwidget
319d5fd 
resolve conflicts, Fix OpenGL status
daschuer pushed a commit that referenced this pull request Jun 13, 2020
@uklotzde
Merge pull request #15 from daschuer/faad2
c775f33 
Faad2 + CMake
Holzhaus referenced this pull request in Holzhaus/mixxx Aug 23, 2020
@kain88-de
Merge pull request #15 from mixxxdj/manual-updates
087390e 
Update manual with Master Sync and Midi Wizard instructions
Holzhaus referenced this pull request in Holzhaus/mixxx Feb 21, 2022
@Holzhaus
EngineBuffer: Fix assert when new track is loaded during playback wit…
3565e26 
…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.
Holzhaus referenced this pull request in Holzhaus/mixxx Mar 1, 2022
@Holzhaus
EngineBuffer: Fix assert when new track is loaded during playback wit…
9614a8f 
…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.
@mixxxbot mixxxbot mentioned this pull request Aug 24, 2022
Closed
@ronso0 ronso0 mentioned this pull request Sep 4, 2022
Merged
@ronso0 ronso0 mentioned this pull request Sep 13, 2022
Merged
@ronso0 ronso0 mentioned this pull request May 9, 2023
Merged
@Swiftb0y Swiftb0y mentioned this pull request Aug 7, 2023
Closed
@acolombier acolombier mentioned this pull request May 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xorik @daschuer @kain88-de @rryan @esbrandt @ywwg