dvbpsi_ValidPSISection: TOT table has a CRC32 and syntax_indicator is '0'.

In the past the b_syntax_indicator field could be used to distinguish between
tables with a CRC32 and those without. However these days this assumption is no
longer valid and the check should be modified to include tables that have a CRC32
even when syntax_indcator is '0'.

src/dvbpsi.c

@@ -498,16 +498,22 @@ bool dvbpsi_packet_push(dvbpsi_t *p_dvbpsi, uint8_t* p_data)
             }
             else
             {
+                bool b_valid_crc32 = false;
+                bool has_crc32 = dvbpsi_has_CRC32(p_section);
+
                 /* PSI section is complete */
                 p_section->b_syntax_indicator = p_section->p_data[1] & 0x80;
                 p_section->b_private_indicator = p_section->p_data[1] & 0x40;
+
                 /* Update the end of the payload if CRC_32 is present */
-                if (p_section->b_syntax_indicator)
+                if (p_section->b_syntax_indicator || has_crc32)
                     p_section->p_payload_end -= 4;
 
-                if ((p_section->p_data[0] == 0x70) /* TDT (has no CRC 32) */ ||
-                    (p_section->p_data[0] == 0x71) /* RST (has no CRC 32) */ ||
-                    (p_section->p_data[0] != 0x72 && dvbpsi_ValidPSISection(p_section)))
+                /* Check CRC32 if present */
+                if (has_crc32)
+                    b_valid_crc32 = dvbpsi_ValidPSISection(p_section);
+
+                if (!has_crc32 || b_valid_crc32)
                 {
                     /* PSI section is valid */
                     p_section->i_table_id = p_section->p_data[0];
@@ -536,9 +542,9 @@ bool dvbpsi_packet_push(dvbpsi_t *p_dvbpsi, uint8_t* p_data)
                 }
                 else
                 {
-                    if (!dvbpsi_ValidPSISection(p_section))
+                    if (has_crc32 && !dvbpsi_ValidPSISection(p_section))
                         dvbpsi_error(p_dvbpsi, "misc PSI", "Bad CRC_32 table 0x%x !!!",
-                                             p_section->p_data[0]);
+                                               p_section->p_data[0]);
                     else
                         dvbpsi_error(p_dvbpsi, "misc PSI", "table 0x%x", p_section->p_data[0]);
 

src/psi.c

@@ -110,7 +110,8 @@ bool dvbpsi_CheckPSISection(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t *p_section,
         goto error;
     }
 
-    if (!p_section->b_syntax_indicator)
+    if (!p_section->b_syntax_indicator &&
+        (table_id != 0x73)) /* TOT has b_syntax_indicator set to '0' */
     {
         /* Invalid section_syntax_indicator */
         dvbpsi_error(p_dvbpsi, psz_table_name,
@@ -137,28 +138,42 @@ error:
  *****************************************************************************/
 bool dvbpsi_ValidPSISection(dvbpsi_psi_section_t* p_section)
 {
-    if (p_section->b_syntax_indicator)
-    {
-        /* Check the CRC_32 if b_syntax_indicator is false */
-        uint32_t i_crc = 0xffffffff;
-        uint8_t* p_byte = p_section->p_data;
-
-        while(p_byte < p_section->p_payload_end + 4)
-        {
-            i_crc = (i_crc << 8) ^ dvbpsi_crc32_table[(i_crc >> 24) ^ (*p_byte)];
-            p_byte++;
-        }
+    uint32_t i_crc = 0xffffffff;
+    uint8_t* p_byte = p_section->p_data;
 
-        if (i_crc == 0)
-            return true;
-        else
-            return false;
+    while(p_byte < p_section->p_payload_end + 4)
+    {
+        i_crc = (i_crc << 8) ^ dvbpsi_crc32_table[(i_crc >> 24) ^ (*p_byte)];
+        p_byte++;
     }
+
+    if (i_crc == 0)
+        return true;
     else
-    {
-        /* No check to do if b_syntax_indicator is false */
         return false;
+}
+
+/*****************************************************************************
+ * dvbpsi_CalculateCRC32
+ *****************************************************************************
+ * Calculate the CRC32 for this section
+ *****************************************************************************/
+void dvbpsi_CalculateCRC32(dvbpsi_psi_section_t *p_section)
+{
+    uint8_t* p_byte = p_section->p_data;
+    p_section->i_crc = 0xffffffff;
+
+    while (p_byte < p_section->p_payload_end)
+    {
+        p_section->i_crc =   (p_section->i_crc << 8)
+                           ^ dvbpsi_crc32_table[(p_section->i_crc >> 24) ^ (*p_byte)];
+        p_byte++;
     }
+
+    p_section->p_payload_end[0] = (p_section->i_crc >> 24) & 0xff;
+    p_section->p_payload_end[1] = (p_section->i_crc >> 16) & 0xff;
+    p_section->p_payload_end[2] = (p_section->i_crc >> 8) & 0xff;
+    p_section->p_payload_end[3] = p_section->i_crc & 0xff;
 }
 
 /*****************************************************************************
@@ -168,8 +183,6 @@ bool dvbpsi_ValidPSISection(dvbpsi_psi_section_t* p_section)
  *****************************************************************************/
 void dvbpsi_BuildPSISection(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t* p_section)
 {
-    uint8_t* p_byte = p_section->p_data;
-
     /* table_id */
     p_section->p_data[0] = p_section->i_table_id;
     /* setion_syntax_indicator | private_indicator |
@@ -197,20 +210,11 @@ void dvbpsi_BuildPSISection(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t* p_section)
         /* last_section_number */
         p_section->p_data[7] = p_section->i_last_number;
 
-        /* CRC_32 */
-        p_section->i_crc = 0xffffffff;
-
-        while (p_byte < p_section->p_payload_end)
-        {
-            p_section->i_crc =   (p_section->i_crc << 8)
-                               ^ dvbpsi_crc32_table[(p_section->i_crc >> 24) ^ (*p_byte)];
-            p_byte++;
-        }
+    }
 
-        p_section->p_payload_end[0] = (p_section->i_crc >> 24) & 0xff;
-        p_section->p_payload_end[1] = (p_section->i_crc >> 16) & 0xff;
-        p_section->p_payload_end[2] = (p_section->i_crc >> 8) & 0xff;
-        p_section->p_payload_end[3] = p_section->i_crc & 0xff;
+    if (dvbpsi_has_CRC32(p_section))
+    {
+        dvbpsi_CalculateCRC32(p_section);
 
         if (!dvbpsi_ValidPSISection(p_section))
         {

src/psi.h

@@ -124,8 +124,8 @@ void dvbpsi_DeletePSISections(dvbpsi_psi_section_t * p_section);
 /*!
  * \fn bool dvbpsi_CheckPSISection(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t *p_section,
                             const uint8_t table_id, const char *psz_table_name)
- * \brief Check if PSI section has the expected table_id and it the syntax indicator
- * is true.
+ * \brief Check if PSI section has the expected table_id. Call this function only for
+ * PSI sections that have a CRC32 (@see dvbpsi_has_CRC32() function)
  * \param p_dvbpsi pointer to dvbpsi library handle
  * \param p_section pointer to the PSI section structure
  * \param table_id expected table id
@@ -140,7 +140,8 @@ bool dvbpsi_CheckPSISection(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t *p_section,
  *****************************************************************************/
 /*!
  * \fn bool dvbpsi_ValidPSISection(dvbpsi_psi_section_t* p_section)
- * \brief Validity check of a PSI section.
+ * \brief Validity check of a PSI section, make sure to call this function on
+ * tables that have a CRC32 (@see dvbpsi_has_CRC32() function)
  * \param p_section pointer to the PSI section structure
  * \return boolean value (false if the section is not valid).
  *
@@ -160,6 +161,41 @@ bool dvbpsi_ValidPSISection(dvbpsi_psi_section_t* p_section);
  */
 void dvbpsi_BuildPSISection(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t* p_section);
 
+/*****************************************************************************
+ * dvbpsi_CalculateCRC32
+ *****************************************************************************/
+/*!
+ * \fn void dvbpsi_CalculateCRC32(dvbpsi_psi_section_t *p_section);
+ * \brief Calculate the CRC32 field accourding to ISO/IEC 13818-1,
+ * ITU-T Rec H.222.0 or ETSI EN 300 468 v1.13.1.
+ * \param p_section pointer to PSI section, make sure p_payload_end does not
+ * include the CRC32 field.
+ * \return nothing.
+ */
+void dvbpsi_CalculateCRC32(dvbpsi_psi_section_t *p_section);
+
+/*****************************************************************************
+ * dvbpsi_has_CRC32
+ *****************************************************************************/
+/*!
+ * \fn static inline bool dvbpsi_has_CRC32(const uint8_t* table_id)
+ * \brief Check if this table_id has a CRC32 field accourding to ISO/IEC 13818-1,
+ * ITU-T Rec H.222.0 or ETSI EN 300 468 v1.13.1.
+ * \param p_section pointer to decoded PSI section
+ * \return false if PSI section has no CRC32 according to the specification,
+ * true otherwise.
+ */
+static inline bool dvbpsi_has_CRC32(dvbpsi_psi_section_t *p_section)
+{
+    if ((p_section->i_table_id == (uint8_t) 0x70) /* TDT (has no CRC 32) */ ||
+        (p_section->i_table_id == (uint8_t) 0x71) /* RST (has no CRC 32) */ ||
+        (p_section->i_table_id == (uint8_t) 0x72) /*  ST (has no CRC 32) */ ||
+        (p_section->i_table_id == (uint8_t) 0x7E))/* DIT (has no CRC 32) */
+        return false;
+
+    return (p_section->b_syntax_indicator || (p_section->i_table_id == 0x73));
+}
+
 #ifdef __cplusplus
 };
 #endif

src/tables/tot.c

@@ -318,8 +318,6 @@ void dvbpsi_tot_sections_gather(dvbpsi_t* p_dvbpsi,
                                  p_section->i_table_id == 0x73)) ? /* TOT */
                                     p_section->i_table_id : 0x70;
 
-    /* FIXME: setting b_syntax_indicator is a workaround here */
-    p_section->b_syntax_indicator = true;
     if (!dvbpsi_CheckPSISection(p_dvbpsi, p_section, i_table_id, "TDT/TOT decoder"))
     {
         dvbpsi_DeletePSISections(p_section);
@@ -403,27 +401,21 @@ void dvbpsi_tot_sections_gather(dvbpsi_t* p_dvbpsi,
  *****************************************************************************/
 static bool dvbpsi_tot_section_valid(dvbpsi_t *p_dvbpsi, dvbpsi_psi_section_t* p_section)
 {
+    /* TDT table */
     if (p_section->i_table_id == 0x70)
     {
         /* A TDT (table_id 0x70) always has a length of 5 bytes (which is only the UTC time) */
         if (p_section->i_length != 5)
         {
-            dvbpsi_error(p_dvbpsi, "TDT/TOT decoder",
+            dvbpsi_error(p_dvbpsi, "TDT decoder",
                          "TDT has an invalid payload size (%d bytes) !!!",
                           p_section->i_length);
             return false;
         }
-        return true;
-    }
-
-    if (p_section->b_syntax_indicator &&
-        !dvbpsi_ValidPSISection(p_section))
-    {
-        dvbpsi_error(p_dvbpsi, "TDT/TOT decoder",
-                               "Bad CRC_32!!!");
-        return false;
     }
 
+    /* CRC32 has already been checked by dvbpsi_packet_push()
+     * and by dvbpsi_BuildPSISection(). */
     return true;
 }
 
@@ -537,34 +529,21 @@ dvbpsi_psi_section_t* dvbpsi_tot_sections_generate(dvbpsi_t *p_dvbpsi, dvbpsi_to
         p_result->p_payload_start[6] =  (p_result->i_length - 7)       & 0xff;
     }
 
+    /* Build the PSI section including the CRC32 on the playload.
+     * NOTE: The p_payload_end pointer should point to the last byte
+     * of the payload without the CRC32 field.
+     */
+    dvbpsi_BuildPSISection(p_dvbpsi, p_result);
+
     if (p_result->i_table_id == 0x73)
     {
         /* A TOT has a CRC_32 although it's a private section,
            but the CRC_32 is part of the payload! */
+        p_tot->i_crc = p_result->i_crc;
         p_result->p_payload_end += 4;
         p_result->i_length += 4;
     }
 
-    dvbpsi_BuildPSISection(p_dvbpsi, p_result);
-
-    if (p_result->i_table_id == 0x73)
-    {
-        uint8_t* p_byte = p_result->p_data;
-        p_tot->i_crc = 0xffffffff;
-
-        while (p_byte < p_result->p_payload_end - 4)
-        {
-            p_tot->i_crc =   (p_tot->i_crc << 8)
-                           ^ dvbpsi_crc32_table[(p_tot->i_crc >> 24) ^ (*p_byte)];
-            p_byte++;
-        }
-
-        p_byte[0] = (p_tot->i_crc >> 24) & 0xff;
-        p_byte[1] = (p_tot->i_crc >> 16) & 0xff;
-        p_byte[2] = (p_tot->i_crc >> 8) & 0xff;
-        p_byte[3] = p_tot->i_crc & 0xff;
-    }
-
     if (!dvbpsi_tot_section_valid(p_dvbpsi, p_result))
     {
         dvbpsi_error(p_dvbpsi, "TDT/TOT generator", "********************************************");