Re: [Discuss-gnuradio] QT GUI time sink (float) unnecessary memmove()?

[Andy Walls]

On Sun, 2015-03-29 at 17:20 -0700, Tom Rondeau wrote:
> On Sat, Mar 28, 2015 at 5:32 PM, Andy Walls
> <address@hidden> wrote:


> Andy, if you have a chance, can you check out this new branch:
> 
> 
> https://github.com/trondeau/gnuradio/tree/qtgui/controlpanel
> 
> 
> 
> It adds the fixes that we talked about. I just want to verify that
> things are still looking and behaving well for you.

I had time to inspect the change for time_sink_f_impl.* but haven't had
time to test yet.  What you did was very close to what I did (including
naming the new buffers 'd_fbuffers' :) ), but I will suggest two
improvements:

1. The size of the allocated double 'd_buffers' arrays can now be
reduced from d_buffer_size to d_size, so they only take up 1/2 as much
memory.

2. (I think) You can defer the call to volk_32f_convert_64f() even more,
to when you know for sure plotting is going to happen in the clause that
checks the update time.  See the patch in line below.
It works for me.


> The other trick of this branch is if you go into the QT GUI Time Sink
> properties and turn "Control Panel" to Yes. I wouldn't mind a quick
> bit of feedback there, either.

I'll have some time on Wednesday to recompile and evaluate.

> Tom

Regards,
Andy


diff --git a/gr-qtgui/lib/time_sink_f_impl.cc b/gr-qtgui/lib/time_sink_f_impl.cc
index 8e45e2c..5aa126d 100644
--- a/gr-qtgui/lib/time_sink_f_impl.cc
+++ b/gr-qtgui/lib/time_sink_f_impl.cc
@@ -68,9 +68,12 @@ namespace gr {
       d_main_gui = NULL;
 
       for(int n = 0; n < d_nconnections; n++) {
-       d_buffers.push_back((double*)volk_malloc(d_buffer_size*sizeof(double),
+       d_fbuffers.push_back((float*)volk_malloc(d_buffer_size*sizeof(float),
                                                  volk_get_alignment()));
-       memset(d_buffers[n], 0, d_buffer_size*sizeof(double));
+       memset(d_fbuffers[n], 0, d_buffer_size*sizeof(float));
+       d_buffers.push_back((double*)volk_malloc(d_size*sizeof(double),
+                                                 volk_get_alignment()));
+       memset(d_buffers[n], 0, d_size*sizeof(double));
       }
 
       // Set alignment properties for VOLK
@@ -96,6 +99,7 @@ namespace gr {
 
       // d_main_gui is a qwidget destroyed with its parent
       for(int n = 0; n < d_nconnections; n++) {
+       volk_free(d_fbuffers[n]);
        volk_free(d_buffers[n]);
       }
 
@@ -329,10 +333,14 @@ namespace gr {
 
        // Resize buffers and replace data
        for(int n = 0; n < d_nconnections; n++) {
+         volk_free(d_fbuffers[n]);
+         d_fbuffers[n] = (float*)volk_malloc(d_buffer_size*sizeof(float),
+                                              volk_get_alignment());
+         memset(d_fbuffers[n], 0, d_buffer_size*sizeof(float));
          volk_free(d_buffers[n]);
-         d_buffers[n] = (double*)volk_malloc(d_buffer_size*sizeof(double),
+         d_buffers[n] = (double*)volk_malloc(d_size*sizeof(double),
                                               volk_get_alignment());
-         memset(d_buffers[n], 0, d_buffer_size*sizeof(double));
+         memset(d_buffers[n], 0, d_size*sizeof(double));
        }
 
         // If delay was set beyond the new boundary, pull it back.
@@ -427,7 +435,7 @@ namespace gr {
       int n;
       if(d_trigger_delay) {
         for(n = 0; n < d_nconnections; n++) {
-          memmove(d_buffers[n], &d_buffers[n][d_size-d_trigger_delay], 
d_trigger_delay*sizeof(double));
+          memmove(d_fbuffers[n], &d_fbuffers[n][d_size-d_trigger_delay], 
d_trigger_delay*sizeof(float));
         }
 
         // Also move the offsets of any tags that occur in the tail
@@ -606,8 +614,7 @@ namespace gr {
       // Copy data into the buffers.
       for(n = 0; n < d_nconnections; n++) {
         in = (const float*)input_items[idx];
-        volk_32f_convert_64f(&d_buffers[n][d_index],
-                             &in[1], nitems);
+        memcpy(&d_fbuffers[n][d_index], &in[1], nitems*sizeof(float));
 
         uint64_t nr = nitems_read(idx);
         std::vector<gr::tag_t> tags;
@@ -622,13 +629,13 @@ namespace gr {
 
       // If we've have a trigger and a full d_size of items in the buffers, 
plot.
       if((d_triggered) && (d_index == d_end)) {
-        // Copy data to be plotted to start of buffers.
-        for(n = 0; n < d_nconnections; n++) {
-          memmove(d_buffers[n], &d_buffers[n][d_start], d_size*sizeof(double));
-        }
-
         // Plot if we are able to update
         if(gr::high_res_timer_now() - d_last_time > d_update_time) {
+          // Convert and copy data to be plotted to start of buffers.
+          for(n = 0; n < d_nconnections; n++) {
+            volk_32f_convert_64f(&d_buffers[n][0],
+                                 &d_fbuffers[n][d_start], d_size);
+          }
           d_last_time = gr::high_res_timer_now();
           d_qApplication->postEvent(d_main_gui,
                                     new TimeUpdateEvent(d_buffers, d_size, 
d_tags));
diff --git a/gr-qtgui/lib/time_sink_f_impl.h b/gr-qtgui/lib/time_sink_f_impl.h
index 2da1db9..d8e2261 100644
--- a/gr-qtgui/lib/time_sink_f_impl.h
+++ b/gr-qtgui/lib/time_sink_f_impl.h
@@ -41,6 +41,7 @@ namespace gr {
       int d_nconnections;
 
       int d_index, d_start, d_end;
+      std::vector<float*> d_fbuffers;
       std::vector<double*> d_buffers;
       std::vector< std::vector<gr::tag_t> > d_tags;