diff mbox

[FFmpeg-devel,6/6] avcodec/nuv: Avoid duplicating frames

Message ID 20190824181829.25724-6-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Aug. 24, 2019, 6:18 p.m. UTC
Testcase: 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
Testcase: 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/nuv.c          | 25 +++++++++++++++++++++----
 tests/ref/fate/nuv-rtjpeg |  1 -
 2 files changed, 21 insertions(+), 5 deletions(-)

Comments

Paul B Mahol Aug. 25, 2019, 6:37 a.m. UTC | #1
Very ugly. NAK.

On Sat, Aug 24, 2019 at 8:26 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Testcase:
> 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> Testcase:
> 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/nuv.c          | 25 +++++++++++++++++++++----
>  tests/ref/fate/nuv-rtjpeg |  1 -
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> index 75b14bce5b..39479d2389 100644
> --- a/libavcodec/nuv.c
> +++ b/libavcodec/nuv.c
> @@ -42,6 +42,7 @@ typedef struct NuvContext {
>      unsigned char *decomp_buf;
>      uint32_t lq[64], cq[64];
>      RTJpegContext rtj;
> +    AVPacket flush_pkt;
>  } NuvContext;
>
>  static const uint8_t fallback_lquant[] = {
> @@ -172,6 +173,20 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame,
>          NUV_COPY_LAST     = 'L'
>      } comptype;
>
> +    if (!avpkt->data) {
> +        if (avctx->internal->need_flush) {
> +            avctx->internal->need_flush = 0;
> +            ret = ff_setup_buffered_frame_for_return(avctx, data, c->pic,
> &c->flush_pkt);
> +            if (ret < 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 +219,8 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame,
>          }
>          break;
>      case NUV_COPY_LAST:
> -        keyframe = 0;
> -        break;
> +        avctx->internal->need_flush = 1;
> +        return buf_size;
>      default:
>          keyframe = 1;
>          break;
> @@ -313,6 +328,7 @@ retry:
>      if ((result = av_frame_ref(picture, c->pic)) < 0)
>          return result;
>
> +    avctx->internal->need_flush = 0;
>      *got_frame = 1;
>      return orig_size;
>  }
> @@ -364,6 +380,7 @@ AVCodec ff_nuv_decoder = {
>      .init           = decode_init,
>      .close          = decode_end,
>      .decode         = decode_frame,
> -    .capabilities   = AV_CODEC_CAP_DR1,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> +    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS |
> FF_CODEC_CAP_SETS_PKT_POS |
> +                      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.23.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-request@ffmpeg.org with subject "unsubscribe".
James Almer Aug. 25, 2019, 4:05 p.m. UTC | #2
On 8/24/2019 3:18 PM, Michael Niedermayer wrote:
> Testcase: 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> Testcase: 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/nuv.c          | 25 +++++++++++++++++++++----
>  tests/ref/fate/nuv-rtjpeg |  1 -
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> index 75b14bce5b..39479d2389 100644
> --- a/libavcodec/nuv.c
> +++ b/libavcodec/nuv.c
> @@ -42,6 +42,7 @@ typedef struct NuvContext {
>      unsigned char *decomp_buf;
>      uint32_t lq[64], cq[64];
>      RTJpegContext rtj;
> +    AVPacket flush_pkt;
>  } NuvContext;
>  
>  static const uint8_t fallback_lquant[] = {
> @@ -172,6 +173,20 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>          NUV_COPY_LAST     = 'L'
>      } comptype;
>  
> +    if (!avpkt->data) {
> +        if (avctx->internal->need_flush) {
> +            avctx->internal->need_flush = 0;
> +            ret = ff_setup_buffered_frame_for_return(avctx, data, c->pic, &c->flush_pkt);
> +            if (ret < 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 +219,8 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>          }
>          break;
>      case NUV_COPY_LAST:
> -        keyframe = 0;
> -        break;
> +        avctx->internal->need_flush = 1;
> +        return buf_size;
>      default:
>          keyframe = 1;
>          break;
> @@ -313,6 +328,7 @@ retry:
>      if ((result = av_frame_ref(picture, c->pic)) < 0)
>          return result;
>  
> +    avctx->internal->need_flush = 0;
>      *got_frame = 1;
>      return orig_size;
>  }
> @@ -364,6 +380,7 @@ AVCodec ff_nuv_decoder = {
>      .init           = decode_init,
>      .close          = decode_end,
>      .decode         = decode_frame,
> -    .capabilities   = AV_CODEC_CAP_DR1,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> +    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS | FF_CODEC_CAP_SETS_PKT_POS |
> +                      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

I haven't been following these cfr -> vfr patches too closely, but why
remove the duplicate frames (thus changing the expected compliant
behavior) instead of ensuring the duplicate ones are made with no
performance penalty by reusing the buffer reference from the previous frame?

>  0,        235,        235,        0,   460800, 0x10e577de
>  0,        269,        269,        0,   460800, 0x4e091ee2
>  0,        302,        302,        0,   460800, 0x2ea88828
>
Michael Niedermayer Aug. 25, 2019, 9:35 p.m. UTC | #3
On Sun, Aug 25, 2019 at 01:05:35PM -0300, James Almer wrote:
> On 8/24/2019 3:18 PM, Michael Niedermayer wrote:
> > Testcase: 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> > Testcase: 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/nuv.c          | 25 +++++++++++++++++++++----
> >  tests/ref/fate/nuv-rtjpeg |  1 -
> >  2 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> > index 75b14bce5b..39479d2389 100644
> > --- a/libavcodec/nuv.c
> > +++ b/libavcodec/nuv.c
> > @@ -42,6 +42,7 @@ typedef struct NuvContext {
> >      unsigned char *decomp_buf;
> >      uint32_t lq[64], cq[64];
> >      RTJpegContext rtj;
> > +    AVPacket flush_pkt;
> >  } NuvContext;
> >  
> >  static const uint8_t fallback_lquant[] = {
> > @@ -172,6 +173,20 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> >          NUV_COPY_LAST     = 'L'
> >      } comptype;
> >  
> > +    if (!avpkt->data) {
> > +        if (avctx->internal->need_flush) {
> > +            avctx->internal->need_flush = 0;
> > +            ret = ff_setup_buffered_frame_for_return(avctx, data, c->pic, &c->flush_pkt);
> > +            if (ret < 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 +219,8 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
> >          }
> >          break;
> >      case NUV_COPY_LAST:
> > -        keyframe = 0;
> > -        break;
> > +        avctx->internal->need_flush = 1;
> > +        return buf_size;
> >      default:
> >          keyframe = 1;
> >          break;
> > @@ -313,6 +328,7 @@ retry:
> >      if ((result = av_frame_ref(picture, c->pic)) < 0)
> >          return result;
> >  
> > +    avctx->internal->need_flush = 0;
> >      *got_frame = 1;
> >      return orig_size;
> >  }
> > @@ -364,6 +380,7 @@ AVCodec ff_nuv_decoder = {
> >      .init           = decode_init,
> >      .close          = decode_end,
> >      .decode         = decode_frame,
> > -    .capabilities   = AV_CODEC_CAP_DR1,
> > -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> > +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> > +    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS | FF_CODEC_CAP_SETS_PKT_POS |
> > +                      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
> 
> I haven't been following these cfr -> vfr patches too closely, but why
> remove the duplicate frames (thus changing the expected compliant
> behavior) instead of ensuring the duplicate ones are made with no
> performance penalty by reusing the buffer reference from the previous frame?

Because a filter or encoder or whatever follows the decoder would then process
them multiple times unneccessarily.

thx

[...]
Paul B Mahol Aug. 26, 2019, 7:53 a.m. UTC | #4
On Sun, Aug 25, 2019 at 11:35 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Sun, Aug 25, 2019 at 01:05:35PM -0300, James Almer wrote:
> > On 8/24/2019 3:18 PM, Michael Niedermayer wrote:
> > > Testcase:
> 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> > > Testcase:
> 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
> > >
> > > Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavcodec/nuv.c          | 25 +++++++++++++++++++++----
> > >  tests/ref/fate/nuv-rtjpeg |  1 -
> > >  2 files changed, 21 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> > > index 75b14bce5b..39479d2389 100644
> > > --- a/libavcodec/nuv.c
> > > +++ b/libavcodec/nuv.c
> > > @@ -42,6 +42,7 @@ typedef struct NuvContext {
> > >      unsigned char *decomp_buf;
> > >      uint32_t lq[64], cq[64];
> > >      RTJpegContext rtj;
> > > +    AVPacket flush_pkt;
> > >  } NuvContext;
> > >
> > >  static const uint8_t fallback_lquant[] = {
> > > @@ -172,6 +173,20 @@ static int decode_frame(AVCodecContext *avctx,
> void *data, int *got_frame,
> > >          NUV_COPY_LAST     = 'L'
> > >      } comptype;
> > >
> > > +    if (!avpkt->data) {
> > > +        if (avctx->internal->need_flush) {
> > > +            avctx->internal->need_flush = 0;
> > > +            ret = ff_setup_buffered_frame_for_return(avctx, data,
> c->pic, &c->flush_pkt);
> > > +            if (ret < 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 +219,8 @@ static int decode_frame(AVCodecContext *avctx,
> void *data, int *got_frame,
> > >          }
> > >          break;
> > >      case NUV_COPY_LAST:
> > > -        keyframe = 0;
> > > -        break;
> > > +        avctx->internal->need_flush = 1;
> > > +        return buf_size;
> > >      default:
> > >          keyframe = 1;
> > >          break;
> > > @@ -313,6 +328,7 @@ retry:
> > >      if ((result = av_frame_ref(picture, c->pic)) < 0)
> > >          return result;
> > >
> > > +    avctx->internal->need_flush = 0;
> > >      *got_frame = 1;
> > >      return orig_size;
> > >  }
> > > @@ -364,6 +380,7 @@ AVCodec ff_nuv_decoder = {
> > >      .init           = decode_init,
> > >      .close          = decode_end,
> > >      .decode         = decode_frame,
> > > -    .capabilities   = AV_CODEC_CAP_DR1,
> > > -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> > > +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> > > +    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS |
> FF_CODEC_CAP_SETS_PKT_POS |
> > > +                      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
> >
> > I haven't been following these cfr -> vfr patches too closely, but why
> > remove the duplicate frames (thus changing the expected compliant
> > behavior) instead of ensuring the duplicate ones are made with no
> > performance penalty by reusing the buffer reference from the previous
> frame?
>
> Because a filter or encoder or whatever follows the decoder would then
> process
> them multiple times unneccessarily.
>

Incorrect. This "extra" as you call processing is necessary.


> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer Aug. 26, 2019, 9:29 a.m. UTC | #5
On Mon, Aug 26, 2019 at 09:53:46AM +0200, Paul B Mahol wrote:
> On Sun, Aug 25, 2019 at 11:35 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Sun, Aug 25, 2019 at 01:05:35PM -0300, James Almer wrote:
> > > On 8/24/2019 3:18 PM, Michael Niedermayer wrote:
> > > > Testcase:
> > 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864
> > > > Testcase:
> > 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280
> > > >
> > > > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > >  libavcodec/nuv.c          | 25 +++++++++++++++++++++----
> > > >  tests/ref/fate/nuv-rtjpeg |  1 -
> > > >  2 files changed, 21 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
> > > > index 75b14bce5b..39479d2389 100644
> > > > --- a/libavcodec/nuv.c
> > > > +++ b/libavcodec/nuv.c
> > > > @@ -42,6 +42,7 @@ typedef struct NuvContext {
> > > >      unsigned char *decomp_buf;
> > > >      uint32_t lq[64], cq[64];
> > > >      RTJpegContext rtj;
> > > > +    AVPacket flush_pkt;
> > > >  } NuvContext;
> > > >
> > > >  static const uint8_t fallback_lquant[] = {
> > > > @@ -172,6 +173,20 @@ static int decode_frame(AVCodecContext *avctx,
> > void *data, int *got_frame,
> > > >          NUV_COPY_LAST     = 'L'
> > > >      } comptype;
> > > >
> > > > +    if (!avpkt->data) {
> > > > +        if (avctx->internal->need_flush) {
> > > > +            avctx->internal->need_flush = 0;
> > > > +            ret = ff_setup_buffered_frame_for_return(avctx, data,
> > c->pic, &c->flush_pkt);
> > > > +            if (ret < 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 +219,8 @@ static int decode_frame(AVCodecContext *avctx,
> > void *data, int *got_frame,
> > > >          }
> > > >          break;
> > > >      case NUV_COPY_LAST:
> > > > -        keyframe = 0;
> > > > -        break;
> > > > +        avctx->internal->need_flush = 1;
> > > > +        return buf_size;
> > > >      default:
> > > >          keyframe = 1;
> > > >          break;
> > > > @@ -313,6 +328,7 @@ retry:
> > > >      if ((result = av_frame_ref(picture, c->pic)) < 0)
> > > >          return result;
> > > >
> > > > +    avctx->internal->need_flush = 0;
> > > >      *got_frame = 1;
> > > >      return orig_size;
> > > >  }
> > > > @@ -364,6 +380,7 @@ AVCodec ff_nuv_decoder = {
> > > >      .init           = decode_init,
> > > >      .close          = decode_end,
> > > >      .decode         = decode_frame,
> > > > -    .capabilities   = AV_CODEC_CAP_DR1,
> > > > -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> > > > +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> > > > +    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS |
> > FF_CODEC_CAP_SETS_PKT_POS |
> > > > +                      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
> > >
> > > I haven't been following these cfr -> vfr patches too closely, but why
> > > remove the duplicate frames (thus changing the expected compliant
> > > behavior) instead of ensuring the duplicate ones are made with no
> > > performance penalty by reusing the buffer reference from the previous
> > frame?
> >
> > Because a filter or encoder or whatever follows the decoder would then
> > process
> > them multiple times unneccessarily.
> >
> 
> Incorrect. This "extra" as you call processing is necessary.

no
if your final output is vfr and for that you drop duplicate frames (be that
in the decoder or filter)
you would get the exact same output as when you drop in the decoder
just that by earlier droping you avoid some processing.
If the frames where marked by some flag as duplicate or if they refer to
the same memory then finding them becomes easier and there is less
processing. (this was suggested by james and myself but sofar others
have not confirmed they would agree to that)

Either a flag or using the same ref for the duplicate frames would
require changes as well. The current ref fixes only work with the
fuzzer IIRC

Thanks


[...]
diff mbox

Patch

diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c
index 75b14bce5b..39479d2389 100644
--- a/libavcodec/nuv.c
+++ b/libavcodec/nuv.c
@@ -42,6 +42,7 @@  typedef struct NuvContext {
     unsigned char *decomp_buf;
     uint32_t lq[64], cq[64];
     RTJpegContext rtj;
+    AVPacket flush_pkt;
 } NuvContext;
 
 static const uint8_t fallback_lquant[] = {
@@ -172,6 +173,20 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
         NUV_COPY_LAST     = 'L'
     } comptype;
 
+    if (!avpkt->data) {
+        if (avctx->internal->need_flush) {
+            avctx->internal->need_flush = 0;
+            ret = ff_setup_buffered_frame_for_return(avctx, data, c->pic, &c->flush_pkt);
+            if (ret < 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 +219,8 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
         }
         break;
     case NUV_COPY_LAST:
-        keyframe = 0;
-        break;
+        avctx->internal->need_flush = 1;
+        return buf_size;
     default:
         keyframe = 1;
         break;
@@ -313,6 +328,7 @@  retry:
     if ((result = av_frame_ref(picture, c->pic)) < 0)
         return result;
 
+    avctx->internal->need_flush = 0;
     *got_frame = 1;
     return orig_size;
 }
@@ -364,6 +380,7 @@  AVCodec ff_nuv_decoder = {
     .init           = decode_init,
     .close          = decode_end,
     .decode         = decode_frame,
-    .capabilities   = AV_CODEC_CAP_DR1,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
+    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS | FF_CODEC_CAP_SETS_PKT_POS |
+                      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