Skip to content

Buffer management bug in radio.cc #1493

@Ksk190809

Description

@Ksk190809

Apologies if this is a duplicate -- I didn't see a match in the multiple pages of issues I reviewed.

In the radio::tx() method in lib/src/radio/radio.cc when interpolating (ratio > 1), the buffer argument (passed by reference) has its channel buffer overwritten with the interpolated buffer pointer:

       // Set the buffer pointer
      buffer.set(ch, tx_buffer[ch].data());

In srsUE method phy_common::worker_end() in srsue/src/phy/phy_common.cc there is code to send zeros when the method is_continuous_tx() returns true -- controlled by configuration parameter rf.continuous_tx defaulting to true -- using a statically declared buffer zeroes_multi that gets its channel array pointer overwritten by the above code. Consequently, zeros are transmitted under the continuous_tx mode until after the first time the radio::tx() method is asked to send actual data, placing non-zero content in the buffer now held by both zeroes_multi and the tx_buffer held by the radio object. This results in non-zero data being transmitted when not expected. An easy work-around would be to disable the continuous_tx option, as these transmissions appear to be unnecessary. The following code patch introduces a holding buffer in the radio class to avoid unnecessary copying of data while preserving the integrity of the passed-in buffer argument:

diff --git a/lib/include/srsran/radio/radio.h b/lib/include/srsran/radio/radio.h
index 367ef35d8..c50ecfa65 100644
--- a/lib/include/srsran/radio/radio.h
+++ b/lib/include/srsran/radio/radio.h
@@ -106,6 +106,7 @@ private:
   std::array<std::vector<cf_t>, SRSRAN_MAX_CHANNELS>      dummy_buffers;
   std::mutex                                              tx_mutex;
   std::mutex                                              rx_mutex;
+  rf_buffer_t                                             tx_buffer_holder;
   std::array<std::vector<cf_t>, SRSRAN_MAX_CHANNELS>      tx_buffer;
   std::array<std::vector<cf_t>, SRSRAN_MAX_CHANNELS>      rx_buffer;
   std::array<srsran_resampler_fft_t, SRSRAN_MAX_CHANNELS> interpolators = {};
diff --git a/lib/src/radio/radio.cc b/lib/src/radio/radio.cc
index 8124185e6..700e0d235 100644
--- a/lib/src/radio/radio.cc
+++ b/lib/src/radio/radio.cc
@@ -426,6 +426,12 @@ bool radio::tx(rf_buffer_interface& buffer, const rf_timestamp_interface& tx_tim
   // Get number of samples at the low rate
   uint32_t nof_samples = buffer.get_nof_samples();
 
+  // Make a copy of "buffer" into the holding buffer; use holding buffer thereafter
+  for (uint32_t ch = 0; ch < nof_channels; ch++) {
+    tx_buffer_holder.set(ch, buffer.get(ch));
+  }
+  tx_buffer_holder.set_nof_samples(nof_samples);
+
   // Check that number of the interpolated samples does not exceed the buffer size
   if (ratio > 1 && (size_t)nof_samples * (size_t)ratio > tx_buffer[0].size()) {
     // This is a corner case that could happen during sample rate change transitions, as it does not have a negative
@@ -433,8 +439,8 @@ bool radio::tx(rf_buffer_interface& buffer, const rf_timestamp_interface& tx_tim
     fmt::memory_buffer buff;
     fmt::format_to(buff,
                    "Tx number of samples ({}/{}) exceeds buffer size ({})\n",
-                   buffer.get_nof_samples(),
-                   buffer.get_nof_samples() * ratio,
+                   tx_buffer_holder.get_nof_samples(),
+                   tx_buffer_holder.get_nof_samples() * ratio,
                    tx_buffer[0].size());
     logger.info("%s", to_c_str(buff));
 
@@ -446,18 +452,18 @@ bool radio::tx(rf_buffer_interface& buffer, const rf_timestamp_interface& tx_tim
   if (interpolators[0].ratio > 1) {
     for (uint32_t ch = 0; ch < nof_channels; ch++) {
       // Perform actual interpolation
-      srsran_resampler_fft_run(&interpolators[ch], buffer.get(ch), tx_buffer[ch].data(), nof_samples);
+      srsran_resampler_fft_run(&interpolators[ch], tx_buffer_holder.get(ch), tx_buffer[ch].data(), nof_samples);
 
       // Set the buffer pointer
-      buffer.set(ch, tx_buffer[ch].data());
+      tx_buffer_holder.set(ch, tx_buffer[ch].data());
     }
 
     // Set buffer size after applying the interpolation
-    buffer.set_nof_samples(nof_samples * ratio);
+    tx_buffer_holder.set_nof_samples(nof_samples * ratio);
   }
 
   for (uint32_t device_idx = 0; device_idx < (uint32_t)rf_devices.size(); device_idx++) {
-    ret &= tx_dev(device_idx, buffer, tx_time.get(device_idx));
+    ret &= tx_dev(device_idx, tx_buffer_holder, tx_time.get(device_idx));
   }
 
   is_start_of_burst = false;

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions