Skip to content

Make CJamStreamer::process data parameter a reference#2

Merged
dingodoppelt merged 1 commit intodingodoppelt:streamer2from
npostavs:streamer2.np.changes
Feb 11, 2021
Merged

Make CJamStreamer::process data parameter a reference#2
dingodoppelt merged 1 commit intodingodoppelt:streamer2from
npostavs:streamer2.np.changes

Conversation

@npostavs
Copy link
Copy Markdown

To avoid redundant copying.

RE: jamulussoftware#967 (comment):

is it in any way possible for you to fork my code and raise a pull request?

Looks like it's possible :)

@dingodoppelt
Copy link
Copy Markdown
Owner

dingodoppelt commented Feb 10, 2021

hi and thanks!
this is what I tried as well, but it won't compile anymore telling me:

src/server.cpp:420:103:   required from here
/usr/include/x86_64-linux-gnu/qt5/QtCore/qobject.h:241:9: error: static assertion failed: Signal and slot arguments are not compatible.

i tried changing the other code (in server.cpp and server.h) as well but couldn't get it to work. could you please try compiling yourself to get a better idea of what is happening there?

@npostavs

This comment has been minimized.

@npostavs npostavs force-pushed the streamer2.np.changes branch from 4f46aa1 to 2fef426 Compare February 10, 2021 19:52
@npostavs
Copy link
Copy Markdown
Author

Okay, I got it working. You have to match the signal declaration in server.h too. And according to this blog post, it should be passed by reference: https://embeddeduse.com/2013/06/29/copied-or-not-copied-arguments-signals-slots/ So possibly the other cases that currently pass by value are bugs.

@dingodoppelt dingodoppelt merged commit 7f5a61d into dingodoppelt:streamer2 Feb 11, 2021
@dingodoppelt
Copy link
Copy Markdown
Owner

thank you very much! happily merged :)

@npostavs npostavs deleted the streamer2.np.changes branch February 11, 2021 13:28
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.

2 participants