Merge pull request #2 from Haivision/master

update

CMakeLists.txt

@@ -8,7 +8,7 @@
 #
 
 cmake_minimum_required (VERSION 2.8.12 FATAL_ERROR)
-set (SRT_VERSION 1.4.2)
+set (SRT_VERSION 1.4.3)
 
 set (CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/scripts")
 include(haiUtil) # needed for set_version_variables

docs/APISocketOptions.md

@@ -228,7 +228,7 @@ The following table lists SRT socket options in alphabetical order. Option detai
 | [`SRTO_MSS`](#SRTO_MSS)                                |       | pre      | `int32_t` | bytes   | 1500          | 76..     | RW  | GSD   |
 | [`SRTO_NAKREPORT`](#SRTO_NAKREPORT)                    | 1.1.0 | pre      | `bool`    |         |  *            |          | RW  | GSD+  |
 | [`SRTO_OHEADBW`](#SRTO_OHEADBW)                        | 1.0.5 | post     | `int32_t` | %       | 25            | 5..100   | RW  | GSD   |
-| [`SRTO_PACKETFILTER`](#SRTO_PACKETFILTER)              | 1.4.0 | pre      | `string`  |         | ""            | [512]    | W   | GSD   |
+| [`SRTO_PACKETFILTER`](#SRTO_PACKETFILTER)              | 1.4.0 | pre      | `string`  |         | ""            | [512]    | RW  | GSD   |
 | [`SRTO_PASSPHRASE`](#SRTO_PASSPHRASE)                  | 0.0.0 | pre      | `string`  |         | ""            | [10..79] | W   | GSD   |
 | [`SRTO_PAYLOADSIZE`](#SRTO_PAYLOADSIZE)                | 1.3.0 | pre      | `int32_t` | bytes   | \*            | \*       | W   | GSD   |
 | [`SRTO_PBKEYLEN`](#SRTO_PBKEYLEN)                      | 0.0.0 | pre      | `int32_t` | bytes   | 0             | *        | RW  | GSD   |
@@ -871,17 +871,61 @@ and break quickly at any rise in packet loss.
 
 | OptName              | Since | Restrict | Type       |  Units  | Default  | Range  | Dir | Entity |
 | -------------------- | ----- | -------- | ---------- | ------- | -------- | ------ | --- | ------ |
-| `SRTO_PACKETFILTER`  | 1.4.0 | pre      | `string`   |         |  ""      | [512]  | W   | GSD    |
+| `SRTO_PACKETFILTER`  | 1.4.0 | pre      | `string`   |         |  ""      | [512]  | RW  | GSD    |
 
 Set up the packet filter. The string must match appropriate syntax for packet
-filter setup.
-
-As there can only be one configuration for both parties, it is recommended that
-one party defines the full configuration while the other only defines the matching
-packet filter type (for example, one sets `fec,cols:10,rows:-5,layout:staircase`
-and the other just `fec`). Both parties can also set this option to the same value.
-The packet filter function will attempt to merge configuration definitions, but if
-the options specified are in conflict, the connection will be rejected.
+filter setup. Note also that:
+
+* The configuration is case-sentitive (e.g. "FEC,Cols:20" is not valid).
+* Setting this option will fail if you use an unknown filter type.
+
+An empty value for this option means that for this connection the filter isn't
+required, but it will accept any filter settings if provided by the peer. If
+this option is changed by both parties simultaneously, the result will be a
+configuration integrating parameters from both parties, that is:
+
+* parameters provided by both parties are accepted, if they are identical
+* parameters that are set only on one side will have the value defined by that side
+* parameters not set in either side will be set as default
+
+The connection will be rejected with `SRT_REJ_FILTER` code in the following cases:
+
+* both sides define a different packet filter type
+* for the same key two different values were provided by both sides
+* mandatory parameters weren't provided by either side
+
+In case of the built-in `fec` filter, the mandatory parameter is `cols`, all
+others have their default values. For example, the configuration specified
+as `fec,cols:10` is `fec,cols:10,rows:1,arq:onreq,layout:even`. See how to
+[configure the FEC Filter](packet-filtering-and-fec.md#configuring-the-fec-filter).
+
+Below in the table are examples for the built-in `fec` filter. Note that the
+negotiated config need not have parameters in the given order.
+
+Cases when negotiation succeeds:
+
+| Peer A               | Peer B              | Negotiated Config            
+|----------------------|---------------------|------------------------------------------------------
+| (no filter)          | (no filter)         | 
+| fec,cols:10          | fec                 | fec,cols:10,rows:1,arq:onreq,layout:even                  
+| fec,cols:10          | fec,cols:10,rows:20 | fec,cols:10,rows:20,arq:onreq,layout:even                  
+| fec,layout:staircase | fec,cols:10         | fec,cols:10,rows:1,arq:onreq,layout:staircase 
+
+In these cases the configuration is rejected with SRT_REJ_FILTER code:
+
+| Peer A                | Peer B              | Error reason
+|-----------------------|---------------------|--------------------------
+| fec                   | (no filter)         | missing `cols` parameter 
+| fec,rows:20,arq:never | fec,layout:even     | missing `cols` parameter 
+| fec,cols:20           | fec,cols:10         | `cols` parameter value conflict 
+| fec,cols:20,rows:20   | fec,cols:20,rows:10 | `rows` parameter value conflict 
+
+In general it is recommended that one party defines the full configuration,
+while the other keeps this value empty.
+
+Reading this option after the connection is established will return the full
+configuration that has been agreed upon by both parties (including default
+values).
 
 For details, see [Packet Filtering & FEC](packet-filtering-and-fec.md).
 

docs/packet-filtering-and-fec.md

@@ -42,21 +42,22 @@ general syntax:
 ```
 The parts of this syntax are separated by commas. The first part is the name of 
 the filter. This is followed by one or more key:value pairs, the interpretation 
-of which depends on the filter type.
+of which depends on the filter type. Note that all keys and values are
+case-sensitive.
 
 You can try this out using the `SRTO_PACKETFILTER` option, or the
 `packetfilter` parameter in an SRT URI in the applications.
 
 The packet filter framework is open for extensions so that users may register
-their own filters. SRT provides also one builtin filter named "fec". This
+their own filters. SRT provides also one built-in filter named "fec". This
 filter implements the FEC mechanism, as described in SMPTE 2022-1-2007.
 
 ![SRT packet filter mechanism](/docs/images/packet-filter-mechanism.png)
 
 On the input side, filtering occurs at the moment when a packet is extracted 
 from the send buffer. A filter may then do two things:
 
-* alter the packet before inserting it into the SRT channel (the builtin "fec"
+* alter the packet before inserting it into the SRT channel (the built-in "fec"
   filter doesn't do it, though)
 
 * insert another packet (such as an FEC control packet) into the channel ahead

srtcore/common.cpp

@@ -462,7 +462,7 @@ extern const char* const srt_rejectreason_msg [] = {
     "Password required or unexpected",
     "MessageAPI/StreamAPI collision",
     "Congestion controller type collision",
-    "Packet Filter type collision",
+    "Packet Filter settings error",
     "Group settings collision",
     "Connection timeout"
 };
@@ -495,7 +495,8 @@ bool SrtParseConfig(string s, SrtConfig& w_config)
         Split(*i, ':', back_inserter(keyval));
         if (keyval.size() != 2)
             return false;
-        w_config.parameters[keyval[0]] = keyval[1];
+        if (keyval[1] != "")
+            w_config.parameters[keyval[0]] = keyval[1];
     }
 
     return true;

srtcore/core.cpp

@@ -2648,6 +2648,7 @@ bool CUDT::interpretSrtHandshake(const CHandShake& hs,
 
                 if (!checkApplyFilterConfig(fltcfg))
                 {
+                    m_RejectReason = SRT_REJ_FILTER;
                     LOGC(cnlog.Error, log << "PEER'S FILTER CONFIG [" << fltcfg << "] has been rejected");
                     return false;
                 }
@@ -2755,7 +2756,7 @@ bool CUDT::interpretSrtHandshake(const CHandShake& hs,
 bool CUDT::checkApplyFilterConfig(const std::string &confstr)
 {
     SrtFilterConfig cfg;
-    if (!ParseFilterConfig(confstr, cfg))
+    if (!ParseFilterConfig(confstr, (cfg)))
         return false;
 
     // Now extract the type, if present, and
@@ -2775,7 +2776,7 @@ bool CUDT::checkApplyFilterConfig(const std::string &confstr)
         }
 
         SrtFilterConfig mycfg;
-        if (!ParseFilterConfig(thisconf, mycfg))
+        if (!ParseFilterConfig(thisconf, (mycfg)))
             return false;
 
         // Check only if both have set a filter of the same type.
@@ -2795,12 +2796,8 @@ bool CUDT::checkApplyFilterConfig(const std::string &confstr)
         }
         else
         {
-            // On a listener, only apply those that you haven't set
-            for (map<string, string>::iterator x = cfg.parameters.begin(); x != cfg.parameters.end(); ++x)
-            {
-                if (!mycfg.parameters.count(x->first))
-                    mycfg.parameters[x->first] = x->second;
-            }
+            if (!CheckFilterCompat((mycfg), cfg))
+                return false;
         }
 
         HLOGC(cnlog.Debug,
@@ -5495,11 +5492,21 @@ SRT_REJECT_REASON CUDT::setupCC()
         // At this point we state everything is checked and the appropriate
         // corrector type is already selected, so now create it.
         HLOGC(pflog.Debug, log << "filter: Configuring: " << m_config.sPacketFilterConfig.c_str());
-        if (!m_PacketFilter.configure(this, &(m_pRcvQueue->m_UnitQueue), m_config.sPacketFilterConfig.str()))
+        bool status = true;
+        try
         {
-            return SRT_REJ_FILTER;
+            // The filter configurer is build the way that allows to quit immediately
+            // exit by exception, but the exception is meant for the filter only.
+            status = m_PacketFilter.configure(this, &(m_pRcvQueue->m_UnitQueue), m_config.sPacketFilterConfig.str());
+        }
+        catch (CUDTException& )
+        {
+            status = false;
         }
 
+        if (!status)
+            return SRT_REJ_FILTER;
+
         m_PktFilterRexmitLevel = m_PacketFilter.arqLevel();
     }
     else

srtcore/core.h

@@ -169,6 +169,7 @@ class CUDT
     friend class PacketFilter;
     friend class CUDTGroup;
     friend struct FByOldestActive; // this functional will use private fields
+    friend class TestMockCUDT;
 
     typedef srt::sync::steady_clock::time_point time_point;
     typedef srt::sync::steady_clock::duration duration;
@@ -413,7 +414,7 @@ class CUDT
     void skipIncoming(int32_t seq);
 
     // For SRT_tsbpdLoop
-    CUDTUnited* uglobal() { return &s_UDTUnited; } // needed by tsbpdLoop
+    static CUDTUnited* uglobal() { return &s_UDTUnited; } // needed by tsbpdLoop
     std::set<int>& pollset() { return m_sPollID; }
 
     CSrtConfig m_config;

srtcore/epoll.cpp

@@ -808,6 +808,12 @@ int CEPoll::swait(CEPollDesc& d, map<SRTSOCKET, int>& st, int64_t msTimeOut, boo
     return 0;
 }
 
+bool CEPoll::empty(CEPollDesc& d)
+{
+    ScopedLock lg (m_EPollLock);
+    return d.watch_empty();
+}
+
 int CEPoll::release(const int eid)
 {
    ScopedLock pg(m_EPollLock);

srtcore/epoll.h

@@ -60,7 +60,7 @@ modified by
 #include "udt.h"
 
 
-struct CEPollDesc
+class CEPollDesc
 {
    const int m_iID;                                // epoll ID
 
@@ -143,8 +143,6 @@ struct CEPollDesc
 std::string DisplayEpollWatch();
 #endif
 
-private:
-
    /// Sockets that are subscribed for events in this eid.
    ewatch_t m_USockWatchState;
 
@@ -159,7 +157,10 @@ std::string DisplayEpollWatch();
 
    enotice_t::iterator nullNotice() { return m_USockEventNotice.end(); }
 
-public:
+   // Only CEPoll class should have access to it.
+   // Guarding private access to the class is not necessary
+   // within the epoll module.
+   friend class CEPoll;
 
    CEPollDesc(int id, int localID)
        : m_iID(id)
@@ -423,6 +424,9 @@ friend class CRendezvousQueue;
    /// @retval >=0 number of ready sockets (actually size of `st`)
    int swait(CEPollDesc& d, fmap_t& st, int64_t msTimeOut, bool report_by_exception = true);
 
+   /// Empty subscription check - for internal use only.
+   bool empty(CEPollDesc& d);
+
    /// Reports which events are ready on the given socket.
    /// @param mp socket event map retirned by `swait`
    /// @param sock which socket to ask