RGB waveform display

[ninomp]

Added RGB waveform widget and renderer.

[rryan]

Hi @ninomp,

This is a neat waveform. I'm curious about the signal colors though. Mixxx has a skinning system that allows skin writers to choose the color scheme of the all the interface widgets. They might have 3 primary colors they are using in the skin that would be natural to use to represent the low, mid, and high.

What do you think about making the 3 strength components represented by skin-chosen colors with the defaults being red, green, and blue?

You could fetch these via the onSetup() method where you get access to the DOM node that created the waveform.

[ritschwumm]

@rryan, in my experience you need pretty strong base colors or everything becomes a mushy grey. strong enough to make your eyes hurt when used for the rest of the gui.

[daschuer]

Thank you for the nice waveform. I have tested it and it works fine. There are only some minor code style issues copied from the HSV waveform I think, so not your fault.

There are a general things I am worrying:

• How many waveform types are good for Mixxx?
• Design is more important than we technicians suppose. ;-)
• How will we integrate key or mood information in Mixxx later?

Sometime a additional preference option is only a leak of a clear use case definition.
Can we skip some other waveform for this?
Normally we should be able to cover all use cases with two waveform types. But since this is an free software project Mixxx suffers/provides more :-/

To take the colours already defined in the skins is a good idea.

How will a key/display solution look like? Will it conflict with this approach?

A pure GL Version of this would be really nice.

By the way: the original HSV waveform is from https://bugs.launchpad.net/mixxx/+bug/1074346

[ninomp]

Hi,

I made some (minor) improvements in WaveformRendererRGB::draw(). Still, I am not using colors from the skin, but that is the next thing I am planning to do. However, I am worried that this will no longer be a 'RGB' renderer. My original idea was to use (pure) red, green and blue (hence the name) for representing low, mid, and high. Still, I understand your concerns and I am going to follow your advice.

Anyway, what do you think about this waveform renderer now ? Are the colors too bright ?

[esbrandt]

Thanks for implementing this waveform type.
I think the colors are a good choice and provide good visibility on darker backgrounds. Should be the default, if skins do not provide any any values for SignalHighColor,SignalMidColol,SignalLowColor.

This would be excellent as waveform overview type too, similar to #17 .

Just curious, how is your CPU utilization?
On OSX 10.8 the rgb waveform, and also the hsv renderer, uses as much as 70% cpu with 2 decks running. As comparison the simple waveform (gl) uses 35%,the filtered (GL) 45%

[rryan]

Hm, I also see poor performance relative to other GL renderers. I wonder if this is due to rapid changing of brushes? Qt's paint system wasn't really built to have the brush changed so frequently. I think an OpenGL shader would be much more efficient at doing per-line colorings. Not sure if that's the issue w/ the software rendering here. Maybe you could try a GL enabled one (just use QPainter on a QGLWidget) to compare the Qt software renderer with the OpenGL renderer.

I'm also curious why you chose RGB? Red, green and blue color mixing isn't very intuitive for humans (i.e. if you gave me 3 random colors and told me to add them in RGB space I don't think I could tell you what they were) whereas if you gave me 3 HLS colors I could probably get you at least roughly the right color.

Maybe it would make more sense if you went one-dimensional and picked 3 points on the hue band (each 120 degrees away from each other -- red green and blue would work) and just added them up?

[ninomp]

Hi !

I agree with you. Using GL for rendering is a great idea, so I am planning to turn this into a Qt GL or pure OpenGL renderer.

About the choice of colors, I noticed that software 'Serato ITCH' (http://serato.com/itch) uses red, green and blue for displaying low, mid and high in audio waveform. It seemed like a (very) good idea, so I thought it would be nice if Mixxx had something similar.

[rryan]

Thanks @ninomp -- I totally get why red green and blue are good seed colors :). They're nice and separate on the color wheel.

@daschuer -- good question about how many waveform renderers are good for Mixxx. I think for Mixxx 1.x we should try a bunch of them and then pare things down for Mixxx 2.x when we find which few work the best.

Given that the HSV renderer suffers the same performance issues maybe we should merge this and try to figure out a general performance fix? I'm curious about what swapping a QWidget for QGLWidge would do for both of those.

[rryan]

As for key -- there are 24 values to represent. If we used color I think the waveform might look fancy (or like an ugly mess of color :) ) but be very hard to interpret. I think it would be cool to have frame-by-frame key information drawn at the top of the waveform (in text) w/ markers indicating where the changes are.

[ninomp]

Hi @rryan,
some time ago I wrote a GL version of my RGB renderer.
Today, I modified non-GL version of my RGB renderer so it uses colors defined in the skin.
How does all this seem to you?

[esbrandt]

With the latest ninomp/mixxx@6cbe64b applied, compile breaks with following error:

src/waveform/renderers/waveformrendererrgb.cpp:26:39: error: no member named 'selectNodeQString' in 'WWidget'
    m_lowColor.setNamedColor(WWidget::selectNodeQString(node, "SignalLowColor"));
                             ~~~~~~~~~^
src/waveform/renderers/waveformrendererrgb.cpp:32:39: error: no member named 'selectNodeQString' in 'WWidget'
    m_midColor.setNamedColor(WWidget::selectNodeQString(node, "SignalMidColor"));
                             ~~~~~~~~~^
src/waveform/renderers/waveformrendererrgb.cpp:38:40: error: no member named 'selectNodeQString' in 'WWidget'
    m_highColor.setNamedColor(WWidget::selectNodeQString(node, "SignalHighColor"));
                              ~~~~~~~~~^
3 errors generated.
scons: *** [osx64_build/waveform/renderers/waveformrendererrgb.o] Error 1
scons: building terminated because of errors.

[ninomp]

@esbrandt, errors are fixed. However, I couldn't fetch colors from the skin in onSetup(), so I had to override setup() in my class. Don't worry, the first thing I do in my (overided) function is call setup() from base class.
Furthermore, colors seemed too dark to me, so I changed the way color is computed.

[esbrandt]

@rryan
Whats the status on this PR?

In https://bugs.launchpad.net/mixxx/+bug/1074392 the talk is to integrate just another very similar waveform, a RGB view but with no envelope visible.

Can`t we just take this PR and add RGB as waveform overview option?

IMO Moodview would only add a benefit, if it displays the data in a library column.

[rryan]

Thanks @ninomp -- this looks good. I'm also very sorry for taking so long reviewing. We have been pretty busy with Mixxx 1.12.0 (which this will be included in).

[ninomp]

Thank you for accepting my contribution. It was a pleasure to work on Mixxx.

[rryan]

@ninomp thank you!

How would you like to be credited in the Mixxx credits? (what name or alias should we use?)

Also, if you haven't already, please sign the Mixxx contributor agreement. This gives us permission to distribute your changes with Mixxx and adds you to our list of copyright holders if we need to contact you in the future:
https://docs.google.com/a/mixxx.org/spreadsheet/viewform?usp=drive_web&formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ#gid=0

[ninomp]

Yes, please. It would be an honor. Can you please use 'Nino MP' as my name / alias.

Btw, I have signed the Mixxx contributor agreement.

[ninomp]

Excuse me, but is there something wrong? I noticed that I wasn't added to contributor list.

Cheers

[daschuer]

Done! 40b5c86
Sorry for the delay.
We are looking forward to your next PR :-)

Thank you very much!