Skip to content

Fix save/load config#9

Merged
cboulay merged 1 commit into
labstreaminglayer:masterfrom
ANDRO90:master
Nov 6, 2022
Merged

Fix save/load config#9
cboulay merged 1 commit into
labstreaminglayer:masterfrom
ANDRO90:master

Conversation

@ANDRO90
Copy link
Copy Markdown
Contributor

@ANDRO90 ANDRO90 commented Jun 17, 2020

Additionally, automatically check the loaded audio format

Signed-off-by: Andrea Bellotti andrea.bellotti@outlook.com

Additionally, automatically check the loaded audio format

Signed-off-by: Andrea Bellotti <andrea.bellotti@outlook.com>
@cboulay
Copy link
Copy Markdown
Contributor

cboulay commented Jun 17, 2020

Hi and thanks for the PR.
Are you able to manually test this out on Win / Mac / Linux? i.e., capture audio input, record to LabRecorder, then open the xdf file and play it back? While the changes seem innocuous, I remember when working on this that I was often surprised by how easy it was to give settings that didn't work properly on one platform or another so I preferred loading values from the device instead of from a config file.

Unfortunately the CI systems don't let us create an audio stream and capture it then verify that the data are captured correctly, so there's no automatic way to test.

@ANDRO90
Copy link
Copy Markdown
Contributor Author

ANDRO90 commented Jun 17, 2020

Hi Chad,

I should be able to test everything (apart form the number of mic channels) on all 3 platforms in the coming days.
I'll post a feedback ASAP.

Copy link
Copy Markdown
Contributor

@tstenner tstenner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All settings are index based, so whenever the ordering of options changes (either because the OS feels like it after a restart or a device is added/removed) the settings are lost. OTOH, it's a big improvement right now and the checkAudioFormat() on top should make sure that there are valid settings after loading the file.

@cboulay cboulay merged commit a9da5a8 into labstreaminglayer:master Nov 6, 2022
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