search

JvanKatwijk / dab-cmdline

DAB decoding library with example of its use
GNU General Public License v2.0
57 stars 29 forks source link

Create mot files in binary mode #75

Closed srcejon closed 3 years ago

srcejon commented 3 years ago

Otherwise they are corrupted on Windows. (Should be valid for Linux too)

JvanKatwijk commented 3 years ago

Good thinking, you see that I am not a Windows person

andimik commented 3 years ago

@srcejon

Could you please also check if the code in Qt-DAB also might be optimized?

JvanKatwijk commented 3 years ago

can you share your MSVC prroject files? Then I might take a big (mental) step and look into windows development

Op vr 23 apr. 2021 om 15:32 schreef srcejon @.***>:

Otherwise they are corrupted on Windows. (Should be valid for Linux too)

You can view, comment on, or merge this pull request online at:

https://github.com/JvanKatwijk/dab-cmdline/pull/75 Commit Summary

  • Create mot files in binary mode

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/pull/75, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQAGQYUUUBBEQYUVVZLTKFZH3ANCNFSM43OVA67Q .

-- Jan van Katwijk

srcejon commented 3 years ago

cmake will generate them for you. However, you'll get much faster builds if you use ninja (https://ninja-build.org/). To get cmake to generate ninja files, do 'cmake -G Ninja' then run 'ninja'

The bigger issue is building the other libraries you need. You should be able to pull these from SDRangel though. Windows build instructions here:

https://github.com/f4exb/sdrangel/wiki/Compile-in-Windows

That will actually checkout and build dab_lib along with it's dependencies.

If you then want to build it separately, you should just be able to point to the versions it has built / precompiled:

(For the library) cmake -G Ninja --config Release -DPTHREADS=C:...\sdrangel\build\external\pthreads4w\src\pthreads4w\pthreadVC2.lib -DZLIB_INCLUDE_DIR=C:...\sdrangel\external\windows\zlib\include -DZLIB_LIBRARY=C:...\sdrangel\external\windows\zlib\lib\zlibstaticd.lib -DPC_FFTW3F_INCLUDE_DIR=C:...\sdrangel\external\windows\fftw-3\include -DPC_FFTW3F_LIBDIR=C:...\sdrangel\external\windows\fftw-3\ -DFAAD_INCLUDE_DIR=C:...\sdrangel\external\windows\faad2\include -DFAAD_LIBRARY=CC:...\sdrangel\external\windows\lib\libfaad.lib ..

And this will also give you windows versions of driver libraries for various SDRs etc if you want to get the examples working.

srcejon commented 3 years ago

One thing to perhaps think about is using std::string in the API (as you recently made the other change to the API). I think it is somewhat frowned upon, as:

What to use instead? Maybe just char * pointing to UTF-8 - but perhaps there is a better solution.

JvanKatwijk commented 3 years ago

It was always a mix between C++ and C, and as said earlier, it was never the intension to have it run under Windows (windows is a black box with widgets, not very friendly got command line driven applications) It was - probably said before as well - basically a gadget, written together with the eti generator.

Originally all character strings were char * things. Looking at what you wrote, I think the best option is first to scan the code to see what kind of operations are used in the library code on it.

Op di 27 apr. 2021 om 11:51 schreef srcejon @.***>:

One thing to perhaps think about is using std::string in the API (as you recently made the other change to the API). I think it is somewhat frowned upon, as:

  • They're not always compatible between different versions of c++ libraries as the ABI can change between versions.
  • With MSVC, Debug and Release builds are incompatible (so you can't use a release build of dab_lib.dll with a Debug build of a program that uses it)
  • std::string doesn't support Unicode on Windows (as Windows uses UTF-16 rather than UTF-8, so I believe you need std::wstring instead)

What to use instead? Maybe just char * pointing to UTF-8 - but perhaps there is a better solution.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/pull/75#issuecomment-827475955, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQDKRCS2L27G6LFF33LTK2CLHANCNFSM43OVA67Q .

-- Jan van Katwijk

JvanKatwijk commented 3 years ago

Thinking about strings: The data encoded within the DAB data is all utf-8, so basically the library should be based on utf-8 encoded strings. The caller of the library, i.e. the main program, provides the external interface, so that program should be responsible for mapping the strings as used in the user interface to and from utf-8.

to be continued

Op di 27 apr. 2021 om 19:53 schreef jan van katwijk @.***

:

It was always a mix between C++ and C, and as said earlier, it was never the intension to have it run under Windows (windows is a black box with widgets, not very friendly got command line driven applications) It was - probably said before as well - basically a gadget, written together with the eti generator.

Originally all character strings were char * things. Looking at what you wrote, I think the best option is first to scan the code to see what kind of operations are used in the library code on it.

Op di 27 apr. 2021 om 11:51 schreef srcejon @.***>:

One thing to perhaps think about is using std::string in the API (as you recently made the other change to the API). I think it is somewhat frowned upon, as:

  • They're not always compatible between different versions of c++ libraries as the ABI can change between versions.
  • With MSVC, Debug and Release builds are incompatible (so you can't use a release build of dab_lib.dll with a Debug build of a program that uses it)
  • std::string doesn't support Unicode on Windows (as Windows uses UTF-16 rather than UTF-8, so I believe you need std::wstring instead)

What to use instead? Maybe just char * pointing to UTF-8 - but perhaps there is a better solution.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/pull/75#issuecomment-827475955, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQDKRCS2L27G6LFF33LTK2CLHANCNFSM43OVA67Q .

-- Jan van Katwijk

-- Jan van Katwijk

srcejon commented 3 years ago

Yep, sounds reasonable. This is actually working on Windows in SDRangel at the moment for program names via std::string as we do that conversion.

One outstanding issue within the library is with MOT filenames though. On Windows, fopen doesn't support UTF-8, so the path and filename would need to be converted to wchar_t and _wfopen called instead, I think. That's a pretty simple change though.

JvanKatwijk commented 3 years ago

The current handling of mot objects is rather inconsistent with the while idea of a - more or less - complete interface. A better approach would be to pass on the mot data - which is packed in a std::vector - together with the name in a callback function, It is upto the user whether or not something is to be stored or not, and if so, the user should be resposnsible

If that approach is taken, then all results from calling api functions are passed back to the user through call backs, if the basic interface uses utf 8, then, if a user want utf 16 or whatever, a simple layer would be sufficient.

Op wo 28 apr. 2021 om 11:07 schreef srcejon @.***>:

Yep, sounds reasonable. This is actually working on Windows in SDRangel at the moment for program names via std::string as we do that conversion.

One outstanding issue within the library is with MOT filenames though. On Windows, fopen doesn't support UTF-8, so the path and filename would need to be converted to wchar_t and _wfopen called instead, I think. That's a pretty simple change though.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/pull/75#issuecomment-828289478, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQHEMR4OI35CMVF7DFLTK7F65ANCNFSM43OVA67Q .

-- Jan van Katwijk

srcejon commented 3 years ago

Yep. Might want to just use char * and a length if you want the API to be accessible from C.

JvanKatwijk commented 3 years ago

That isbasically the original idea of the api: being callable from C. I'll do some redesiging and rewriting

Op wo 28 apr. 2021 om 17:08 schreef srcejon @.***>:

Yep. Might want to just use char * and a length if you want the API to be accessible from C.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/pull/75#issuecomment-828534345, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQABUFMMAFS5EEBNODDTLAQIFANCNFSM43OVA67Q .

-- Jan van Katwijk

JvanKatwijk commented 3 years ago

The dab-api interface is now std::string free, the examples 1 .. 6 adapted some other examples removed. Of course the implementation of the library is not (yet?) std::string free

Modification for motdfata: function is called with as additional parameters a pointer to the data, and the size of the data. In the current version the name is (a const char *) still extended with /tmp

Op wo 28 apr. 2021 om 17:50 schreef jan van katwijk @.***

:

That isbasically the original idea of the api: being callable from C. I'll do some redesiging and rewriting

Op wo 28 apr. 2021 om 17:08 schreef srcejon @.***>:

Yep. Might want to just use char * and a length if you want the API to be accessible from C.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/pull/75#issuecomment-828534345, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQABUFMMAFS5EEBNODDTLAQIFANCNFSM43OVA67Q .

-- Jan van Katwijk

-- Jan van Katwijk

srcejon commented 3 years ago

Thanks.

Passing mot data as an array has actually fixed a small problem I've just noticed - a few programs include a directory in the filename. As this dir doesn't exist, fopen was failing, so the files were never written.

I don't think you need to remove internal use of std::string. It's only in the API where there's potentially issues.

andimik commented 3 years ago

Yes, RAI in Italy or ORS in Austria use a directory