OpenTimelineIO / OpenTimelineIO-C-Bindings

C bindings for the OpenTimelineIO Library (http://opentimeline.io)
http://opentimeline.io
Apache License 2.0
11 stars 7 forks source link

Some symbols are not properly defined as extern #22

Closed hisergiorojas closed 1 year ago

hisergiorojas commented 2 years ago

It works fine on Mac however on Ubuntu 21.04 I got this error.


/usr/bin/ld: ../src/copentimelineio/libcopentimelineio.a(track.cpp.o):(.data+0x8): multiple definition of `TrackKind_Audio'; CMakeFiles/COTIOCompositionTests.dir/OTIOCompositionTests.c.o:(.bss+0x60): first defined here
/usr/bin/ld: ../src/copentimelineio/libcopentimelineio.a(track.cpp.o):(.data+0x0): multiple definition of `TrackKind_Video'; CMakeFiles/COTIOCompositionTests.dir/OTIOCompositionTests.c.o:(.bss+0x58): first defined here
clang: error: linker command failed with exit code 1 (use -v to see invocation)
gmake[2]: *** [tests/CMakeFiles/COTIOCompositionTests.dir/build.make:103: tests/COTIOCompositionTests] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1341: tests/CMakeFiles/COTIOCompositionTests.dir/all] Error 2
gmake: *** [Makefile:166: all] Error 2
scott-wilson commented 2 years ago

I was able to reproduce the above issue in Ubuntu 21.10 using the build instructions here: https://github.com/OpenTimelineIO/OpenTimelineIO-C-Bindings#building-opentimelineio-c-bindings

KarthikRIyer commented 2 years ago

I wasn't able to reproduce this issue.

Tried with both clang and gcc.

Versions

gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
clang version 10.0.0-4ubuntu1
GNU ld (GNU Binutils for Ubuntu) 2.34
hisergiorojas commented 2 years ago

I was able to build the c binding on Github Codespace, which is running on

Linux codespaces_d0119b 5.4.0-1067-azure #70~18.04.1-Ubuntu SMP Thu Jan 13 19:46:01 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Screenshot 2-4-2022 6-56-37 PM

hisergiorojas commented 2 years ago

This is command to reproduce the error:

/usr/bin/make -f tests/CMakeFiles/COTIOCompositionTests.dir/build.make tests/CMakeFiles/COTIOCompositionTests.dir/build
hisergiorojas commented 2 years ago

I think I found more information about the problem, under C language issues Default to -fno-common section. https://gcc.gnu.org/gcc-10/porting_to.html

hisergiorojas commented 2 years ago

I kind of understand the error more. It was complaining that there were two OTIO_API const char *TrackKind_Video. The first one is in track.h file and the other one was in track.cpp

https://github.com/OpenTimelineIO/OpenTimelineIO-C-Bindings/blob/main/include/copentimelineio/track.h#L31 https://github.com/OpenTimelineIO/OpenTimelineIO-C-Bindings/blob/main/src/copentimelineio/track.cpp#L39

meshula commented 1 year ago

Thanks for that diagnosis, and sorry for a late response. We should look at these in headers, and check for further instances:

OTIO_API const char *TrackKind_Video;
OTIO_API const char *TrackKind_Audio;

Rather than fixing these with macro spaghetti, the fix should be to replace these extern references with accessor functions.

JeanChristopheMorinPerso commented 1 year ago

I'm guessing something like

index 82c507f..2e4b36b 100644
--- a/include/copentimelineio/track.h
+++ b/include/copentimelineio/track.h
@@ -31,8 +31,8 @@ typedef enum {
 } OTIO_Track_NeighbourGapPolicy_;
 typedef int OTIO_Track_NeighbourGapPolicy;

-OTIO_API const char *TrackKind_Video;
-OTIO_API const char *TrackKind_Audio;
+extern const char *TrackKind_Video;
+extern const char *TrackKind_Audio;

 OTIO_API Track *Track_create(
         const char *name,
diff --git a/src/copentimelineio/track.cpp b/src/copentimelineio/track.cpp
index f15e8e0..1edd36a 100644
--- a/src/copentimelineio/track.cpp
+++ b/src/copentimelineio/track.cpp
@@ -39,8 +39,8 @@ typedef std::vector<OTIO_NS::Composable *> ComposableVectorDef;
 typedef std::vector<OTIO_NS::Composable *>::iterator ComposableVectorIteratorDef;

-OTIO_API const char *TrackKind_Video = OTIO_NS::Track::Kind::video;
-OTIO_API const char *TrackKind_Audio = OTIO_NS::Track::Kind::audio;
+const char *TrackKind_Video = OTIO_NS::Track::Kind::video;
+const char *TrackKind_Audio = OTIO_NS::Track::Kind::audio;

 OTIO_API Track *Track_create(
         const char *name,

won't work? Because it seems to work locally for me (on GCC 12.2). Tests pass and the compiler seems happy. Which means I might be missing something (which wouldn't surprised me since C is pretty mysterious to me).

meshula commented 1 year ago

That's a great hint. With that in mind, I looked at some other projects, I think the right answer is to add an OTIO_API_EXPORT_CONST.

#if defined(_MSC_VER) || defined(__MINGW32__)
#define OTIO_API_EXPORT_CONST OTIO_API
#else
#define OTIO_API_EXPORT_CONST
#endif