Ticket #11852: 0001-BTimeSource-cleanup-rework-slave-nodes-management-wh.patch

src/kits/media/TimeSource.cpp

@@ -1 +1 @@
 /*
- * Copyright 2002-2012, Haiku. All Rights Reserved.
- * This file may be used under the terms of the MIT License.
+ * Copyright 2002-2015 Haiku, Inc. All rights reserved.
+ * Distributed under the terms of the MIT License.
  *
- * Author: Marcus Overhagen
+ * Authors:
+ *      Marcus Overhagen
+ *      Dario Casalinuovo
  */
 
 
-#include <TimeSource.h>
 #include <Autolock.h>
+#include <ObjectList.h>
+#include <TimeSource.h>
+
 #include <string.h>
+
 #include "debug.h"
 #include "DataExchange.h"
 #include "ServerInterface.h"
@@ -26 +31 @@
 
 #define _atomic_read(p)     atomic_or((p), 0)
 
-#define TS_AREA_SIZE        B_PAGE_SIZE     // must be multiple of page size
-#define TS_INDEX_COUNT      128             // must be power of two
+// must be multiple of page size
+#define TS_AREA_SIZE        B_PAGE_SIZE
+// must be power of two
+#define TS_INDEX_COUNT      128
 
-struct TimeSourceTransmit // sizeof(TimeSourceTransmit) must be <= TS_AREA_SIZE
+// sizeof(TimeSourceTransmit) must be <= TS_AREA_SIZE
+struct TimeSourceTransmit
 {
     int32 readindex;
     int32 writeindex;
@@ -39 +47 @@
     float drift[TS_INDEX_COUNT];
 };
 
-#define SLAVE_NODES_COUNT 300
+#define MAX_SLAVE_NODES 300
 
-// XXX TODO: storage for slave nodes uses public data members, this should be changed
 
-class SlaveNodes
+class SlaveNode
 {
 public:
-                    SlaveNodes();
-                    ~SlaveNodes();
+                            SlaveNode(media_node_id nodeId, port_id nodePort);
+
+    media_node_id           NodeId() const;
+    port_id                 NodePort() const;
+
+private:
+    media_node_id           fNodeId;
+    port_id                 fNodePort;
+};
+
+
+SlaveNode::SlaveNode(media_node_id nodeId, port_id nodePort)
+{
+    fNodeId = nodeId;
+    fNodePort = nodePort;
+}
+
+
+media_node_id
+SlaveNode::NodeId() const
+{
+    return fNodeId;
+}
+
+
+port_id
+SlaveNode::NodePort() const
+{
+    return fNodePort;
+}
+
+
+class SlaveNodes : public BLocker
+{
 public:
-    BLocker *       locker;
-    int32           count;
-    media_node_id   node_id[SLAVE_NODES_COUNT];
-    port_id         node_port[SLAVE_NODES_COUNT];
+                            SlaveNodes();
+
+    int32                   CountSlaves() const;
+
+    port_id                 PortAt(int32 index) const;
+    SlaveNode*              SlaveAt(int32 index) const;
+
+    bool                    InsertSlave(const media_node& node);
+    bool                    RemoveSlave(const media_node& node);
+
+private:
+    BObjectList<SlaveNode>  fSlaveList;
 };
 
 
 SlaveNodes::SlaveNodes()
 {
-    locker = new BLocker("BTimeSource SlaveNodes");
-    count = 0;
-    memset(node_id, 0, sizeof(node_id));
-    memset(node_port, 0, sizeof(node_port));
+    fSlaveList = BObjectList<SlaveNode>(true);
+}
+
+
+int32
+SlaveNodes::CountSlaves() const
+{
+    return fSlaveList.CountItems();
 }
 
 
-SlaveNodes::~SlaveNodes()
+port_id
+SlaveNodes::PortAt(int32 index) const
 {
-    delete locker;
+    return SlaveAt(index)->NodePort();
 }
 
 
-} }
+SlaveNode*
+SlaveNodes::SlaveAt(int32 index) const
+{
+    return fSlaveList.ItemAt(index);
+}
+
+
+bool
+SlaveNodes::InsertSlave(const media_node& node)
+{
+    return fSlaveList.AddItem(new SlaveNode(node.node, node.port));
+}
+
+
+bool
+SlaveNodes::RemoveSlave(const media_node& node)
+{
+    for (int i = 0; i < CountSlaves(); i++) {
+        SlaveNode* slave = fSlaveList.ItemAt(i);
+        if (slave->NodeId() == node.node
+            && slave->NodePort() == node.port) {
+            return fSlaveList.RemoveItemAt(i);
+        }
+    }
+    return false;
+}
+
+
+} } // namespace BPrivate::media
 
 
 /*************************************************************
@@ -92 +172 @@
 
 status_t
 BTimeSource::SnoozeUntil(bigtime_t performance_time,
-                         bigtime_t with_latency,
-                         bool retry_signals)
+    bigtime_t with_latency, bool retry_signals)
 {
     CALLED();
     bigtime_t time;
@@ -125 +204 @@
     if (GetTime(&last_perf_time, &last_real_time, &last_drift) != B_OK)
         debugger("BTimeSource::PerformanceTimeFor: GetTime failed");
 
-    return last_perf_time + (bigtime_t)((real_time - last_real_time) * last_drift);
+    return last_perf_time
+        + (bigtime_t)((real_time - last_real_time) * last_drift);
 }
 
 
 bigtime_t
 BTimeSource::RealTimeFor(bigtime_t performance_time,
-                         bigtime_t with_latency)
+    bigtime_t with_latency)
 {
     PRINT(8, "CALLED BTimeSource::RealTimeFor()\n");
 
@@ -146 +226 @@
     if (GetTime(&last_perf_time, &last_real_time, &last_drift) != B_OK)
         debugger("BTimeSource::RealTimeFor: GetTime failed");
 
-    return last_real_time - with_latency + (bigtime_t)((performance_time - last_perf_time) / last_drift);
+    return last_real_time - with_latency
+        + (bigtime_t)((performance_time - last_perf_time) / last_drift);
 }
 
 
@@ -157 +238 @@
 
     bool isrunning;
 
+    // The system time source is always running
     if (fIsRealtime)
-        isrunning = true; // The system time source is always running :)
+        isrunning = true;
     else
         isrunning = fBuf ? atomic_add(&fBuf->isrunning, 0) : fStarted;
 
@@ -169 +251 @@
 
 
 status_t
-BTimeSource::GetTime(bigtime_t *performance_time,
-                     bigtime_t *real_time,
-                     float *drift)
+BTimeSource::GetTime(bigtime_t* performance_time,
+    bigtime_t* real_time, float* drift)
 {
     PRINT(8, "CALLED BTimeSource::GetTime()\n");
 
@@ -180 +261 @@
         *drift = 1.0f;
         return B_OK;
     }
-//  if (fBuf == 0) {
-//      PRINT(1, "BTimeSource::GetTime: fBuf == 0, name %s, id %ld\n",Name(),ID());
-//      *performance_time = *real_time = system_time();
-//      *drift = 1.0f;
-//      return B_OK;
-//  }
 
     int32 index;
     index = _atomic_read(&fBuf->readindex);
@@ -194 +269 @@
     *performance_time = fBuf->perftime[index];
     *drift = fBuf->drift[index];
 
-//  if (*real_time == 0) {
-//      *performance_time = *real_time = system_time();
-//      *drift = 1.0f;
-//      return B_OK;
-//  }
-//  printf("BTimeSource::GetTime timesource %ld, index %ld, perf %16Ld, real %16Ld, drift %2.2f\n", ID(), index, *performance_time, *real_time, *drift);
-
     TRACE_TIMESOURCE("BTimeSource::GetTime     timesource %" B_PRId32
         ", perf %16" B_PRId64 ", real %16" B_PRId64 ", drift %2.2f\n", ID(),
         *performance_time, *real_time, *drift);
@@ -217 +285 @@
 
 
 status_t
-BTimeSource::GetStartLatency(bigtime_t *out_latency)
+BTimeSource::GetStartLatency(bigtime_t* out_latency)
 {
     CALLED();
     *out_latency = 0;
@@ -229 +297 @@
  *************************************************************/
 
 
-BTimeSource::BTimeSource() :
+BTimeSource::BTimeSource()
+    :
     BMediaNode("This one is never called"),
     fStarted(false),
     fArea(-1),
@@ -239 +308 @@
 {
     CALLED();
     AddNodeKind(B_TIME_SOURCE);
-//  printf("##### BTimeSource::BTimeSource() name %s, id %ld\n", Name(), ID());
-
     // This constructor is only called by real time sources that inherit
     // BTimeSource. We create the communication area in FinishCreate(),
     // since we don't have a correct ID() until this node is registered.
@@ -248 +315 @@
 
 
 status_t
-BTimeSource::HandleMessage(int32 message,
-                           const void *rawdata,
-                           size_t size)
+BTimeSource::HandleMessage(int32 message, const void* rawdata,
+    size_t size)
 {
     PRINT(4, "BTimeSource::HandleMessage %#" B_PRIx32 ", node %" B_PRId32 "\n",
         message, fNodeID);
@@ -258 +324 @@
     switch (message) {
         case TIMESOURCE_OP:
         {
-            const time_source_op_info *data = static_cast<const time_source_op_info *>(rawdata);
+            const time_source_op_info* data
+                = static_cast<const time_source_op_info*>(rawdata);
 
             status_t result;
             result = TimeSourceOp(*data, NULL);
@@ -285 +352 @@
 
         case TIMESOURCE_ADD_SLAVE_NODE:
         {
-            const timesource_add_slave_node_command *data = static_cast<const timesource_add_slave_node_command *>(rawdata);
+            const timesource_add_slave_node_command* data
+                = static_cast<const timesource_add_slave_node_command*>(rawdata);
             DirectAddMe(data->node);
             return B_OK;
         }
 
         case TIMESOURCE_REMOVE_SLAVE_NODE:
         {
-            const timesource_remove_slave_node_command *data = static_cast<const timesource_remove_slave_node_command *>(rawdata);
+            const timesource_remove_slave_node_command* data
+                = static_cast<const timesource_remove_slave_node_command*>(rawdata);
             DirectRemoveMe(data->node);
             return B_OK;
         }
 
         case TIMESOURCE_GET_START_LATENCY:
         {
-            const timesource_get_start_latency_request *request = static_cast<const timesource_get_start_latency_request *>(rawdata);
+            const timesource_get_start_latency_request* request
+                = static_cast<const timesource_get_start_latency_request*>(rawdata);
             timesource_get_start_latency_reply reply;
             rv = GetStartLatency(&reply.start_latency);
             request->SendReply(rv, &reply, sizeof(reply));
@@ -312 +382 @@
 
 void
 BTimeSource::PublishTime(bigtime_t performance_time,
-                         bigtime_t real_time,
-                         float drift)
+    bigtime_t real_time, float drift)
 {
     TRACE_TIMESOURCE("BTimeSource::PublishTime timesource %" B_PRId32
         ", perf %16" B_PRId64 ", real %16" B_PRId64 ", drift %2.2f\n", ID(),
@@ -332 +401 @@
     fBuf->perftime[index] = performance_time;
     fBuf->drift[index] = drift;
     atomic_add(&fBuf->readindex, 1);
-
-//  printf("BTimeSource::PublishTime timesource %ld, write index %ld, perf %16Ld, real %16Ld, drift %2.2f\n", ID(), index, performance_time, real_time, drift);
 }
 
 
 void
 BTimeSource::BroadcastTimeWarp(bigtime_t at_real_time,
-                               bigtime_t new_performance_time)
+    bigtime_t new_performance_time)
 {
     CALLED();
     ASSERT(fSlaveNodes != NULL);
@@ -350 +417 @@
         ", new_performance_time %" B_PRId64 "\n", at_real_time,
         new_performance_time);
 
-    BAutolock lock(fSlaveNodes->locker);
+    BAutolock lock(fSlaveNodes);
 
-    for (int i = 0, n = 0; i < SLAVE_NODES_COUNT && n != fSlaveNodes->count; i++) {
-        if (fSlaveNodes->node_id[i] != 0) {
-            node_time_warp_command cmd;
-            cmd.at_real_time = at_real_time;
-            cmd.to_performance_time = new_performance_time;
-            SendToPort(fSlaveNodes->node_port[i], NODE_TIME_WARP, &cmd, sizeof(cmd));
-            n++;
-        }
+    for (int i = 0; i < fSlaveNodes->CountSlaves(); i++) {
+        node_time_warp_command cmd;
+        cmd.at_real_time = at_real_time;
+        cmd.to_performance_time = new_performance_time;
+        SendToPort(fSlaveNodes->PortAt(i), NODE_TIME_WARP,
+            &cmd, sizeof(cmd));
     }
 }
 
@@ -372 +437 @@
 
     // send the run mode change to all slaved nodes
 
-    BAutolock lock(fSlaveNodes->locker);
+    BAutolock lock(fSlaveNodes);
 
-    for (int i = 0, n = 0; i < SLAVE_NODES_COUNT && n != fSlaveNodes->count; i++) {
-        if (fSlaveNodes->node_id[i] != 0) {
-            node_set_run_mode_command cmd;
-            cmd.mode = mode;
-            SendToPort(fSlaveNodes->node_port[i], NODE_SET_RUN_MODE, &cmd, sizeof(cmd));
-            n++;
-        }
+    for (int i = 0; i < fSlaveNodes->CountSlaves(); i++) {
+        node_set_run_mode_command cmd;
+        cmd.mode = mode;
+        SendToPort(fSlaveNodes->PortAt(i), NODE_SET_RUN_MODE,
+            &cmd, sizeof(cmd));
     }
 }
 
@@ -410 +473 @@
 status_t BTimeSource::_Reserved_TimeSource_5(void *) { return B_ERROR; }
 
 /* explicit */
-BTimeSource::BTimeSource(media_node_id id) :
+BTimeSource::BTimeSource(media_node_id id)
+    :
     BMediaNode("This one is never called"),
     fStarted(false),
     fArea(-1),
@@ -421 +485 @@
     CALLED();
     AddNodeKind(B_TIME_SOURCE);
     ASSERT(id > 0);
-//  printf("###### explicit BTimeSource::BTimeSource() id %ld, name %s\n", id, Name());
 
-    // This constructor is only called by the derived BPrivate::media::TimeSourceObject objects
-    // We create a clone of the communication area
+    // This constructor is only called by the derived
+    // BPrivate::media::TimeSourceObject objects
+    // We create a clone of the communication area.
     char name[32];
     area_id area;
     sprintf(name, "__timesource_buf_%" B_PRId32, id);
@@ -435 +499 @@
         return;
     }
     sprintf(name, "__cloned_timesource_buf_%" B_PRId32, id);
-    fArea = clone_area(name, reinterpret_cast<void **>(const_cast<BPrivate::media::TimeSourceTransmit **>(&fBuf)), B_ANY_ADDRESS, B_READ_AREA | B_WRITE_AREA, area);
+
+    void** buf = reinterpret_cast<void**>
+        (const_cast<BPrivate::media::TimeSourceTransmit**>(&fBuf));
+
+    fArea = clone_area(name, buf, B_ANY_ADDRESS,
+        B_READ_AREA | B_WRITE_AREA, area);
+
     if (fArea <= 0) {
         ERROR("BTimeSource::BTimeSource couldn't clone area, node %" B_PRId32
             "\n", id);
@@ -448 +518 @@
 BTimeSource::FinishCreate()
 {
     CALLED();
-    //printf("BTimeSource::FinishCreate(), id %ld\n", ID());
 
     char name[32];
     sprintf(name, "__timesource_buf_%" B_PRId32, ID());
-    fArea = create_area(name, reinterpret_cast<void **>(const_cast<BPrivate::media::TimeSourceTransmit **>(&fBuf)), B_ANY_ADDRESS, TS_AREA_SIZE, B_FULL_LOCK, B_READ_AREA | B_WRITE_AREA);
+
+    void** buf = reinterpret_cast<void**>
+        (const_cast<BPrivate::media::TimeSourceTransmit**>(&fBuf));
+
+    fArea = create_area(name, buf, B_ANY_ADDRESS, TS_AREA_SIZE,
+        B_FULL_LOCK, B_READ_AREA | B_WRITE_AREA);
+
     if (fArea <= 0) {
         ERROR("BTimeSource::BTimeSource couldn't create area, node %" B_PRId32
             "\n", ID());
@@ -469 +544 @@
 
 
 status_t
-BTimeSource::RemoveMe(BMediaNode *node)
+BTimeSource::RemoveMe(BMediaNode* node)
 {
     CALLED();
     if (fKinds & NODE_KIND_SHADOW_TIMESOURCE) {
         timesource_remove_slave_node_command cmd;
         cmd.node = node->Node();
-        SendToPort(fControlPort, TIMESOURCE_REMOVE_SLAVE_NODE, &cmd, sizeof(cmd));
+        SendToPort(fControlPort, TIMESOURCE_REMOVE_SLAVE_NODE,
+            &cmd, sizeof(cmd));
     } else {
         DirectRemoveMe(node->Node());
     }
@@ -484 +560 @@
 
 
 status_t
-BTimeSource::AddMe(BMediaNode *node)
+BTimeSource::AddMe(BMediaNode* node)
 {
     CALLED();
     if (fKinds & NODE_KIND_SHADOW_TIMESOURCE) {
@@ -499 +575 @@
 
 
 void
-BTimeSource::DirectAddMe(const media_node &node)
+BTimeSource::DirectAddMe(const media_node& node)
 {
     // XXX this code has race conditions and is pretty dumb, and it
     // XXX won't detect nodes that crash and don't remove themself.
 
     CALLED();
     ASSERT(fSlaveNodes != NULL);
-    BAutolock lock(fSlaveNodes->locker);
+    BAutolock lock(fSlaveNodes);
 
-    if (fSlaveNodes->count == SLAVE_NODES_COUNT) {
+    if (fSlaveNodes->CountSlaves() == MAX_SLAVE_NODES) {
         ERROR("BTimeSource::DirectAddMe out of slave node slots\n");
         return;
     }
@@ -516 +592 @@
         ERROR("BTimeSource::DirectAddMe should not add itself to slave nodes\n");
         return;
     }
-    for (int i = 0; i < SLAVE_NODES_COUNT; i++) {
-        if (fSlaveNodes->node_id[i] == 0) {
-            fSlaveNodes->node_id[i] = node.node;
-            fSlaveNodes->node_port[i] = node.port;
-            fSlaveNodes->count += 1;
-            if (fSlaveNodes->count == 1) {
-                // start the time source
-                time_source_op_info msg;
-                msg.op = B_TIMESOURCE_START;
-                msg.real_time = RealTime();
-                TRACE_TIMESOURCE("starting time source %" B_PRId32 "\n", ID());
-                write_port(fControlPort, TIMESOURCE_OP, &msg, sizeof(msg));
-            }
-            return;
-        }
+
+    if (fSlaveNodes->InsertSlave(node) != true) {
+        ERROR("BTimeSource::DirectAddMe failed\n");
+        return;
+    }
+
+    if (fSlaveNodes->CountSlaves() == 1) {
+        // start the time source
+        time_source_op_info msg;
+        msg.op = B_TIMESOURCE_START;
+        msg.real_time = RealTime();
+
+        TRACE_TIMESOURCE("starting time source %" B_PRId32 "\n", ID());
+
+        write_port(fControlPort, TIMESOURCE_OP, &msg, sizeof(msg));
     }
-    ERROR("BTimeSource::DirectAddMe failed\n");
 }
 
 void
-BTimeSource::DirectRemoveMe(const media_node &node)
+BTimeSource::DirectRemoveMe(const media_node& node)
 {
     // XXX this code has race conditions and is pretty dumb, and it
     // XXX won't detect nodes that crash and don't remove themself.
 
     CALLED();
     ASSERT(fSlaveNodes != NULL);
-    BAutolock lock(fSlaveNodes->locker);
+    BAutolock lock(fSlaveNodes);
 
-    if (fSlaveNodes->count == 0) {
+    if (fSlaveNodes->CountSlaves() == 0) {
         ERROR("BTimeSource::DirectRemoveMe no slots used\n");
         return;
     }
-    for (int i = 0; i < SLAVE_NODES_COUNT; i++) {
-        if (fSlaveNodes->node_id[i] == node.node && fSlaveNodes->node_port[i] == node.port) {
-            fSlaveNodes->node_id[i] = 0;
-            fSlaveNodes->node_port[i] = 0;
-            fSlaveNodes->count -= 1;
-            if (fSlaveNodes->count == 0) {
-                // stop the time source
-                time_source_op_info msg;
-                msg.op = B_TIMESOURCE_STOP_IMMEDIATELY;
-                msg.real_time = RealTime();
-                TRACE_TIMESOURCE("stopping time source %" B_PRId32 "\n", ID());
-                write_port(fControlPort, TIMESOURCE_OP, &msg, sizeof(msg));
-            }
-            return;
-        }
+
+    if (fSlaveNodes->RemoveSlave(node) != true) {
+        ERROR("BTimeSource::DirectRemoveMe failed\n");
+        return;
+    }
+
+    if (fSlaveNodes->CountSlaves() == 0) {
+        // stop the time source
+        time_source_op_info msg;
+        msg.op = B_TIMESOURCE_STOP_IMMEDIATELY;
+        msg.real_time = RealTime();
+
+        TRACE_TIMESOURCE("stopping time source %" B_PRId32 "\n", ID());
+
+        write_port(fControlPort, TIMESOURCE_OP, &msg, sizeof(msg));
     }
-    ERROR("BTimeSource::DirectRemoveMe failed\n");
 }
 
 void
@@ -580 +654 @@
 
 
 void
-BTimeSource::DirectStop(bigtime_t at,
-                        bool immediate)
+BTimeSource::DirectStop(bigtime_t at, bool immediate)
 {
     CALLED();
     if (fBuf)
@@ -592 +665 @@
 
 
 void
-BTimeSource::DirectSeek(bigtime_t to,
-                        bigtime_t at)
+BTimeSource::DirectSeek(bigtime_t to, bigtime_t at)
 {
     UNIMPLEMENTED();
 }