[FFmpeg-devel,1/4] h264_mp4toannexb: Remove unnecessary check

Submitted by Andreas Rheinhardt on July 22, 2019, 3:27 a.m.

Details

Message ID 20190722032716.40032-1-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt July 22, 2019, 3:27 a.m.
There can be at most 31 SPS and 255 PPS in the mp4/Matroska extradata.
Given that each has a size of at most 2^16-1, the length of the output
derived from these parameter sets can never overflow an ordinary 32 bit
integer. So use a simple int instead of uint64_t and remove the
unnecessary check.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/h264_mp4toannexb_bsf.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Michael Niedermayer July 24, 2019, 7:31 a.m.
On Mon, Jul 22, 2019 at 05:27:12AM +0200, Andreas Rheinhardt wrote:
> There can be at most 31 SPS and 255 PPS in the mp4/Matroska extradata.
> Given that each has a size of at most 2^16-1, the length of the output
> derived from these parameter sets can never overflow an ordinary 32 bit
> integer. So use a simple int instead of uint64_t and remove the
> unnecessary check.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/h264_mp4toannexb_bsf.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/libavcodec/h264_mp4toannexb_bsf.c b/libavcodec/h264_mp4toannexb_bsf.c
> index fb3f24ea40..8b2899f300 100644
> --- a/libavcodec/h264_mp4toannexb_bsf.c
> +++ b/libavcodec/h264_mp4toannexb_bsf.c
> @@ -68,7 +68,7 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding)
>  {
>      H264BSFContext *s = ctx->priv_data;
>      uint16_t unit_size;
> -    uint64_t total_size                 = 0;
> +    int     total_size                  = 0;
>      uint8_t *out                        = NULL, unit_nb, sps_done = 0,
>               sps_seen                   = 0, pps_seen = 0;
>      const uint8_t *extradata            = ctx->par_in->extradata + 4;
> @@ -91,12 +91,6 @@ static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding)
>  
>          unit_size   = AV_RB16(extradata);
>          total_size += unit_size + 4;
> -        if (total_size > INT_MAX - padding) {
> -            av_log(ctx, AV_LOG_ERROR,
> -                   "Too big extradata size, corrupted stream or invalid MP4/AVCC bitstream\n");
> -            av_free(out);
> -            return AVERROR(EINVAL);
> -        }

this should be replaced by an av_assertX() to ensure that no future change
or extension exceeds the limits needed for the check to be not needed.

thanks

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/h264_mp4toannexb_bsf.c b/libavcodec/h264_mp4toannexb_bsf.c
index fb3f24ea40..8b2899f300 100644
--- a/libavcodec/h264_mp4toannexb_bsf.c
+++ b/libavcodec/h264_mp4toannexb_bsf.c
@@ -68,7 +68,7 @@  static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding)
 {
     H264BSFContext *s = ctx->priv_data;
     uint16_t unit_size;
-    uint64_t total_size                 = 0;
+    int     total_size                  = 0;
     uint8_t *out                        = NULL, unit_nb, sps_done = 0,
              sps_seen                   = 0, pps_seen = 0;
     const uint8_t *extradata            = ctx->par_in->extradata + 4;
@@ -91,12 +91,6 @@  static int h264_extradata_to_annexb(AVBSFContext *ctx, const int padding)
 
         unit_size   = AV_RB16(extradata);
         total_size += unit_size + 4;
-        if (total_size > INT_MAX - padding) {
-            av_log(ctx, AV_LOG_ERROR,
-                   "Too big extradata size, corrupted stream or invalid MP4/AVCC bitstream\n");
-            av_free(out);
-            return AVERROR(EINVAL);
-        }
         if (extradata + 2 + unit_size > ctx->par_in->extradata + ctx->par_in->extradata_size) {
             av_log(ctx, AV_LOG_ERROR, "Packet header is not contained in global extradata, "
                    "corrupted stream or invalid MP4/AVCC bitstream\n");