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

LB302 - no sound #7411

Closed
1 task done
firewall1110 opened this issue Jul 31, 2024 · 10 comments
Closed
1 task done

LB302 - no sound #7411

firewall1110 opened this issue Jul 31, 2024 · 10 comments
Labels

Comments

@firewall1110
Copy link
Contributor

firewall1110 commented Jul 31, 2024

System Information

Debian Stable, wine32 on Debian Stable, Windows10 64-bit

LMMS Version(s)

lmms-1.3.0-alpha.1.667+g1c865843f

Most Recent Working Version

For LB302: point just after 2024-03-28 19:13:32 Fix glitch in SlicerT #7174

Bug Summary

On start working version(1.102), but the 2-nd part - compiled master (the same with win64 version on Windows 10)
https://github.com/user-attachments/assets/5067ce77-760a-4a21-acc7-14ba4647cb9b
LB302 is OK if use win32 version installed using wine (but SID notes with clicks)

Expected Behaviour

Sound without clicks.
But:
No sound (LB302),
(clicks on every note start (SID))

Steps To Reproduce

Open (attached) project: lay with LB302 and SID

Logs

Click to expand
  

Screenshots / Minimum Reproducible Project

LCuTeml.zip

Please search the issue tracker for existing bug reports before submitting your own.

  • I have searched all existing issues and confirmed that this is not a duplicate.
@firewall1110
Copy link
Contributor Author

Reverting
"Switch to libsamplerate's callback API in Sample" #7361
not fix bug - this bug is NOT connected with #7361

My future plans (Lento Tranquillo , but in parallel):
(1) find point, there this regression happens (I'm not familiar with git - this is good practice for me);
(2) some how replace LB302 (SID) code in master using pre-release code.

@firewall1110
Copy link
Contributor Author

Just compiled at point just after 2024-03-28 19:13:32 Fix glitch in SlicerT (#7174) :
LB302 working, but SID is with clicks.
P.S.
For LB302: pre-release code no more actual.

@firewall1110
Copy link
Contributor Author

firewall1110 commented Aug 3, 2024

It seems is "Mate in 2":
(in folder plugins/Lb302/)
if file Lb302.h (~217 line):
should change
int release_frame;
to
f_cnt_t release_frame;
In file Lb302.cpp (~756 line):
should change
release_frame = std::max(release_frame, static_cast<int>(_n->framesLeft()) + static_cast<int>(_n->offset()));
to
release_frame = std::max(release_frame, _n->framesLeft() + _n->offset());
(as it was before, except std::max)
P.S.:
But I wander: why release_frame has so big value:
Before: ~1073741823 (int32_t)
Now: ~9223372036854775807(size_t : unsigned long)
It is something wrong to give NotePlayHandle object so big working parameter values ...

@firewall1110
Copy link
Contributor Author

firewall1110 commented Aug 5, 2024

For SID: "conflict wall" at point after "Fix uninitialized variable "pCurPreset" (#6639) (#6723)" (2023-06-05 06:44:48)
Can not compile : new fetched code has conflicts with this revision.
P.S.
In new master bug is not "magically disappeared".

@firewall1110
Copy link
Contributor Author

Now I compare Master with PreRelase : 1-st iteration do not give a result ...
(1) My opinion - old code more readable, compact and elegant, than new "cool C++ style", but it should work in the same way as old code. (There are some points, but if bug is there - sound effect should be different.)
(2) On every note SID send some kind of pattern - and this make click. This is old chip emulator and I do not like this on my flute like setting, but may be this is not bug but emulated chip sound?
(3) May be bug is in plugins/Sid/resid/ , downloaded pre release zip not contain this ...

@firewall1110
Copy link
Contributor Author

firewall1110 commented Aug 6, 2024

I will pause 10 days waiting answers:
(1) I gave enough information to fix Lb302 no sound bug. Can someone, who has write access simply commit this 2 line changes to master?
(2) Can someone from maintainers/members (best variant - SID authors) confirm, that SID sound with start clicks now is a bug? (and SID should sound as in pre release)
(Edited:)P.S.
With Jack SID now introduce:

JackEngine::XRun: client = lmms was not finished, state = Triggered
JackAudioDriver::ProcessGraphAsyncMaster: Process error
port created: LMMS:midi/capture_22
ALSA-sequencer: unhandled input event 66
...

But no problems in pre release ...
So SID is buggy and it is serious regression ...

@firewall1110
Copy link
Contributor Author

So 10 days over and I have no SID sound bug confirmation: assuming SID should produce clicks on every note start.

SID XRun bug should be another issuer opened, and this XRun can be reproduced using SDL audio back-end as project sound defect (really - pause) on 1-st SID note played.

I will make new issue soon, than edit this one.

@firewall1110 firewall1110 changed the title LB302 - no sound, SID - clicks on note start LB302 - no sound Aug 19, 2024
@firewall1110
Copy link
Contributor Author

Now my plan is to try to find way to fix "why release_frame has so big value".

This big value now are negative values castled to unsigned long (size_t).

Mate in 2 fix "chess blunder" in :
Define fpp_tandf_cnt_tto be ofsize_t (#7363)

Such change introduce a lot of compiler errors and warnings, at first time (some of) were simply castled. (Refactoring PR) Author forgot about this one line. Also Mate in 2 make release_frame "type consistent" : LB302 implementation author ignore lmms_basics.h type using C/C++ base type - "base type" for f_cnt_t before Define fpp_tandf_cnt_tto be ofsize_t (#7363) .

But it seems, that f_cnt_t:int32_t -> size_t introduce some hidden problems ... Joke is that in 90% CPU treat signed and unsigned types in the same way - so code is working even if underflow happens by negative values used before.

But there are 10% ... One of the such signed and unsigned difference happens, than type size is expanded.
On 32-bit platforms sizeof(size_t) == sizeof(int32_t), but this is not true on 64-bit platforms.
May be this is a answer to question: "Why LB302 have sound on MinGW32 compiled version".

@firewall1110
Copy link
Contributor Author

It seems that Fix Lb302 silence #7504 should fix this bug, but I have not tested this yet.

P.S.
PR is closed, issue not mentioned, so I connect ...

@zonkmachine
Copy link
Member

It seems that Fix Lb302 silence #7504 should fix this bug, but I have not tested this yet.

Yes. The fix is already in master.
This issue is closed with #7504

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants