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

Oscpack #152

Closed
wants to merge 2 commits into from
Closed

Oscpack #152

wants to merge 2 commits into from

Conversation

daschuer
Copy link
Member

This adds the oscpack package used for OSC communication.

@daschuer
Copy link
Member Author

@JoergAtGithub can you have a look here?

@JoergAtGithub
Copy link
Member

@daschuer I opened already a topic in the core dev group to discuss this. I guess you haven't seen this.

Copy link
Member

@JoergAtGithub JoergAtGithub left a comment

Choose a reason for hiding this comment

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

Please rebase to 2.6

set(VCPKG_LIBRARY_LINKAGE static)
else()
set(VCPKG_LIBRARY_LINKAGE dynamic)
endif()
Copy link
Member

Choose a reason for hiding this comment

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

This is not the right approach. Instead please provide a patch with the missing __declspec(dllexport) declarations upstream and use dynamic linkage according our standards.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have not a big interest to do it now and I also think that a patch is a too fragile solution.
It would be better to do this change via upstream.
Until this has happened, a static build is a good intermediate solution.

Copy link
Member

Choose a reason for hiding this comment

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

It is the common procedure to fix build issues upstream microsoft/vcpkg this way. This shouldn't require an overlay in our repo. An overlay is only appropriate if the upstream rejects a patch.

@daschuer
Copy link
Member Author

daschuer commented Feb 1, 2025

Please rebase to 2.6

I can do it. However the idea was to avoid hassle with an incompatible 2.5 branch for main builds.Will it help you on Windows?

@Eve00000
Copy link

Eve00000 commented Feb 2, 2025

Hi,
Q to be sure. Will my changes to the OSCpack be included in the 'merge into vcpkg' ?
That is very important.
Thanks for taking care.

@daschuer
Copy link
Member Author

daschuer commented Feb 2, 2025

Can you point about which changes are important?
I have tried that but only found code style changes and additional debug messages.

@Eve00000
Copy link

Eve00000 commented Feb 2, 2025

Can you point about which changes are important? I have tried that but only found code style changes and additional debug messages.

indeed codestyle & debug messages

  • changes for arm (values)
  • order changes in calling vars & other files -> important were for older environments
  • changes fot '#if !(defined(x86_64) || defined(_M_X64))"
  • changes for rounding: uint32 RoundUp4( uint32 x ) to uint32 RoundUp4_UInt32(uint32 x)
  • changes of argumentPtr_ to argument_
  • some value_.argumentPtr_ settings

I invested a lot of time in finding these ;-)
If it was only a lib I would have placed it in the libs
see

@daschuer
Copy link
Member Author

daschuer commented Feb 2, 2025

Some of these are only required because you copied the source into the Mixxx tree. I would like to test the upstream version first and than apply all the reaming patches.
@JoergAtGithub to continue here without much hassle. Can we merge this as it is?

@Eve00000
Copy link

Eve00000 commented Feb 2, 2025

I'm not so sure about that considering the ARM.
But OK, test as much as you want, if the test fails I want my work back 😄

@JoergAtGithub
Copy link
Member

I can do it. However the idea was to avoid hassle with an incompatible 2.5 branch for main builds. Will it help you on Windows?

Yes please, I though that was the plan.

Now that 2.5 is in the maintenance phase, it's time for an update. 2.6 should use the new builddenv over the whole beta cycle, but we should use the same buildenv for 2.7alpha as long as possible.

@daschuer
Copy link
Member Author

daschuer commented Feb 2, 2025

OK

@daschuer daschuer closed this Feb 2, 2025
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