KejPi / AbracaDABra

Abraca DAB radio: DAB/DAB+ Software Defined Radio (SDR)
MIT License
67 stars 8 forks source link

Some MSVC/clang-cl build issues #38

Open gvanem opened 2 years ago

gvanem commented 2 years ago

Building AbracaDABra.exe using MSVC or clang-cl has been very easy (compared to e.g. Welle-IO or Qt-DAB (yuk!)). But I had some minor issues:

And PS, it would be nicer if the DAB date/time had seconds printed:

--- a/gui/mainwindow.cpp 2022-09-01 10:45:57
+++ b/gui/mainwindow.cpp 2022-09-01 12:43:24
@@ -694,7 +694,7 @@

 void MainWindow::updateDabTime(const QDateTime & d)
 {
-    timeLabel->setText(d.toString(QString("dddd, dd.MM.yyyy, hh:mm")));
+    timeLabel->setText(d.toString(QString("dddd, dd.MM.yyyy, hh:mm:ss")));
 }
gvanem commented 2 years ago

I forgot this issue: error C4716: 'SPIApp::parseAttributes': must return a value. Fixed by:

--- a/gui/spiapp.cpp 2022-07-26 11:29:08
+++ b/gui/spiapp.cpp 2022-09-01 13:12:30
@@ -614,4 +614,5 @@
     default:
         break;
     }
+    return nullptr;
 }
KejPi commented 2 years ago

bool muteRequest = m_muteFlag | m_stopFlag; // false == do unmute, true == do mute Shouldn't it be bool muteRequest = m_muteFlag || m_stopFlag;? Same issue on line 791.

Correct, I will fix that stupid mistake, thanks!

  • MSVC fails on lines like: #warning "Best ensemble to be implemented". But a #pragma message ("Best ensemble to be implemented") works. Looks like these are for internal use. So could they be dropped?

I would say that this is caused by strict compiler settings when warning leads to error. Normally #warning should just do warning, but you are right, these are for me to remember, I will remove it to clean-up the compilation.

And PS, it would be nicer if the DAB date/time had seconds printed:

--- a/gui/mainwindow.cpp 2022-09-01 10:45:57
+++ b/gui/mainwindow.cpp 2022-09-01 12:43:24
@@ -694,7 +694,7 @@

 void MainWindow::updateDabTime(const QDateTime & d)
 {
-    timeLabel->setText(d.toString(QString("dddd, dd.MM.yyyy, hh:mm")));
+    timeLabel->setText(d.toString(QString("dddd, dd.MM.yyyy, hh:mm:ss")));
 }

No, if you try, you will see that is it not behaving as you would like, the update is not fast enough.

KejPi commented 2 years ago

I forgot this issue: error C4716: 'SPIApp::parseAttributes': must return a value. Fixed by:

--- a/gui/spiapp.cpp 2022-07-26 11:29:08
+++ b/gui/spiapp.cpp 2022-09-01 13:12:30
@@ -614,4 +614,5 @@
     default:
         break;
     }
+    return nullptr;
 }

The same issue as above - compiler is too strict and interprets warnings as error. But it agree to remove this warning, SPI application code was just a trial that I did to remove from the repository. The code is not working and not used, I hope I will find some time to work on that in future.

gvanem commented 2 years ago

I would say that this is caused by strict compiler settings when warning leads to error.

That's not it. MSVC does not parse a #warning statement.

KejPi commented 2 years ago

I hate that compiler, always something special :-/

gvanem commented 2 years ago

you will see that is it not behaving as you would like, the update is not fast enough.

I can see it jumps over a second often. But isn't that's due to rounding errors of the milli-sec part?

gvanem commented 2 years ago

I hate that compiler, always something special :-/

But it's fast. With cl -MP all-sources-except-the-MOC-files, it completes in 10 seconds here.

KejPi commented 2 years ago

you will see that is it not behaving as you would like, the update is not fast enough.

I can see it jumps over a second often. But isn't that's due to rounding errors of the milli-sec part?

I would say it is because it gets updated approx. every 800ms. To make is running smoothly we would need probably half of this period. This would increase CPU load for nothing I would say. Still, with all the delays in the system it will not be precise time.

gvanem commented 2 years ago

The new gui/soapysdrinpuyt.cpp does not compile due to the use of VLA. Errors:

gui/soapysdrinput.cpp(494): error C2956: usual deallocation function
'void operator delete[](void *,std::align_val_t) noexcept' would be chosen as placement deallocation function.
predefined C++ types (compiler internal)(62): note: see declaration of 'operator delete[]'
...
gui/soapysdrinput.cpp(600): error C2131: expression did not evaluate to a constant
gui/soapysdrinput.cpp(600): note: failure was caused by a read of a variable outside its lifetime
gui/soapysdrinput.cpp(600): note: see usage of 'len'
gui/soapysdrinput.cpp(603): error C3863: array type 'int16_t [len/4]' is not assignable

which I fixed by:

--- a/gui/soapysdrinput.cpp 2022-11-05 14:02:33
+++ b/gui/soapysdrinput.cpp 2022-11-05 14:57:03
@@ -491,14 +491,22 @@
     m_rxChannel = rxChannel;

     // we cannot produce more samples in SRC
+#ifdef _MSC_VER
+    m_filterOutBuffer = (float*) _aligned_malloc (sizeof(float)*SOAPYSDR_INPUT_SAMPLES*2, 16);
+#else
     m_filterOutBuffer = new ( std::align_val_t(16) ) float[SOAPYSDR_INPUT_SAMPLES*2];
+#endif
     m_src = new InputDeviceSRC(sampleRate);
 }

 SoapySdrWorker::~SoapySdrWorker()
 {
     delete m_src;
+#ifdef _MSC_VER
+    _aligned_free (m_filterOutBuffer);
+#else
     operator delete [] (m_filterOutBuffer, std::align_val_t(16));
+#endif
 }

 void SoapySdrWorker::run()
@@ -597,7 +605,7 @@
 #if SOAPYSDR_DUMP_INT16
         // dumping in int16
         float * floatBuf = (float *) buf;
-        int16_t int16Buf[len/sizeof(float)];
+        int16_t int16Buf[SOAPYSDR_INPUT_SAMPLES*2];
         for (int n = 0; n < len/sizeof(float); ++n)
         {
             int16Buf[n] = *floatBuf++ * SOAPYSDR_DUMP_FLOAT2INT16;
KejPi commented 2 years ago

Could you please try with b24d853e018a019549df18fadb0a2e6952c011a6?

gvanem commented 2 years ago

Works fine!

KejPi commented 2 years ago

Great, I will leave this issue open for further incompatibilities of MSVC with C++ standard ;-)

gvanem commented 1 year ago

Another minor issue which I continue here: Enabling MOTOBJECT_VERBOSE, the MOTDirectory class member is named m_carousel (not carousel):

--- a/gui/motobject.cpp 2022-11-05 14:02:33
+++ b/gui/motobject.cpp 2023-01-04 14:31:35
@@ -400,7 +400,7 @@
     if (m_carousel->end() == it)
     {  // object does not exist in carousel - this should not happen for current directory
 #if MOTOBJECT_VERBOSE
-        qDebug() << "New MOT object" << transportId << "number of objects in carousel" << carousel->size();
+        qDebug() << "New MOT object" << transportId << "number of objects in carousel" << m_carousel->size();
 #endif
         // add new object to cache
         it = m_carousel->addMotObj(MOTObject(transportId));
@@ -560,7 +560,7 @@

 #if MOTOBJECT_VERBOSE
     qDebug() << "MOT directory carousel contents:";
-    for (MOTObjectCache::const_iterator it = carousel->cbegin(); it < carousel->cend(); ++it)
+    for (MOTObjectCache::const_iterator it = m_carousel->cbegin(); it < m_carousel->cend(); ++it)
     {
         qDebug("\tID: %d, isComplete = %d, body size = %lld, name = %s", it->getId(), it->isComplete(), it->getBody().size(), it->getContentName().toLocal8Bit().data());