Re: [FFmpeg-devel] [PATCH 8/8] avcodec/nuv: Avoid duplicating frames

2019-08-15 Thread James Almer 
On 8/12/2019 4:17 PM, Michael Niedermayer wrote:
> Fixes: Timeout (14sec -> 133ms)
> Fixes: 
> 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> Fixes: 
> 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
>  (35sec ->0.5sec)
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/nuv.c  | 37 ++---
>  tests/ref/fate/nuv-rtjpeg |  1 -
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> index 75b14bce5b..0952b537b7 100644
> --- a/libavcodec/nuv.c
> +++ b/libavcodec/nuv.c
> @@ -42,6 +42,8 @@ typedef struct NuvContext {
>  unsigned char *decomp_buf;
>  uint32_t lq[64], cq[64];
>  RTJpegContext rtj;
> +int need_flush;
> +AVPacket flush_pkt;
>  } NuvContext;
>  
>  static const uint8_t fallback_lquant[] = {
> @@ -66,6 +68,12 @@ static const uint8_t fallback_cquant[] = {
>  99, 99, 99, 99, 99, 99, 99, 99
>  };
>  
> +static void decode_flush(AVCodecContext *avctx){
> +NuvContext *s = avctx->priv_data;
> +
> +s->need_flush = 0;
> +}
> +
>  /**
>   * @brief copy frame data from buffer to AVFrame, handling stride.
>   * @param f destination AVFrame
> @@ -172,6 +180,26 @@ static int decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  NUV_COPY_LAST = 'L'
>  } comptype;
>  
> +if (!avpkt->data) {
> +if (c->need_flush) {
> +c->need_flush = 0;
> +if ((ret = ff_reget_buffer(avctx, c->pic)) < 0)
> +return ret;
> +c->pic->pkt_pos  = c->flush_pkt.pos;
> +c->pic->pkt_duration = c->flush_pkt.duration;
> +c->pic->pkt_dts  = c->flush_pkt.dts;
> +c->pic->pkt_pts  =
> +c->pic->pts  = c->flush_pkt.pts;
> +if ((ret = av_frame_ref(data, c->pic)) < 0)
> +return ret;
> +*got_frame = 1;

Can't this be split off and shared in some way instead of implementing
it for every decoder that needs to remove duplicate frames? Or maybe
handle it in the generic decode.c code.

> +}
> +return 0;
> +}
> +c->flush_pkt = *avpkt;
> +c->pic->pkt_dts = c->flush_pkt.dts;
> +
> +
>  if (buf_size < 12) {
>  av_log(avctx, AV_LOG_ERROR, "coded frame too small\n");
>  return AVERROR_INVALIDDATA;
> @@ -204,8 +232,8 @@ static int decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  }
>  break;
>  case NUV_COPY_LAST:
> -keyframe = 0;
> -break;
> +c->need_flush = 1;
> +return buf_size;
>  default:
>  keyframe = 1;
>  break;
> @@ -313,6 +341,7 @@ retry:
>  if ((result = av_frame_ref(picture, c->pic)) < 0)
>  return result;
>  
> +c->need_flush = 0;
>  *got_frame = 1;
>  return orig_size;
>  }
> @@ -364,6 +393,8 @@ AVCodec ff_nuv_decoder = {
>  .init   = decode_init,
>  .close  = decode_end,
>  .decode = decode_frame,
> -.capabilities   = AV_CODEC_CAP_DR1,
> +.flush  = decode_flush,
> +.caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS,
> +.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
>  .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
>  };
> diff --git a/tests/ref/fate/nuv-rtjpeg b/tests/ref/fate/nuv-rtjpeg
> index b6f3b080dc..0914b985ec 100644
> --- a/tests/ref/fate/nuv-rtjpeg
> +++ b/tests/ref/fate/nuv-rtjpeg
> @@ -6,7 +6,6 @@
>  0,118,118,0,   460800, 0x54aedafe
>  0,152,152,0,   460800, 0xb7aa8b56
>  0,177,177,0,   460800, 0x283ea3b5
> -0,202,202,0,   460800, 0x283ea3b5
>  0,235,235,0,   460800, 0x10e577de
>  0,269,269,0,   460800, 0x4e091ee2
>  0,302,302,0,   460800, 0x2ea88828
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 8/8] avcodec/nuv: Avoid duplicating frames

2019-08-14 Thread Michael Niedermayer 
On Wed, Aug 14, 2019 at 06:02:13PM +0200, Paul B Mahol wrote:
> On Mon, Aug 12, 2019 at 9:20 PM Michael Niedermayer 
> wrote:
> 
> > Fixes: Timeout (14sec -> 133ms)
> > Fixes:
> > 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> > Fixes:
> > 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
> > (35sec ->0.5sec)
> >
> >
> Why? This is bad idea, same like for qtrle and bunch of other cases.

This was discussed previously IIRC
Either a codec can be used to turn tiny input to huge number of output frames 
by frame duplication. or
it doesnt do the duplication and rather provides metadata of some sort

In the first case if the next step is a filter it can be slower
In the first case if the next step is a variable fps encoder it can be slower
in the first case it can make a DOS attack less expensive for an attacker

The second case, that is with metadata for example not returning a frame but
relying on timestamps these issues are reduced
thats why i suggest that way but if the community prefers something else then
sure it can be done. But from what i remember the oppinions where mixed on
which way is preferred

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 8/8] avcodec/nuv: Avoid duplicating frames

2019-08-14 Thread Paul B Mahol 
On Mon, Aug 12, 2019 at 9:20 PM Michael Niedermayer 
wrote:

> Fixes: Timeout (14sec -> 133ms)
> Fixes:
> 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> Fixes:
> 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
> (35sec ->0.5sec)
>
>
Why? This is bad idea, same like for qtrle and bunch of other cases.



> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> :
> Michael Niedermayer 
> ---
>  libavcodec/nuv.c  | 37 ++---
>  tests/ref/fate/nuv-rtjpeg |  1 -
>  2 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> index 75b14bce5b..0952b537b7 100644
> --- a/libavcodec/nuv.c
> +++ b/libavcodec/nuv.c
> @@ -42,6 +42,8 @@ typedef struct NuvContext {
>  unsigned char *decomp_buf;
>  uint32_t lq[64], cq[64];
>  RTJpegContext rtj;
> +int need_flush;
> +AVPacket flush_pkt;
>  } NuvContext;
>
>  static const uint8_t fallback_lquant[] = {
> @@ -66,6 +68,12 @@ static const uint8_t fallback_cquant[] = {
>  99, 99, 99, 99, 99, 99, 99, 99
>  };
>
> +static void decode_flush(AVCodecContext *avctx){
> +NuvContext *s = avctx->priv_data;
> +
> +s->need_flush = 0;
> +}
> +
>  /**
>   * @brief copy frame data from buffer to AVFrame, handling stride.
>   * @param f destination AVFrame
> @@ -172,6 +180,26 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame,
>  NUV_COPY_LAST = 'L'
>  } comptype;
>
> +if (!avpkt->data) {
> +if (c->need_flush) {
> +c->need_flush = 0;
> +if ((ret = ff_reget_buffer(avctx, c->pic)) < 0)
> +return ret;
> +c->pic->pkt_pos  = c->flush_pkt.pos;
> +c->pic->pkt_duration = c->flush_pkt.duration;
> +c->pic->pkt_dts  = c->flush_pkt.dts;
> +c->pic->pkt_pts  =
> +c->pic->pts  = c->flush_pkt.pts;
> +if ((ret = av_frame_ref(data, c->pic)) < 0)
> +return ret;
> +*got_frame = 1;
> +}
> +return 0;
> +}
> +c->flush_pkt = *avpkt;
> +c->pic->pkt_dts = c->flush_pkt.dts;
> +
> +
>  if (buf_size < 12) {
>  av_log(avctx, AV_LOG_ERROR, "coded frame too small\n");
>  return AVERROR_INVALIDDATA;
> @@ -204,8 +232,8 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame,
>  }
>  break;
>  case NUV_COPY_LAST:
> -keyframe = 0;
> -break;
> +c->need_flush = 1;
> +return buf_size;
>  default:
>  keyframe = 1;
>  break;
> @@ -313,6 +341,7 @@ retry:
>  if ((result = av_frame_ref(picture, c->pic)) < 0)
>  return result;
>
> +c->need_flush = 0;
>  *got_frame = 1;
>  return orig_size;
>  }
> @@ -364,6 +393,8 @@ AVCodec ff_nuv_decoder = {
>  .init   = decode_init,
>  .close  = decode_end,
>  .decode = decode_frame,
> -.capabilities   = AV_CODEC_CAP_DR1,
> +.flush  = decode_flush,
> +.caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS,
> +.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
>  .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
>  };
> diff --git a/tests/ref/fate/nuv-rtjpeg b/tests/ref/fate/nuv-rtjpeg
> index b6f3b080dc..0914b985ec 100644
> --- a/tests/ref/fate/nuv-rtjpeg
> +++ b/tests/ref/fate/nuv-rtjpeg
> @@ -6,7 +6,6 @@
>  0,118,118,0,   460800, 0x54aedafe
>  0,152,152,0,   460800, 0xb7aa8b56
>  0,177,177,0,   460800, 0x283ea3b5
> -0,202,202,0,   460800, 0x283ea3b5
>  0,235,235,0,   460800, 0x10e577de
>  0,269,269,0,   460800, 0x4e091ee2
>  0,302,302,0,   460800, 0x2ea88828
> --
> 2.22.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".