Potential fixes for 4 code quality findings

[ann0see]

This PR applies 4/4 suggestions from code quality AI findings.

Only for review. We should then decide if we actually have a memory leak here.

[ann0see]

I don't think everything here is valid.

[ann0see]

This may be valid. Stack vs heap allocation?

[ann0see]

-> memory leak

[softins]

No this won't work. The original does indeed have a memory leak, as the pointer returned from new QSoundEffect() is only local, and disappears at the end of the block. But I think changing it so that the QSoundEffect sf is allocated locally on the stack may cause a crash, or at least truncate the playback, as the object will be destroyed immediately at the end of the block while the sound is still playing.

The correct solution is actually the one shown below around line 910: connect a lambda to the playingChanged signal and call deleteLater() when it is finished with.

[ann0see]

Unsure about those.

[ann0see]

@dcorson-ticino-com ?

[softins]

See 3117acd in #2356 from @henkdegroot

I think the point is that when narrow/small is selected, it should only apply to the mixer channel meters. The above lines refer to the input meters, which need to remain wide with big LEDs, even in narrow mixer mode.

It would be worth adding explanatory comments to that effect, to forestall future confusion.

[softins]

I suggest the following for the whole function:

void CClientDlg::SetMeterStyle ( const EMeterStyle eNewMeterStyle )
{
    // apply MeterStyle to current window
    // Note: input meter uses big LED and wide bar even in narrow mode
    switch ( eNewMeterStyle )
    {
    case MT_LED_STRIPE:
        lbrInputLevelL->SetLevelMeterType ( CLevelMeter::MT_LED_STRIPE );
        lbrInputLevelR->SetLevelMeterType ( CLevelMeter::MT_LED_STRIPE );
        break;

    case MT_LED_ROUND_BIG:
    case MT_LED_ROUND_SMALL:
        lbrInputLevelL->SetLevelMeterType ( CLevelMeter::MT_LED_ROUND_BIG );
        lbrInputLevelR->SetLevelMeterType ( CLevelMeter::MT_LED_ROUND_BIG );
        break;

    case MT_BAR_WIDE:
    case MT_BAR_NARROW:
        lbrInputLevelL->SetLevelMeterType ( CLevelMeter::MT_BAR_WIDE );
        lbrInputLevelR->SetLevelMeterType ( CLevelMeter::MT_BAR_WIDE );
        break;

    }

    // also apply MeterStyle to child GUI controls
    MainMixerBoard->SetMeterStyle ( eNewMeterStyle );
}

[softins]

No this won't work. The original does indeed have a memory leak, as the pointer returned from new QSoundEffect() is only local, and disappears at the end of the block. But I think changing it so that the QSoundEffect sf is allocated locally on the stack may cause a crash, or at least truncate the playback, as the object will be destroyed immediately at the end of the block while the sound is still playing.

The correct solution is actually the one shown below around line 910: connect a lambda to the playingChanged signal and call deleteLater() when it is finished with.

[softins]

This looks ok and is the correct way to avoid the memory leak.

[softins]

See 3117acd in #2356 from @henkdegroot

I think the point is that when narrow/small is selected, it should only apply to the mixer channel meters. The above lines refer to the input meters, which need to remain wide with big LEDs, even in narrow mixer mode.

It would be worth adding explanatory comments to that effect, to forestall future confusion.