Added some comment fixes

apps/srt-file-transmit.cpp

@@ -1,3 +1,27 @@
+/*****************************************************************************
+ * SRT - Secure, Reliable, Transport
+ * Copyright (c) 2017 Haivision Systems Inc.
+ * 
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ * 
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see <http://www.gnu.org/licenses/>
+ * 
+ *****************************************************************************/
+
+/*****************************************************************************
+written by
+   Haivision Systems Inc.
+ *****************************************************************************/
+
 
 #include <iostream>
 #include <iterator>

apps/srt-live-transmit.cpp

@@ -46,7 +46,8 @@
 //   option is SRTO_RCVSYN, which makes connect/accept call asynchronous.
 //   Because of that this option is treated special way in this app.
 //
-// See 'srt_options' global variable for a list of all options.
+// See 'srt_options' global variable (common/socketoptions.hpp) for a list of
+// all options.
 
 // MSVS likes to complain about lots of standard C functions being unsafe.
 #ifdef _MSC_VER
@@ -78,7 +79,7 @@
 
 // NOTE: This is without "haisrt/" because it uses an internal path
 // to the library. Application using the "installed" library should
-// use <haisrt/srt.h>
+// use <srt/srt.h>
 #include <srt.h>
 #include <logging.h>
 

apps/srt-multiplex.cpp

@@ -37,7 +37,7 @@ using namespace std;
 const size_t DEFAULT_CHUNK = 1316;
 
 const logging::LogFA SRT_LOGFA_APP = 10;
-logging::Logger applog(SRT_LOGFA_APP, srt_logger_config, "siplex");
+logging::Logger applog(SRT_LOGFA_APP, srt_logger_config, "srt-mplex");
 
 volatile bool siplex_int_state = false;
 void OnINT_SetIntState(int)
@@ -416,17 +416,17 @@ void Help(string program)
 "interperted by the application and used to set resource id on an SRT socket when connecting\n"
 "or to match with the id extracted from the accepted socket of incoming connection.\n"
 "Example:\n"
-"\tSender:    siplex srt://remhost:2000 -i udp://:5000?id=low udp://:6000?id=high\n"
-"\tReceiver:  siplex srt://:2000 -o output-high.ts?id=high output-low.ts?id=low\n"
+"\tSender:    srt-multiplex srt://remhost:2000 -i udp://:5000?id=low udp://:6000?id=high\n"
+"\tReceiver:  srt-multiplex srt://:2000 -o output-high.ts?id=high output-low.ts?id=low\n"
 "\nHere you create a Sender which will connect to 'remhost' port 2000 using multiple SRT\n"
 "sockets, all of which will be using the same outgoing port. Here the port is autoselected\n"
 "by the first socket when connecting, every next one will reuse that port. Alternatively you\n"
 "can enforce the outgoing port using 'port' parameter in the SRT URI.\n\n"
 "Then for every input resource a separate connection is made and appropriate resource id\n"
 "will be set to particular socket assigned to that resource according to the 'id' parameter.\n"
 "When the listener side (here Receiver) gets the socket accepted, it will have the resource\n"
-"id set just as the caller side did, in which case siplex will search for this id among the\n"
-"registered resources and match the resource (output here) with this id. If the resource is\n"
+"id set just as the caller side did, in which case srt-multiplex will search for this id among\n"
+"the registered resources and match the resource (output here) with this id. If the resource is\n"
 "not found, the connection is closed immediately. This works the same way regardless of which\n"
 "direction is used by caller or listener\n";
 
@@ -455,9 +455,6 @@ int main( int argc, char** argv )
         }
     } cleanupobj;
 
-    //vector<string> args;
-    //copy(argv+1, argv+argc, back_inserter(args));
-
     // Check options
     vector<OptionScheme> optargs = {
         { {"ll", "loglevel"}, OptionScheme::ARG_ONE },
@@ -468,7 +465,7 @@ int main( int argc, char** argv )
 
     // The call syntax is:
     //
-    //      siplex <SRT URI> -o/-i ARGS...
+    //      srt-multiplex <SRT URI> -o/-i ARGS...
     //
     // SRT URI should contain:
     // srt://[host]:port?mode=MODE&adapter=ADAPTER&port=PORT&otherparameters...

apps/suflip.cpp

@@ -54,7 +54,7 @@ struct InitializeMe
         copy(false_names_i, false_names_i+4, inserter(false_names, false_names.begin()));
     }
 
-} g_moron;
+} g_initialize_names;
 
 bool verbose = false;
 volatile bool throw_on_interrupt = false;

common/appcommon.hpp

@@ -93,7 +93,7 @@ static inline int inet_pton(int af, const char * src, void * dst)
 
    ZeroMemory(&ss, sizeof(ss));
 
-   // work around stupid non-const API
+   // work around non-const API
    strncpy(srcCopy, src, INET6_ADDRSTRLEN + 1);
    srcCopy[INET6_ADDRSTRLEN] = '\0';
 

common/netinet_any.h

@@ -28,8 +28,11 @@ written by
 #include <cstring>
 #include "platform_sys.h"
 
-// This is a smart structure that this moron who has designed BSD sockets
-// should have defined in the first place.
+// This structure should replace every use of sockaddr and its currently
+// used specializations, sockaddr_in and sockaddr_in6. This is to simplify
+// the use of the original BSD API that relies on type-violating type casts.
+// You can use the instances of sockaddr_any in every place where sockaddr is
+// required.
 
 struct sockaddr_any
 {
@@ -122,7 +125,6 @@ struct sockaddr_any
             return memcmp(&c1, &c2, sizeof(c1)) < 0;
         }
     };
-
 };
 
 template<> struct sockaddr_any::TypeMap<AF_INET> { typedef sockaddr_in type; };

common/transmitmedia.cpp

@@ -737,9 +737,9 @@ int SrtTarget::ConfigurePre(SRTSOCKET sock)
         return result;
 
     int yes = 1;
-    // This is for the HSv4 compatibility, if both parties are HSv5
+    // This is for the HSv4 compatibility; if both parties are HSv5
     // (min. version 1.2.1), then this setting simply does nothing.
-    // In HSv4 this setting is obligatory otherwise the SRT handshake
+    // In HSv4 this setting is obligatory; otherwise the SRT handshake
     // extension will not be done at all.
     result = srt_setsockopt(sock, 0, SRTO_SENDER, &yes, sizeof yes);
     if ( result == -1 )
@@ -1013,8 +1013,8 @@ class UdpCommon
             attr.erase("adapter");
         }
 
-        // The "ttl" options is handled separately, it maps to either IP_TTL
-        // or IP_MULTICAST_TTL, depending on whether the address is sc or mc.
+        // The "ttl" options is handled separately, it maps to both IP_TTL
+        // and IP_MULTICAST_TTL so that TTL setting works for both uni- and multicast.
         if (attr.count("ttl"))
         {
             int ttl = stoi(attr.at("ttl"));

common/utilities.h

@@ -82,9 +82,24 @@ written by
 // -------------- UTILITIES ------------------------
 
 // Bit numbering utility.
-// Usage: Bits<leftmost, rightmost>
 //
-// You can use it as a typedef (say, "MASKTYPE") and then use the following members:
+// This is something that allows you to turn 32-bit integers into bit fields.
+// Although bitfields are part of C++ language, they are not designed to be
+// interchanged with 32-bit numbers, and any attempt to doing it (by placing
+// inside a union, for example) is nonportable (order of bitfields inside
+// same-covering 32-bit integer number is dependent on the endian), so they are
+// popularly disregarded as useless. Instead the 32-bit numbers with bits
+// individually selected is preferred, with usually manual playing around with
+// & and | operators, as well as << and >>. This tool is designed to simplify
+// the use of them. This can be used to qualify a range of bits inside a 32-bit
+// number to be a separate number, you can "wrap" it by placing the integer
+// value in the range of these bits, as well as "unwrap" (extract) it from
+// the given place. For your own safety, use one prefix to all constants that
+// concern bit ranges intended to be inside the same "bit container".
+//
+// Usage: typedef Bits<leftmost, rightmost> MASKTYPE;  // MASKTYPE is a name of your choice.
+//
+// With this defined, you can use the following members:
 // - MASKTYPE::mask - to get the int32_t value with bimask (used bits set to 1, others to 0)
 // - MASKTYPE::offset - to get the lowermost bit number, or number of bits to shift
 // - MASKTYPE::wrap(int value) - to create a bitset where given value is encoded in given bits
@@ -495,7 +510,7 @@ class DriftTracer
     //
     // IMPORTANT: drift() can be called at any time, just remember
     // that this value may look different than before only if the
-    // last update() return true, which need not be important for you.
+    // last update() returned true, which need not be important for you.
     //
     // CASE: CLEAR_ON_UPDATE = true
     // overdrift() should be read only immediately after update() returned

srtcore/api.cpp

@@ -632,12 +632,12 @@ int CUDTUnited::bind(SRTSOCKET u, UDPSOCKET udpsock)
 
 int CUDTUnited::listen(const SRTSOCKET u, int backlog)
 {
-   if (backlog <= 0 )
+   if (backlog <= 0)
       throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
 
    // Don't search for the socket if it's already -1;
    // this never is a valid socket.
-   if ( u == UDT::INVALID_SOCK )
+   if (u == UDT::INVALID_SOCK)
       throw CUDTException(MJ_NOTSUP, MN_SIDINVAL, 0);
 
    CUDTSocket* s = locate(u);
@@ -675,8 +675,7 @@ int CUDTUnited::listen(const SRTSOCKET u, int backlog)
    catch (...)
    {
       delete s->m_pQueuedSockets;
-      delete s->m_pAcceptSockets;   // XXX If this was exception-interrupted,
-                                    // then nothing is allocated!
+      delete s->m_pAcceptSockets;
 
       // XXX Translated std::bad_alloc into CUDTException specifying
       // memory allocation failure...
@@ -725,11 +724,13 @@ SRTSOCKET CUDTUnited::accept(const SRTSOCKET listen, sockaddr* addr, int* addrle
        }
        else if (ls->m_pQueuedSockets->size() > 0)
        {
-           // XXX Actually this should at best be something like that:
+           // XXX REFACTORING REQUIRED HERE!
+           // Actually this should at best be something like that:
            // set<SRTSOCKET>::iterator b = ls->m_pQueuedSockets->begin();
            // u = *b;
            // ls->m_pQueuedSockets->erase(b);
-           // ls->m_pAcceptSockets.insert(u);
+           // ls->m_pAcceptSockets->insert(u);
+           //
            // It is also questionable why m_pQueuedSockets should be of type 'set'.
            // There's no quick-searching capabilities of that container used anywhere except
            // checkBrokenSockets and garbageCollect, which aren't performance-critical,
@@ -739,9 +740,10 @@ SRTSOCKET CUDTUnited::accept(const SRTSOCKET listen, sockaddr* addr, int* addrle
            // the first is taken here, which is actually the socket with lowest
            // possible descriptor value (as default operator< and ascending sorting
            // used for std::set<SRTSOCKET> where SRTSOCKET=int).
+           //
+           // Consider using std::list or std::vector here.
 
            u = *(ls->m_pQueuedSockets->begin());
-           // why suggest the position - it is std::set!
            ls->m_pAcceptSockets->insert(ls->m_pAcceptSockets->end(), u);
            ls->m_pQueuedSockets->erase(ls->m_pQueuedSockets->begin());
            accepted = true;
@@ -909,14 +911,14 @@ int CUDTUnited::close(const SRTSOCKET u)
       s->m_TimeStamp = CTimer::getTime();
       s->m_pUDT->m_bBroken = true;
 
-      // NOTE: (changed by Sektor)
+      // Change towards original UDT: 
       // Leave all the closing activities for garbageCollect to happen,
       // however remove the listener from the RcvQueue IMMEDIATELY.
-      // This is because the listener socket is useless anyway and should
-      // not be used for anything NEW since now.
-
-      // But there's no reason to destroy the world by occupying the
-      // listener slot in the RcvQueue.
+      // Even though garbageCollect would eventually remove the listener
+      // as well, there would be some time interval between now and the
+      // moment when it's done, and during this time the application will
+      // be unable to bind to this port that the about-to-delete listener
+      // is currently occupying (due to blocked slot in the RcvQueue).
 
       LOGC(mglog.Debug, log << s->m_pUDT->CONID() << " CLOSING (removing listener immediately)");
       {
@@ -948,7 +950,7 @@ int CUDTUnited::close(const SRTSOCKET u)
 
        s->m_Status = SRTS_CLOSED;
 
-       // a socket will not be immediated removed when it is closed
+       // a socket will not be immediately removed when it is closed
        // in order to prevent other methods from accessing invalid address
        // a timer is started and the socket will be removed after approximately
        // 1 second
@@ -1693,7 +1695,7 @@ void CUDTUnited::updateMux(
       throw e;
    }
 
-   // XXX Looks stupid. Simplify. Use sockaddr_any.
+   // XXX Simplify this. Use sockaddr_any.
    sockaddr* sa = (AF_INET == s->m_pUDT->m_iIPversion)
       ? (sockaddr*) new sockaddr_in
       : (sockaddr*) new sockaddr_in6;
@@ -1726,7 +1728,8 @@ void CUDTUnited::updateMux(
       "creating new multiplexer for port %i\n", m.m_iPort);
 }
 
-// XXX This is actually something completely stupid.
+// XXX This functionality needs strong refactoring.
+//
 // This function is going to find a multiplexer for the port contained
 // in the 'ls' listening socket, by searching through the multiplexer
 // container.
@@ -1738,34 +1741,37 @@ void CUDTUnited::updateMux(
 // didn't, this function wouldn't even have a chance to be called.
 //
 // Why can't then the multiplexer be recorded in the 'ls' listening socket data
-// to be accessed immediately, especially when one listener can't bind to more than
-// one multiplexer at a time (well, even if it could, there's still no reason why
-// this should be extracted by "querying")?
+// to be accessed immediately, especially when one listener can't bind to more
+// than one multiplexer at a time (well, even if it could, there's still no
+// reason why this should be extracted by "querying")?
 //
 // Maybe because the multiplexer container is a map, not a list.
 // Why is this then a map? Because it's addressed by MuxID. Why do we need
 // mux id? Because we don't have a list... ?
 // 
-// But what's the multiplexer ID? It's a socket ID for which it was originally created.
+// But what's the multiplexer ID? It's a socket ID for which it was originally
+// created.
 //
-// Is this then shared? Yes, only between the listener socket and the accepted sockets,
-// or in case of "bound" connecting sockets (by binding you can enforce the port number,
-// which can be the same for multiple SRT sockets).
+// Is this then shared? Yes, only between the listener socket and the accepted
+// sockets, or in case of "bound" connecting sockets (by binding you can
+// enforce the port number, which can be the same for multiple SRT sockets).
 // Not shared in case of unbound connecting socket or rendezvous socket.
 //
-// Ok, in which situation do we need dispatching by mux id? Only when the socket is being
-// deleted. How does the deleting procedure know the muxer id? Because it is recorded here
-// at the time when it's found, as... the socket ID of the actual listener socket being
-// actually the first socket to create the multiplexer, so the multiplexer gets its id.
+// Ok, in which situation do we need dispatching by mux id? Only when the
+// socket is being deleted. How does the deleting procedure know the muxer id?
+// Because it is recorded here at the time when it's found, as... the socket ID
+// of the actual listener socket being actually the first socket to create the
+// multiplexer, so the multiplexer gets its id.
 //
-// Still, no reasons found why the socket can't contain a list iterator to a multiplexer
-// INSTEAD of m_iMuxID. There's no danger in this solutio because the multiplexer is never
-// deleted until there's at least one socket using it.
+// Still, no reasons found why the socket can't contain a list iterator to a
+// multiplexer INSTEAD of m_iMuxID. There's no danger in this solution because
+// the multiplexer is never deleted until there's at least one socket using it.
 //
-// The multiplexer may even physically be contained in the CUDTUnited object, just track
-// the multiple users of it (the listener and the accepted sockets). When deleting, you
-// simply "unsubscribe" yourself from the multiplexer, which will unref it and remove the
-// list element by the iterator kept by the socket.
+// The multiplexer may even physically be contained in the CUDTUnited object,
+// just track the multiple users of it (the listener and the accepted sockets).
+// When deleting, you simply "unsubscribe" yourself from the multiplexer, which
+// will unref it and remove the list element by the iterator kept by the
+// socket.
 void CUDTUnited::updateListenerMux(CUDTSocket* s, const CUDTSocket* ls)
 {
    CGuard cg(m_ControlLock);
@@ -3041,8 +3047,6 @@ int getlasterror_errno()
 // Get error string of a given error code
 const char* geterror_desc(int code, int err)
 {
-   // static CUDTException e; //>>Need something better than static here.
-   // Yeah, of course. Here you are:
    CUDTException e (CodeMajor(code/1000), CodeMinor(code%1000), err);
    return(e.getErrorMessage());
 }

srtcore/buffer.cpp

@@ -1122,8 +1122,7 @@ bool CRcvBuffer::getRcvReadyMsg(ref_t<uint64_t> tsbpdtime, ref_t<int32_t> curpkt
 /*
 * Return receivable data status (packet timestamp ready to play if TsbPd mode)
 * Return playtime (tsbpdtime) of 1st packet in queue, ready to play or not
-*/
-/* 
+*
 * Return data ready to be received (packet timestamp ready to play if TsbPd mode)
 * Using getRcvDataSize() to know if there is something to read as it was widely
 * used in the code (core.cpp) is expensive in TsbPD mode, hence this simpler function
@@ -1403,7 +1402,7 @@ int CRcvBuffer::setRcvTsbPdMode(uint64_t timebase, uint32_t delay)
     //
     // This function is called in the HSREQ reception handler only.
     m_ullTsbPdTimeBase = timebase;
-    // XXX Note that this is completely wrong.
+    // XXX Seems like this may not work correctly.
     // At least this solution this way won't work with application-supplied
     // timestamps. For that case the timestamps should be taken exclusively
     // from the data packets because in case of application-supplied timestamps