diff mbox series

[FFmpeg-devel,01/36] avcodec/vp9_superframe_bsf: Check for existence of data before reading it

Message ID 20200530160541.29517-1-andreas.rheinhardt@gmail.com
State New
Headers show
Series [FFmpeg-devel,01/36] avcodec/vp9_superframe_bsf: Check for existence of data before reading it
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt May 30, 2020, 4:05 p.m. UTC
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/vp9_superframe_bsf.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

James Almer May 30, 2020, 5 p.m. UTC | #1
On 5/30/2020 1:05 PM, Andreas Rheinhardt wrote:
> Using par->extradata instead of ctx->par_in->extradata makes the code
> more readable and removes an overlong line.
> 
> Furthermore, check for extradata_size instead of for the existence of
> extradata as nothing needs to be done in case extradata_size is zero.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/dump_extradata_bsf.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/dump_extradata_bsf.c b/libavcodec/dump_extradata_bsf.c
> index b6ef8b3e6b..be5e23ba5b 100644
> --- a/libavcodec/dump_extradata_bsf.c
> +++ b/libavcodec/dump_extradata_bsf.c
> @@ -40,6 +40,7 @@ typedef struct DumpExtradataContext {
>  static int dump_extradata(AVBSFContext *ctx, AVPacket *out)
>  {
>      DumpExtradataContext *s = ctx->priv_data;
> +    const AVCodecParameters *par;
>      AVPacket *in = &s->pkt;
>      int ret = 0;
>  
> @@ -47,17 +48,17 @@ static int dump_extradata(AVBSFContext *ctx, AVPacket *out)
>      if (ret < 0)
>          return ret;
>  
> -    if (ctx->par_in->extradata &&
> +    if ((par = ctx->par_in)->extradata_size &&

Put the assignment in its own line, please. Saving one line is not worth
making the code less readable.

>          (s->freq == DUMP_FREQ_ALL ||
>           (s->freq == DUMP_FREQ_KEYFRAME && in->flags & AV_PKT_FLAG_KEY)) &&
> -         (in->size < ctx->par_in->extradata_size ||
> -          memcmp(in->data, ctx->par_in->extradata, ctx->par_in->extradata_size))) {
> -        if (in->size >= INT_MAX - ctx->par_in->extradata_size) {
> +         (in->size < par->extradata_size ||
> +          memcmp(in->data, par->extradata, par->extradata_size))) {
> +        if (in->size >= INT_MAX - par->extradata_size) {
>              ret = AVERROR(ERANGE);
>              goto fail;
>          }
>  
> -        ret = av_new_packet(out, in->size + ctx->par_in->extradata_size);
> +        ret = av_new_packet(out, in->size + par->extradata_size);
>          if (ret < 0)
>              goto fail;
>  
> @@ -67,8 +68,8 @@ static int dump_extradata(AVBSFContext *ctx, AVPacket *out)
>              goto fail;
>          }
>  
> -        memcpy(out->data, ctx->par_in->extradata, ctx->par_in->extradata_size);
> -        memcpy(out->data + ctx->par_in->extradata_size, in->data, in->size);
> +        memcpy(out->data, par->extradata, par->extradata_size);
> +        memcpy(out->data + par->extradata_size, in->data, in->size);
>      } else {
>          av_packet_move_ref(out, in);
>      }
>
Anton Khirnov June 1, 2020, 8:22 a.m. UTC | #2
Quoting Andreas Rheinhardt (2020-05-30 18:05:06)
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/vp9_superframe_bsf.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/vp9_superframe_bsf.c b/libavcodec/vp9_superframe_bsf.c
> index 57681e29e4..34a47aa69e 100644
> --- a/libavcodec/vp9_superframe_bsf.c
> +++ b/libavcodec/vp9_superframe_bsf.c
> @@ -108,6 +108,11 @@ static int vp9_superframe_filter(AVBSFContext *ctx, AVPacket *pkt)
>      if (res < 0)
>          return res;
>  
> +    if (pkt->size <= 0) {
> +        res = AVERROR_INVALIDDATA;
> +        goto done;
> +    }
> +
>      marker = pkt->data[pkt->size - 1];
>      if ((marker & 0xe0) == 0xc0) {
>          int nbytes = 1 + ((marker >> 3) & 0x3);
> -- 
> 2.20.1

I wonder if it wouldn't be better to add an internal capability for
"bsf supports zero-sized packets" and have the internal code reject
packets instead.
diff mbox series

Patch

diff --git a/libavcodec/vp9_superframe_bsf.c b/libavcodec/vp9_superframe_bsf.c
index 57681e29e4..34a47aa69e 100644
--- a/libavcodec/vp9_superframe_bsf.c
+++ b/libavcodec/vp9_superframe_bsf.c
@@ -108,6 +108,11 @@  static int vp9_superframe_filter(AVBSFContext *ctx, AVPacket *pkt)
     if (res < 0)
         return res;
 
+    if (pkt->size <= 0) {
+        res = AVERROR_INVALIDDATA;
+        goto done;
+    }
+
     marker = pkt->data[pkt->size - 1];
     if ((marker & 0xe0) == 0xc0) {
         int nbytes = 1 + ((marker >> 3) & 0x3);