[vlc-devel] [PATCH] h264_nal: fix NAL parsing

Thomas Guillem thomas at gllm.fr
Mon Jul 13 13:22:21 CEST 2015


The NAL start code prefix is "0x00 0x00 0x01" and is not always followed by an
other zero byte.

This commit also change the behavior of h264_get_spspps, h264_parse_sps, and
h264_parse_pps functions:
 - h264_get_spspps return pointers to NAL unit without the start code prefix.
 - h264_parse_sps and h264_parse_pps need NAL unit pointers without the start
   code prefix.
---
 modules/codec/omxil/mediacodec.c |  36 +++++++++-----
 modules/codec/videotoolbox.m     |  29 ++---------
 modules/packetizer/h264.c        |   4 +-
 modules/packetizer/h264_nal.c    | 102 ++++++++++++++++++++++-----------------
 modules/packetizer/h264_nal.h    |   5 +-
 5 files changed, 92 insertions(+), 84 deletions(-)

diff --git a/modules/codec/omxil/mediacodec.c b/modules/codec/omxil/mediacodec.c
index ccf9919..6b65906 100644
--- a/modules/codec/omxil/mediacodec.c
+++ b/modules/codec/omxil/mediacodec.c
@@ -207,6 +207,8 @@ vlc_module_begin ()
         add_shortcut( "mediacodec_jni" )
 vlc_module_end ()
 
+static const uint8_t p_nal_startcode[] = { 0x00, 0x00, 0x00, 0x01 };
+static const size_t i_nal_startcode_length = 4;
 
 static void CSDFree(decoder_t *p_dec)
 {
@@ -223,7 +225,8 @@ static void CSDFree(decoder_t *p_dec)
 }
 
 /* Create the p_sys->p_csd that will be sent via PutInput */
-static int CSDDup(decoder_t *p_dec, const struct csd *p_csd, size_t i_count)
+static int CSDDup(decoder_t *p_dec, const struct csd *p_csd, size_t i_count,
+                  const void *p_start_code, size_t i_start_code_length)
 {
     decoder_sys_t *p_sys = p_dec->p_sys;
     unsigned int i_last_csd_count = p_sys->i_csd_count;
@@ -248,19 +251,24 @@ static int CSDDup(decoder_t *p_dec, const struct csd *p_csd, size_t i_count)
     for (size_t i = 0; i < p_sys->i_csd_count; ++i)
     {
         p_sys->p_csd[i].p_buf = realloc_or_free(p_sys->p_csd[i].p_buf,
-                                                p_csd[i].i_size);
+                                                p_csd[i].i_size
+                                                + i_start_code_length);
         if (!p_sys->p_csd[i].p_buf)
         {
             CSDFree(p_dec);
             return VLC_ENOMEM;
         }
-        memcpy(p_sys->p_csd[i].p_buf, p_csd[i].p_buf, p_csd[i].i_size);
-        p_sys->p_csd[i].i_size = p_csd[i].i_size;
+        if( p_start_code )
+            memcpy(p_sys->p_csd[i].p_buf, p_start_code, i_start_code_length);
+        memcpy(p_sys->p_csd[i].p_buf + i_start_code_length, p_csd[i].p_buf,
+               p_csd[i].i_size);
+        p_sys->p_csd[i].i_size = p_csd[i].i_size + i_start_code_length;
     }
     return VLC_SUCCESS;
 }
 
-static bool CSDCmp(decoder_t *p_dec, struct csd *p_csd, size_t i_csd_count)
+static bool CSDCmp(decoder_t *p_dec, struct csd *p_csd, size_t i_csd_count,
+                   const void *p_start_code, size_t i_start_code_length)
 {
     decoder_sys_t *p_sys = p_dec->p_sys;
 
@@ -268,8 +276,9 @@ static bool CSDCmp(decoder_t *p_dec, struct csd *p_csd, size_t i_csd_count)
         return false;
     for (size_t i = 0; i < i_csd_count; ++i)
     {
-        if (p_sys->p_csd[i].i_size != p_csd[i].i_size
-         || memcmp(p_sys->p_csd[i].p_buf, p_csd[i].p_buf, p_csd[i].i_size) != 0)
+        if (p_sys->p_csd[i].i_size != p_csd[i].i_size + i_start_code_length
+         || memcmp(p_sys->p_csd[i].p_buf, p_start_code, i_start_code_length) != 0
+         || memcmp(p_sys->p_csd[i].p_buf + i_start_code_length, p_csd[i].p_buf, p_csd[i].i_size) != 0)
             return false;
     }
     return true;
@@ -307,7 +316,8 @@ static int H264SetCSD(decoder_t *p_dec, void *p_buf, size_t i_size,
         }
 
         /* Compare the SPS PPS with the old one */
-        if (!CSDCmp(p_dec, csd, i_csd_count))
+        if (!CSDCmp(p_dec, csd, i_csd_count, p_nal_startcode,
+                    i_nal_startcode_length))
         {
 char buffer[__MIN(200, i_size) * 4];
 memset(buffer, 0, __MIN(200, i_size));
@@ -333,8 +343,10 @@ fprintf(stderr, "PPS: %s\n", buffer);
                      i_sps_size, i_pps_size);
 
             /* In most use cases, p_sys->p_csd[0] contains a SPS, and
-             * p_sys->p_csd[1] contains a PPS */
-            if (CSDDup(p_dec, csd, i_csd_count))
+             * p_sys->p_csd[1] contains a PPS.
+             * These CSD need to be prefixed with the nal start code. */
+            if (CSDDup(p_dec, csd, i_csd_count, p_nal_startcode,
+                       i_nal_startcode_length))
                 return VLC_ENOMEM;
 
             if (p_size_changed)
@@ -394,7 +406,7 @@ static int StartMediaCodec(decoder_t *p_dec)
 
                     csd.p_buf = p_buf;
                     csd.i_size = size;
-                    CSDDup(p_dec, &csd, 1);
+                    CSDDup(p_dec, &csd, 1, NULL, 0);
                 }
             }
             free(p_buf);
@@ -405,7 +417,7 @@ static int StartMediaCodec(decoder_t *p_dec)
 
             csd.p_buf = p_dec->fmt_in.p_extra;
             csd.i_size = p_dec->fmt_in.i_extra;
-            CSDDup(p_dec, &csd, 1);
+            CSDDup(p_dec, &csd, 1, NULL, 0);
         }
 
         p_sys->i_csd_send = 0;
diff --git a/modules/codec/videotoolbox.m b/modules/codec/videotoolbox.m
index 7dc5da5..60ffa69 100644
--- a/modules/codec/videotoolbox.m
+++ b/modules/codec/videotoolbox.m
@@ -335,33 +335,12 @@ static int StartVideoToolbox(decoder_t *p_dec, block_t *p_block)
                         bo_add_8(&bo, 0xff);   /* 0b11111100 | lengthsize = 0x11 */
 
                         bo_add_8(&bo, 0xe0 | (i_sps_size > 0 ? 1 : 0));   /* 0b11100000 | sps_count */
-
-                        if (i_sps_size > 4) {
-                            /* the SPS data we have got includes 4 leading
-                             * bytes which we need to remove */
-                            uint8_t *fixed_sps = malloc(i_sps_size - 4);
-                            for (int i = 0; i < i_sps_size - 4; i++) {
-                                fixed_sps[i] = p_sps_buf[i+4];
-                            }
-
-                            bo_add_16be(&bo, i_sps_size - 4);
-                            bo_add_mem(&bo, i_sps_size - 4, fixed_sps);
-                            free(fixed_sps);
-                        }
+                        bo_add_16be(&bo, i_sps_size);
+                        bo_add_mem(&bo, i_sps_size, p_sps_buf);
 
                         bo_add_8(&bo, (i_pps_size > 0 ? 1 : 0));   /* pps_count */
-                        if (i_pps_size > 4) {
-                            /* the PPS data we have got includes 4 leading
-                             * bytes which we need to remove */
-                            uint8_t *fixed_pps = malloc(i_pps_size - 4);
-                            for (int i = 0; i < i_pps_size - 4; i++) {
-                                fixed_pps[i] = p_pps_buf[i+4];
-                            }
-
-                            bo_add_16be(&bo, i_pps_size - 4);
-                            bo_add_mem(&bo, i_pps_size - 4, fixed_pps);
-                            free(fixed_pps);
-                        }
+                        bo_add_16be(&bo, i_pps_size);
+                        bo_add_mem(&bo, i_pps_size, p_pps_buf);
 
                         extradata = CFDataCreate(kCFAllocatorDefault,
                                                  bo.b->p_buffer,
diff --git a/modules/packetizer/h264.c b/modules/packetizer/h264.c
index 292daf3..a265a83 100644
--- a/modules/packetizer/h264.c
+++ b/modules/packetizer/h264.c
@@ -817,7 +817,7 @@ static void PutSPS( decoder_t *p_dec, block_t *p_frag )
     decoder_sys_t *p_sys = p_dec->p_sys;
     struct nal_sps sps;
 
-    if( h264_parse_sps( p_frag->p_buffer, p_frag->i_buffer, &sps ) != 0 )
+    if( h264_parse_sps( &p_frag->p_buffer[4], p_frag->i_buffer - 4, &sps ) != 0 )
     {
         msg_Warn( p_dec, "invalid SPS (sps_id=%d)", sps.i_id );
         block_Release( p_frag );
@@ -867,7 +867,7 @@ static void PutPPS( decoder_t *p_dec, block_t *p_frag )
     decoder_sys_t *p_sys = p_dec->p_sys;
     struct nal_pps pps;
 
-    if( h264_parse_pps( p_frag->p_buffer, p_frag->i_buffer, &pps ) != 0 )
+    if( h264_parse_pps( &p_frag->p_buffer[4], p_frag->i_buffer - 4, &pps ) != 0 )
     {
         msg_Warn( p_dec, "invalid PPS (pps_id=%d sps_id=%d)", pps.i_id, pps.i_sps_id );
         block_Release( p_frag );
diff --git a/modules/packetizer/h264_nal.c b/modules/packetizer/h264_nal.c
index a35f861..a43a4aa 100644
--- a/modules/packetizer/h264_nal.c
+++ b/modules/packetizer/h264_nal.c
@@ -20,9 +20,10 @@
 
 #include "h264_nal.h"
 
+#include <assert.h>
 #include <limits.h>
 
-static const uint8_t annexb_startcode[] = { 0x00, 0x00, 0x00, 0x01 };
+static const uint8_t annexb_startcode[] = { 0x00, 0x00, 0x01 };
 
 int convert_sps_pps( decoder_t *p_dec, const uint8_t *p_buf,
                      uint32_t i_buf_size, uint8_t *p_out_buf,
@@ -140,6 +141,15 @@ void convert_h264_to_annexb( uint8_t *p_buf, uint32_t i_len,
     }
 }
 
+static size_t get_spspps_size( uint8_t *p_start, uint8_t *p_end )
+{
+    /* cf B.1.1: don't count trailing_zero_8bits */
+
+    while( *p_end == 0 && p_end > p_start )
+        p_end--;
+    return p_end - p_start + 1;
+}
+
 int h264_get_spspps( uint8_t *p_buf, size_t i_buf,
                      uint8_t **pp_sps, size_t *p_sps_size,
                      uint8_t **pp_pps, size_t *p_pps_size )
@@ -148,57 +158,63 @@ int h264_get_spspps( uint8_t *p_buf, size_t i_buf,
     size_t i_sps_size = 0, i_pps_size = 0;
     int i_nal_type = NAL_UNKNOWN;
 
-    if (unlikely(p_buf == NULL || i_buf == 0))
-        return -1;
+    assert( p_buf );
 
-    while( true )
+    while( i_buf > 0 )
     {
-        int i_inc = 0;
-
-        if( i_buf > 5 && memcmp( p_buf, annexb_startcode, 4 ) == 0 )
+        if( i_buf > 4 && memcmp( p_buf, annexb_startcode, 3 ) == 0 )
         {
-            i_nal_type = p_buf[4] & 0x1F;
-
-            /* size of startcode + nal_type */
-            i_inc = 5;
+            i_nal_type = p_buf[3] & 0x1F;
 
             /* pointer to the beginning of the sps/pps  */
             if( i_nal_type == NAL_SPS && !p_sps )
-                p_sps = p_buf;
+                p_sps = p_buf + 3;
             if( i_nal_type == NAL_PPS && !p_pps )
-                p_pps = p_buf;
-        } else {
-            i_inc = 1;
-        }
+                p_pps = p_buf + 3;
 
-        /* cf. 7.4.1.2.3 */
-        if( i_nal_type == NAL_UNKNOWN || i_nal_type > 18
-         || (i_nal_type >= 10 && i_nal_type <= 12))
-            return -1;
+            /* cf. 7.4.1.2.3 */
+            if( i_nal_type == NAL_UNKNOWN || i_nal_type > 18
+             || ( i_nal_type >= 10 && i_nal_type <= 12 ) )
+                return -1;
 
-        /* update SPS/PPS size if the new NAL is different than the last one */
-        if( !i_sps_size && p_sps && i_nal_type != NAL_SPS )
-            i_sps_size = p_buf - p_sps;
-        if( !i_pps_size && p_pps && i_nal_type != NAL_PPS )
-            i_pps_size = p_buf - p_pps;
+            /* update SPS/PPS size if the new NAL is different than the last
+             * one */
+            if( !i_sps_size && p_sps && i_nal_type != NAL_SPS )
+                i_sps_size = get_spspps_size( p_sps, p_buf );
+            if( !i_pps_size && p_pps && i_nal_type != NAL_PPS )
+                i_pps_size = get_spspps_size( p_pps, p_buf );
 
-        /* SPS/PPS are before the slices */
-        if (i_nal_type >= NAL_SLICE && i_nal_type <= NAL_SLICE_IDR )
-            break;
+            /* Found a SPS and a PPS */
+            if( i_sps_size && i_pps_size )
+                break;
 
-        i_buf -= i_inc;
-        p_buf += i_inc;
+            /* SPS/PPS are before the slices */
+            if ( i_nal_type >= NAL_SLICE && i_nal_type <= NAL_SLICE_IDR )
+                break;
 
-        if( i_buf == 0 )
-        {
-            /* update SPS/PPS size if we reach the end of the buffer */
-            if( !i_sps_size && p_sps )
-                i_sps_size = p_buf - p_sps;
-            if( !i_pps_size && p_pps )
-                i_pps_size = p_buf - p_pps;
-            break;
+            /* size of startcode + nal_type */
+            i_buf -= 4;
+            p_buf += 4;
+        } else {
+            /* cf B.1: Don't abort if there are leading_zero_8bits (only for
+             * the first Nal unit of the bytestream). */
+            if( i_nal_type == NAL_UNKNOWN && p_buf[0] != 0 )
+                return -1;
+
+            i_buf--;
+            p_buf++;
         }
     }
+
+    if( i_buf == 0 )
+    {
+        /* update SPS/PPS size if we reach the end of the bytestream */
+        if( !i_sps_size && p_sps )
+            i_sps_size = p_buf - p_sps;
+        if( !i_pps_size && p_pps )
+            i_pps_size = p_buf - p_pps;
+    }
+
     if( ( !p_sps || !i_sps_size ) && ( !p_pps || !i_pps_size ) )
         return -1;
     *pp_sps = p_sps;
@@ -217,13 +233,11 @@ int h264_parse_sps( const uint8_t *p_sps_buf, int i_sps_size,
     bs_t s;
     int i_tmp;
 
-    if (i_sps_size < 5 || (p_sps_buf[4] & 0x1f) != NAL_SPS)
+    if (i_sps_size < 1 || (p_sps_buf[0] & 0x1f) != NAL_SPS)
         return -1;
 
     memset( p_sps, 0, sizeof(struct nal_sps) );
-    CreateDecodedNAL( &pb_dec, &i_dec, &p_sps_buf[5],
-                      i_sps_size - 5 );
-
+    CreateDecodedNAL( &pb_dec, &i_dec, &p_sps_buf[1], i_sps_size - 1 );
     bs_init( &s, pb_dec, i_dec );
     int i_profile_idc = bs_read( &s, 8 );
     p_sps->i_profile = i_profile_idc;
@@ -471,11 +485,11 @@ int h264_parse_pps( const uint8_t *p_pps_buf, int i_pps_size,
 {
     bs_t s;
 
-    if (i_pps_size < 5 || (p_pps_buf[4] & 0x1f) != NAL_PPS)
+    if (i_pps_size < 1 || (p_pps_buf[0] & 0x1f) != NAL_PPS)
         return -1;
 
     memset( p_pps, 0, sizeof(struct nal_pps) );
-    bs_init( &s, &p_pps_buf[5], i_pps_size - 5 );
+    bs_init( &s, &p_pps_buf[1], i_pps_size - 1 );
     p_pps->i_id = bs_read_ue( &s ); // pps id
     p_pps->i_sps_id = bs_read_ue( &s ); // sps id
     if( p_pps->i_id >= PPS_MAX || p_pps->i_sps_id >= SPS_MAX )
diff --git a/modules/packetizer/h264_nal.h b/modules/packetizer/h264_nal.h
index 5a577d4..2632f7c 100644
--- a/modules/packetizer/h264_nal.h
+++ b/modules/packetizer/h264_nal.h
@@ -131,17 +131,20 @@ void convert_h264_to_annexb( uint8_t *p_buf, uint32_t i_len,
                              struct H264ConvertState *state );
 
 /* Get the SPS/PPS pointers from an Annex B buffer
- * Returns 0 if a SPS and/or a PPS is found */
+ * Returns 0 if a SPS and/or a PPS is found, pp_sps and pp_pps point to the nal
+ * unit without the start code prefix */
 int h264_get_spspps( uint8_t *p_buf, size_t i_buf,
                      uint8_t **pp_sps, size_t *p_sps_size,
                      uint8_t **pp_pps, size_t *p_pps_size );
 
 /* Parse a SPS into the struct nal_sps
+ * p_sps_buf should point to a SPS NAL unit without the start code prefix
  * Returns 0 in case of success */
 int h264_parse_sps( const uint8_t *p_sps_buf, int i_sps_size,
                     struct nal_sps *p_sps );
 
 /* Parse a PPS into the struct nal_pps
+ * p_pps_buf should point to a PPS NAL unit without the start code prefix
  * Returns 0 in case of success */
 int h264_parse_pps( const uint8_t *p_pps_buf, int i_pps_size,
                     struct nal_pps *p_pps );
-- 
2.1.4




More information about the vlc-devel mailing list