[FFmpeg-devel] lavc: vt_hevc: fix crash if vps_list[0] or sps_list[0] are null

Submitted by Rodger Combs on Jan. 10, 2019, 2:26 a.m.

Details

Message ID 20190110022641.95676-1-rodger.combs@gmail.com
State New
Headers show

Commit Message

Rodger Combs Jan. 10, 2019, 2:26 a.m.
Instead of assuming id 0 is used, use the same logic as used for PPS,
where all available entries in the list are emitted.
---
 libavcodec/videotoolbox.c | 86 ++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 46 deletions(-)

Comments

Aman Gupta Jan. 10, 2019, 7:10 p.m.
On Wed, Jan 9, 2019 at 6:32 PM Rodger Combs <rodger.combs@gmail.com> wrote:

> Instead of assuming id 0 is used, use the same logic as used for PPS,
> where all available entries in the list are emitted.
>

Patch LGTM. Apart from the SEGV fix, this probably helps VideoToolbox
decode certain types of samples with multiple VPS/SPS correctly.

I think I started a similar patch at once point but it turned into a mess.
The use of macros here helps maintain some sanity.

Will commit in a few days if no one objects.

Aman


> ---
>  libavcodec/videotoolbox.c | 86 ++++++++++++++++++---------------------
>  1 file changed, 40 insertions(+), 46 deletions(-)
>
> diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> index da7236f100..fb3501f413 100644
> --- a/libavcodec/videotoolbox.c
> +++ b/libavcodec/videotoolbox.c
> @@ -176,26 +176,31 @@ CFDataRef
> ff_videotoolbox_avcc_extradata_create(AVCodecContext *avctx)
>  CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
>  {
>      HEVCContext *h = avctx->priv_data;
> -    const HEVCVPS *vps = (const HEVCVPS *)h->ps.vps_list[0]->data;
> -    const HEVCSPS *sps = (const HEVCSPS *)h->ps.sps_list[0]->data;
> -    int i, num_pps = 0;
> +    int i, num_vps = 0, num_sps = 0, num_pps = 0;
> +    const HEVCVPS *vps = h->ps.vps;
> +    const HEVCSPS *sps = h->ps.sps;
>      const HEVCPPS *pps = h->ps.pps;
>      PTLCommon ptlc = vps->ptl.general_ptl;
>      VUI vui = sps->vui;
>      uint8_t parallelismType;
>      CFDataRef data = NULL;
>      uint8_t *p;
> -    int vt_extradata_size = 23 + 5 + vps->data_size + 5 + sps->data_size
> + 3;
> +    int vt_extradata_size = 23 + 3 + 3 + 3;
>      uint8_t *vt_extradata;
>
> -    for (i = 0; i < HEVC_MAX_PPS_COUNT; i++) {
> -        if (h->ps.pps_list[i]) {
> -            const HEVCPPS *pps = (const HEVCPPS *)h->ps.pps_list[i]->data;
> -            vt_extradata_size += 2 + pps->data_size;
> -            num_pps++;
> -        }
> +#define COUNT_SIZE_PS(T, t) \
> +    for (i = 0; i < HEVC_MAX_##T##PS_COUNT; i++) { \
> +        if (h->ps.t##ps_list[i]) { \
> +            const HEVC##T##PS *lps = (const HEVC##T##PS
> *)h->ps.t##ps_list[i]->data; \
> +            vt_extradata_size += 2 + lps->data_size; \
> +            num_##t##ps++; \
> +        } \
>      }
>
> +    COUNT_SIZE_PS(V, v)
> +    COUNT_SIZE_PS(S, s)
> +    COUNT_SIZE_PS(P, p)
> +
>      vt_extradata = av_malloc(vt_extradata_size);
>      if (!vt_extradata)
>          return NULL;
> @@ -286,44 +291,33 @@ CFDataRef
> ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
>      AV_W8(p + 22, 3);
>
>      p += 23;
> -    /* vps */
> -    /*
> -     * bit(1) array_completeness;
> -     * unsigned int(1) reserved = 0;
> -     * unsigned int(6) NAL_unit_type;
> -     */
> -    AV_W8(p, 1 << 7 |
> -             HEVC_NAL_VPS & 0x3f);
> -    /* unsigned int(16) numNalus; */
> -    AV_WB16(p + 1, 1);
> -    /* unsigned int(16) nalUnitLength; */
> -    AV_WB16(p + 3, vps->data_size);
> -    /* bit(8*nalUnitLength) nalUnit; */
> -    memcpy(p + 5, vps->data, vps->data_size);
> -    p += 5 + vps->data_size;
> -
> -    /* sps */
> -    AV_W8(p, 1 << 7 |
> -             HEVC_NAL_SPS & 0x3f);
> -    AV_WB16(p + 1, 1);
> -    AV_WB16(p + 3, sps->data_size);
> -    memcpy(p + 5, sps->data, sps->data_size);
> -    p += 5 + sps->data_size;
> -
> -    /* pps */
> -    AV_W8(p, 1 << 7 |
> -             HEVC_NAL_PPS & 0x3f);
> -    AV_WB16(p + 1, num_pps);
> -    p += 3;
> -    for (i = 0; i < HEVC_MAX_PPS_COUNT; i++) {
> -        if (h->ps.pps_list[i]) {
> -            const HEVCPPS *pps = (const HEVCPPS *)h->ps.pps_list[i]->data;
> -            AV_WB16(p, pps->data_size);
> -            memcpy(p + 2, pps->data, pps->data_size);
> -            p += 2 + pps->data_size;
> -        }
> +
> +#define APPEND_PS(T, t) \
> +    /* \
> +     * bit(1) array_completeness; \
> +     * unsigned int(1) reserved = 0; \
> +     * unsigned int(6) NAL_unit_type; \
> +     */ \
> +    AV_W8(p, 1 << 7 | \
> +             HEVC_NAL_##T##PS & 0x3f); \
> +    /* unsigned int(16) numNalus; */ \
> +    AV_WB16(p + 1, num_##t##ps); \
> +    p += 3; \
> +    for (i = 0; i < HEVC_MAX_##T##PS_COUNT; i++) { \
> +        if (h->ps.t##ps_list[i]) { \
> +            const HEVC##T##PS *lps = (const HEVC##T##PS
> *)h->ps.t##ps_list[i]->data; \
> +            /* unsigned int(16) nalUnitLength; */ \
> +            AV_WB16(p, lps->data_size); \
> +            /* bit(8*nalUnitLength) nalUnit; */ \
> +            memcpy(p + 2, lps->data, lps->data_size); \
> +            p += 2 + lps->data_size; \
> +        } \
>      }
>
> +    APPEND_PS(V, v)
> +    APPEND_PS(S, s)
> +    APPEND_PS(P, p)
> +
>      av_assert0(p - vt_extradata == vt_extradata_size);
>
>      data = CFDataCreate(kCFAllocatorDefault, vt_extradata,
> vt_extradata_size);
> --
> 2.19.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Patch hide | download patch | download mbox

diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
index da7236f100..fb3501f413 100644
--- a/libavcodec/videotoolbox.c
+++ b/libavcodec/videotoolbox.c
@@ -176,26 +176,31 @@  CFDataRef ff_videotoolbox_avcc_extradata_create(AVCodecContext *avctx)
 CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
 {
     HEVCContext *h = avctx->priv_data;
-    const HEVCVPS *vps = (const HEVCVPS *)h->ps.vps_list[0]->data;
-    const HEVCSPS *sps = (const HEVCSPS *)h->ps.sps_list[0]->data;
-    int i, num_pps = 0;
+    int i, num_vps = 0, num_sps = 0, num_pps = 0;
+    const HEVCVPS *vps = h->ps.vps;
+    const HEVCSPS *sps = h->ps.sps;
     const HEVCPPS *pps = h->ps.pps;
     PTLCommon ptlc = vps->ptl.general_ptl;
     VUI vui = sps->vui;
     uint8_t parallelismType;
     CFDataRef data = NULL;
     uint8_t *p;
-    int vt_extradata_size = 23 + 5 + vps->data_size + 5 + sps->data_size + 3;
+    int vt_extradata_size = 23 + 3 + 3 + 3;
     uint8_t *vt_extradata;
 
-    for (i = 0; i < HEVC_MAX_PPS_COUNT; i++) {
-        if (h->ps.pps_list[i]) {
-            const HEVCPPS *pps = (const HEVCPPS *)h->ps.pps_list[i]->data;
-            vt_extradata_size += 2 + pps->data_size;
-            num_pps++;
-        }
+#define COUNT_SIZE_PS(T, t) \
+    for (i = 0; i < HEVC_MAX_##T##PS_COUNT; i++) { \
+        if (h->ps.t##ps_list[i]) { \
+            const HEVC##T##PS *lps = (const HEVC##T##PS *)h->ps.t##ps_list[i]->data; \
+            vt_extradata_size += 2 + lps->data_size; \
+            num_##t##ps++; \
+        } \
     }
 
+    COUNT_SIZE_PS(V, v)
+    COUNT_SIZE_PS(S, s)
+    COUNT_SIZE_PS(P, p)
+
     vt_extradata = av_malloc(vt_extradata_size);
     if (!vt_extradata)
         return NULL;
@@ -286,44 +291,33 @@  CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx)
     AV_W8(p + 22, 3);
 
     p += 23;
-    /* vps */
-    /*
-     * bit(1) array_completeness;
-     * unsigned int(1) reserved = 0;
-     * unsigned int(6) NAL_unit_type;
-     */
-    AV_W8(p, 1 << 7 |
-             HEVC_NAL_VPS & 0x3f);
-    /* unsigned int(16) numNalus; */
-    AV_WB16(p + 1, 1);
-    /* unsigned int(16) nalUnitLength; */
-    AV_WB16(p + 3, vps->data_size);
-    /* bit(8*nalUnitLength) nalUnit; */
-    memcpy(p + 5, vps->data, vps->data_size);
-    p += 5 + vps->data_size;
-
-    /* sps */
-    AV_W8(p, 1 << 7 |
-             HEVC_NAL_SPS & 0x3f);
-    AV_WB16(p + 1, 1);
-    AV_WB16(p + 3, sps->data_size);
-    memcpy(p + 5, sps->data, sps->data_size);
-    p += 5 + sps->data_size;
-
-    /* pps */
-    AV_W8(p, 1 << 7 |
-             HEVC_NAL_PPS & 0x3f);
-    AV_WB16(p + 1, num_pps);
-    p += 3;
-    for (i = 0; i < HEVC_MAX_PPS_COUNT; i++) {
-        if (h->ps.pps_list[i]) {
-            const HEVCPPS *pps = (const HEVCPPS *)h->ps.pps_list[i]->data;
-            AV_WB16(p, pps->data_size);
-            memcpy(p + 2, pps->data, pps->data_size);
-            p += 2 + pps->data_size;
-        }
+
+#define APPEND_PS(T, t) \
+    /* \
+     * bit(1) array_completeness; \
+     * unsigned int(1) reserved = 0; \
+     * unsigned int(6) NAL_unit_type; \
+     */ \
+    AV_W8(p, 1 << 7 | \
+             HEVC_NAL_##T##PS & 0x3f); \
+    /* unsigned int(16) numNalus; */ \
+    AV_WB16(p + 1, num_##t##ps); \
+    p += 3; \
+    for (i = 0; i < HEVC_MAX_##T##PS_COUNT; i++) { \
+        if (h->ps.t##ps_list[i]) { \
+            const HEVC##T##PS *lps = (const HEVC##T##PS *)h->ps.t##ps_list[i]->data; \
+            /* unsigned int(16) nalUnitLength; */ \
+            AV_WB16(p, lps->data_size); \
+            /* bit(8*nalUnitLength) nalUnit; */ \
+            memcpy(p + 2, lps->data, lps->data_size); \
+            p += 2 + lps->data_size; \
+        } \
     }
 
+    APPEND_PS(V, v)
+    APPEND_PS(S, s)
+    APPEND_PS(P, p)
+
     av_assert0(p - vt_extradata == vt_extradata_size);
 
     data = CFDataCreate(kCFAllocatorDefault, vt_extradata, vt_extradata_size);