leadedge / ofxNDI

An Openframeworks addon to allow sending and receiving images over a network using the NewTek Network Device Protocol.
GNU General Public License v3.0
134 stars 48 forks source link

Linux on ARM port #12

Closed dropmeaword closed 4 years ago

dropmeaword commented 4 years ago

Hi, thanks for ofxNDI! Awesome work!

I ported it to Linux on ARM, found that the addon was trying to compile WIN32 code on other platforms too so I cleaned that up a little. I split the code of the dynamic loading of libdni into a separate file as this was where most of the platform-specific was to be found.

I also changed __WIN32, etc. for the OF platform defines, since this is an addon for OF we might as well use OFs platform defines.

I commented out the SSE versions of memcpy (you might not want to accept that change) because my target platform (ARM aarch64) doesn't seem to support SSE intrinsics and I couldn't see an easy way of providing alternative implementations.

Not sure if it is wise to integrate this PR directly, it's probably good if we first discuss how to keep intrinsics support in non-Linux platforms...

leadedge commented 4 years ago

Hey that's fantastic. No I don't want to merge anything just yet or it might mess something up. I have to sort out the restructure pull request first.

What I will do is make another branch and we can do what we like with that.

I can mention a couple of things straight away.

There was some work previously with intrinsics and I will have to track that down.

The dynamic loading is necessary to fully comply with Newtek's licencing requirements. To make it seamless for existing applications and avoid needing to install the runtime, I first look for the dlls in the executable folder. It's up to you whether you want to do that.

The receive and send classes can used independently of Openframeworks. I use them for other projects. So any platform defines also have to be independent.

I will make the additional branch shortly.

leadedge commented 4 years ago

OK it all seems to have worked. The new "testing" branch now has your modifications and I will have a look at it.

leadedge commented 4 years ago

Not sure how to communicate here since this closed itself.

I have been through it and made changes so it compiles. Search on "////" throughout.

I haven't done anything about SSE intrinsics for ARM, and I can't test it anyway. Is this useful ?

https://docs.microsoft.com/en-us/cpp/intrinsics/arm-intrinsics?view=vs-2019

dropmeaword commented 4 years ago

Hi, thanks! Sure I get the need for dynamic loading of NewTek's stuff, great work there. It just wasn't compiling for me, that's why I got my hands on it.

I couldn't find much on intrinsics on ARM either. Seems that GCC supports that just fine, might be the target platform, but I find it strange that they wouldn't be suported on the Tegra processor that I am using. In any case, to get stuff rolling on my side I decided to "disable" the optimized memcpy stuff for now, if I see that the performance penalty is too great I will dig my teeth into it later. Thanks for your quick response!

dropmeaword commented 4 years ago

Just seen your integration! Thanks a lot! Glad it worked for you too. Am curious about one thing though, why do you use "cout" for output?

leadedge commented 4 years ago

Those files need to be used independently of Openframeworks so ofLog won't be available. I just put cout to have something in it's place. Maybe use of intrinsics needs a bit more research.

The main thing here is that this will build for ARM as well as Windows. I have to go through it again for OSX and Linux. I can't test it but can compare with the other forks. Are you able to test a build with Linux and/or OSX?

leadedge commented 4 years ago

I am doing a clean-up now that you have seen the changes so stand by for that.

magdesign commented 4 years ago

Just tried to compile on a RPi (using the testing branch) After generating the config.make and Makefile I did a make. But receiver and sender struggles with:

In file included from /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDIsender.h:49:0,
                 from /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDI.h:28,
                 from /home/pvj/openFrameworks/addons/ofxNDI/example-sender/src/ofApp.h:31,
                 from /home/pvj/openFrameworks/addons/ofxNDI/example-sender/src/main.cpp:2:
/home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDIsend.h:61:44: fatal error: shlwapi.h: No such file or directory
 #include <shlwapi.h>  // for path functions

Did I miss something?

leadedge commented 4 years ago

This is excellent that you can help with testing this. I get the feeling that Raspberry Pi could be important for many people. I guess all you missed is that I haven't even looked at the make files and it looks like shlwapi.h is windows only. I will put some defines around that when I get a moment and check that the code excludes it for Linux. But you could do this quickly, perhaps just comment it out. I can't test the make files, but you could help a lot if you can get it working.

magdesign commented 4 years ago

Will keep on testing, used the standard config and makefile. I know that there is no official support for RPi on NDI, but I red that some people are able to compile a working one. We discussed this over here.

dropmeaword commented 4 years ago

@magdesign, thanks for testing this too! I think NewTek offer and embedded SDK with RPi support that you can get here http://new.tk/NDISDKE . I haven't tested the Rpi binaries, but I am using that SDK myself for the Jetson Nano, though I haven't tested all its features yet. Work in progress still.

@leadedge I typically work on an Ubuntu laptop (x64) and deploy on a Jetson Nano running NVIDIA Ubuntu on an ARM (aarch64). In my branch I try to keep everything compiling and tested on both platforms. Indeed shlwapi.h is win only. That's why in the PR I submitted I proposed that this block:

#if defined(TARGET_WIN32)
    #include <windows.h>
    #include <shlwapi.h>  // for path functions
    #include <Shellapi.h> // for shellexecute
    #pragma comment(lib, "shlwapi.lib")  // for path functions
    #pragma comment(lib, "Shell32.lib")  // for shellexecute
#elif defined(TARGET_LINUX) || defined(TARGET_OSX)
    #include <dlfcn.h> // dynamic library loading in Linux
#endif

Be in the .cpp file instead of the .h file. Less hassle for would-be porters. ;)

leadedge commented 4 years ago

@magdesign. Yes if it has been done then it must be possible. I had a quick look and shlwapi.h should be excluded unless TARGET_WIN32 is defined. I have gone through it with such defines in the hope that I got it right, but I can't test it other than for Windows. It's getting close I think.

@dropmeaword. I moved that block of code it to the header because I encountered what is apparently a "circular inclusion" problem as so - "error C3646: 'm_hNDILib': unknown override specifier". If I put it in the header it goes away. I have no idea why.

I will look into the makefiles today.

leadedge commented 4 years ago

From what I can see, addons.make is picking up the master branch rather than the testing branch

As indicated in the output, line 61 in the master branch is -

include // for path functions

In the testing branch it's an empty line.

If the addon source is derived from the openframeworks addons\ofxNDI folder, this would make sense. The output you show does indicate this - /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDIsend.h

I guess you could just back up that folder and copy the testing branch files into it. Then see what happens.

dropmeaword commented 4 years ago

@leadedge oh I see! It has to do with the HMODULE m_hNDILib definition, the HMODULE type is defined somewhere up the chain of shlwapi.h, and because you are using that type in the header it misses it if you do not do the include on top. HMODULE is in fact just a void pointer. See here: windows data types

typedef void *LPVOID;
typedef LPVOID HMODULE;

One way to avoid the messy header part perhaps could be to declare:

void *m_hNDILib;

And then cast that when needed, e.g.:

FreeLibrary((HMODULE)m_hNDILib);

But that gets annoying after a while.

It makes more sense to me to use openFrameworks to solve the platform dependency issues. OF comes with BOOST libraries, and BOOST has a way of working with dynamic libraries. I rewrote the dynamic loader using BOOST here: https://github.com/IDArnhem/ofxNDI/blob/master/src/ofxNDIdynloader.cpp

Works for me on Linux and it doesn't require any platform-specific stuff. I have made a pull-request against your testing branch for your perusal.

dropmeaword commented 4 years ago

@leadedge I have tested the testing branch from your repo in Linux but I haven't really managed to make it work. It was easier for me to get the original master to compile. :/ I think that duplicating the conditional compilation constants in ofxNDIplatforms.h confuses the heck out of gcc and I am not sure what to do about this, because it clashes a bit with the way of doing things in OF addons. I am personally using ofxNDI in an openFrameworks app, so removing that file is no problem for me, but I get the feeling that your use case is different (@leadedge), so I am not sure how to help further. My branch compiles and runs on Linux x64 and aarch64 at the moment, but I am not sure if this is usable for you.

leadedge commented 4 years ago

OK let's call that a failure then. I will remove the platform definition header file and go back to the compiler definitions in those files that are used separately from Openframeworks.

I have been doing some performance testing for the SSE functions and to my surprise the standard memcpy is twice as fast! There has clearly been some optimisation since this was first done. The best performance is with __movsd. In fact the only time this is used is when you receive into a buffer within the application. Data transfer from NDI to texture is handled by Openframeworks using "texture.loadData" (which uses memcpy). So I am thinking that these lower level files could be separated if a character buffer is not necessary.

The problem with using BOOST for the dynamic loader is that it creates a dependency. Again the basic files ofxNDIsend, ofxNDIreceive and ofxNDIutils need to be independent of Openframeworks. They can then be used in any standard windows program. More recently I am using Openframeworks for distributed applications with ofxWinMenu. The result is quite good but there are a lot of dependent dlls and I still need this to work independently of Openframeworks.

I am going through the whole thing again now that we know these things. It's ideal if this can be common for Windows, OSX and Linux and I think it's worth the effort.

dropmeaword commented 4 years ago

@leadedge ok, I'm slightly perplexed by the semi-OF addon approach, but you clearly have your need for this so let's make it work. :) In a way it becomes two wrappers in one. It's interesting. The way I see it, the only real windows-only dependencies this source has are the HMODULE members, some bits in the utils namespace which seem to be non-essential, the header includes that only really need to be in one place and the dynamic loader which we could fix with a little bit more attention to detail. The HMODULE members do not need to be in all the classes, as they only ever seem to be used in the LoadNDI call, that now has it's own encapsulation in ofxNDIdynloader. The only thing the dyn-loader needs to expose in the public interface is a reference to the library binding p_NDILib, which is cross-platform.

I would think that perhaps separating the implementations of the loaders would help in keeping the headers tidy and conflict free. So there could be aofxNDIdynloaderWinImpl.cpp and a ofxNDIdynloaderLinuxImpl.cpp that stay out of each other's way with their respective headers, I could take care of the LinuxImpl and go back to avoiding boost.

Then a ofxNDIplatforms.h that first checks if it is being used from OF (maybe define your own platform constants only if OF_VERSION_MAJOR wasn't first defined by OF).

Just an idea.

leadedge commented 4 years ago

This started as Windows only and was extended for Openframeworks so that the top classes maintain sizes and adjust textures etc. and are much easier to use. I still maintain projects using the basic classes such as these plugins and would like them to build without modification when changes are finalised.

I will remove ofxNDIplatforms. It seemed a good idea at the time but I am sure we can do without it.

OF_VERSION_MAJOR might be just the thing to detect Openframeworks.

The idea of separate loader files is a good thought but we need to cater for OSX as well as Linux. I will have a go at it again. Using BOOST certainly simplifies it. But if code is already there that isn't dependent on BOOST and it works anyway it doesn't seem necessary unless you can do more with it.

.

leadedge commented 4 years ago

Second revision committed. The examples work OK for Windows. Tested with the plugins mentioned above and they also build without issues and function normally. The dynamic loader class has to be independent of Openframeworks for the lower level classes to compile, so the changes to use Boost can't be used. The compiler platform defines seem to work without problems for Windows but I can't test for anything else. Regarding the comment above about "texture.loadData", that's for a receiver. I forgot that the utility memory copy functions are actually used for an Openframeworks sender, including streaming download of texture data from the GPU with PBOs.

magdesign commented 4 years ago

Just tried to build on RPi. Now it complains about xmmintrin.h (on sender and receiver): `In file included from /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDIsend.h:55:0, from /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDIsender.h:49, from /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDI.h:28, from /home/pvj/openFrameworks/addons/ofxNDI/example-receiver/src/ofApp.h:31, from /home/pvj/openFrameworks/addons/ofxNDI/example-receiver/src/main.cpp:2: /home/pvj/openFrameworks/addons/ofxNDI/src/ofxNDIutils.h:49:23: fatal error: xmmintrin.h: No such file or directory

include

                   ^

compilation terminated. `

leadedge commented 4 years ago

Just comment that out and see what happens. Do what you need to do to get it working. and let me know what that is.