| Message ID | 20240716171155.31838-37-anton@khirnov.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/39] tests/fate/vcodec: add vsynth tests for FFV1 version 2 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Anton Khirnov: > Instead of a single AVFrame. > > Will be useful in future commits, where we will want to produce multiple > output frames for a single coded frame. > --- > libavcodec/Makefile | 2 +- > libavcodec/hevc/hevcdec.c | 30 ++++++++++++++++++++---------- > libavcodec/hevc/hevcdec.h | 5 +++-- > libavcodec/hevc/refs.c | 11 +++-------- > 4 files changed, 27 insertions(+), 21 deletions(-) > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index 771e2b597e..e67b729bd3 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -431,7 +431,7 @@ OBJS-$(CONFIG_HCA_DECODER) += hcadec.o > OBJS-$(CONFIG_HCOM_DECODER) += hcom.o > OBJS-$(CONFIG_HDR_DECODER) += hdrdec.o > OBJS-$(CONFIG_HDR_ENCODER) += hdrenc.o > -OBJS-$(CONFIG_HEVC_DECODER) += aom_film_grain.o h274.o > +OBJS-$(CONFIG_HEVC_DECODER) += aom_film_grain.o h274.o container_fifo.o > OBJS-$(CONFIG_HEVC_AMF_ENCODER) += amfenc_hevc.o > OBJS-$(CONFIG_HEVC_CUVID_DECODER) += cuviddec.o > OBJS-$(CONFIG_HEVC_D3D12VA_ENCODER) += d3d12va_encode_hevc.o h265_profile_level.o \ > diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c > index fc87169a1a..5e28bfe54e 100644 > --- a/libavcodec/hevc/hevcdec.c > +++ b/libavcodec/hevc/hevcdec.c > @@ -3006,6 +3006,10 @@ static int hevc_frame_start(HEVCContext *s) > s->cur_frame->frame_grain->height = s->cur_frame->f->height; > if ((ret = ff_thread_get_buffer(s->avctx, s->cur_frame->frame_grain, 0)) < 0) > goto fail; > + > + ret = av_frame_copy_props(s->cur_frame->frame_grain, s->cur_frame->f); > + if (ret < 0) > + goto fail; > } > > s->cur_frame->f->pict_type = 3 - s->sh.slice_type; > @@ -3013,8 +3017,7 @@ static int hevc_frame_start(HEVCContext *s) > if (!IS_IRAP(s)) > ff_hevc_bump_frame(s); > > - av_frame_unref(s->output_frame); > - ret = ff_hevc_output_frame(s, s->output_frame, 0); > + ret = ff_hevc_output_frame(s, 0); > if (ret < 0) > goto fail; > > @@ -3411,13 +3414,16 @@ static int hevc_receive_frame(AVCodecContext *avctx, AVFrame *frame) > uint8_t *sd; > size_t sd_size; > > + if (ff_container_fifo_can_read(s->output_fifo)) > + goto do_output; > + > av_packet_unref(avpkt); > ret = ff_decode_get_packet(avctx, avpkt); > if (ret == AVERROR_EOF) { > - ret = ff_hevc_output_frame(s, frame, 1); > + ret = ff_hevc_output_frame(s, 1); > if (ret < 0) > return ret; > - return (ret > 0) ? 0 : AVERROR_EOF; > + goto do_output; > } else if (ret < 0) > return ret; > > @@ -3442,12 +3448,15 @@ static int hevc_receive_frame(AVCodecContext *avctx, AVFrame *frame) > if (ret < 0) > return ret; > > - if (s->output_frame->buf[0]) { > - av_frame_move_ref(frame, s->output_frame); > +do_output: > + if (ff_container_fifo_read(s->output_fifo, frame) >= 0) { This looks like it adds a (several) av_frame_move_ref() for the common case in which we only have one output frame. Can't this be avoided? > + if (!(avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) > + av_frame_remove_side_data(frame, AV_FRAME_DATA_FILM_GRAIN_PARAMS); > + > return 0; > } > > - return AVERROR(EAGAIN); > + return avci->draining ? AVERROR_EOF : AVERROR(EAGAIN); > } > > static int hevc_ref_frame(HEVCFrame *dst, const HEVCFrame *src) > @@ -3499,7 +3508,8 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx) > av_freep(&s->sao_pixel_buffer_h[i]); > av_freep(&s->sao_pixel_buffer_v[i]); > } > - av_frame_free(&s->output_frame); > + > + ff_container_fifo_free(&s->output_fifo); > > for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) { > ff_hevc_unref_frame(&s->DPB[i], ~0); > @@ -3537,8 +3547,8 @@ static av_cold int hevc_init_context(AVCodecContext *avctx) > s->local_ctx[0].logctx = avctx; > s->local_ctx[0].common_cabac_state = &s->cabac; > > - s->output_frame = av_frame_alloc(); > - if (!s->output_frame) > + s->output_fifo = ff_container_fifo_alloc_avframe(0); > + if (!s->output_fifo) > return AVERROR(ENOMEM); > > for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) { > diff --git a/libavcodec/hevc/hevcdec.h b/libavcodec/hevc/hevcdec.h > index da4d83e661..f2705b8de2 100644 > --- a/libavcodec/hevc/hevcdec.h > +++ b/libavcodec/hevc/hevcdec.h > @@ -31,6 +31,7 @@ > #include "libavcodec/avcodec.h" > #include "libavcodec/bswapdsp.h" > #include "libavcodec/cabac.h" > +#include "libavcodec/container_fifo.h" Unnecessary. Just use struct ContainerFifo* below. > #include "libavcodec/dovi_rpu.h" > #include "libavcodec/get_bits.h" > #include "libavcodec/h2645_parse.h" > @@ -457,7 +458,7 @@ typedef struct HEVCContext { > /** 1 if the independent slice segment header was successfully parsed */ > uint8_t slice_initialized; > > - AVFrame *output_frame; > + ContainerFifo *output_fifo; > uint8_t *sao_pixel_buffer_h[3]; > uint8_t *sao_pixel_buffer_v[3]; > > @@ -636,7 +637,7 @@ static av_always_inline int ff_hevc_nal_is_nonref(enum HEVCNALUnitType type) > * Find next frame in output order and put a reference to it in frame. > * @return 1 if a frame was output, 0 otherwise > */ > -int ff_hevc_output_frame(HEVCContext *s, AVFrame *frame, int flush); > +int ff_hevc_output_frame(HEVCContext *s, int flush); > > void ff_hevc_bump_frame(HEVCContext *s); > > diff --git a/libavcodec/hevc/refs.c b/libavcodec/hevc/refs.c > index 48fc4d27da..44ee7c837a 100644 > --- a/libavcodec/hevc/refs.c > +++ b/libavcodec/hevc/refs.c > @@ -171,7 +171,7 @@ static void unref_missing_refs(HEVCContext *s) > } > } > > -int ff_hevc_output_frame(HEVCContext *s, AVFrame *out, int flush) > +int ff_hevc_output_frame(HEVCContext *s, int flush) > { > if (IS_IRAP(s) && s->no_rasl_output_flag == 1) { > const static int mask = HEVC_FRAME_FLAG_BUMPING | HEVC_FRAME_FLAG_OUTPUT; > @@ -211,7 +211,8 @@ int ff_hevc_output_frame(HEVCContext *s, AVFrame *out, int flush) > if (nb_output) { > HEVCFrame *frame = &s->DPB[min_idx]; > > - ret = av_frame_ref(out, frame->needs_fg ? frame->frame_grain : frame->f); > + ret = ff_container_fifo_write(s->output_fifo, > + frame->needs_fg ? frame->frame_grain : frame->f); > if (frame->flags & HEVC_FRAME_FLAG_BUMPING) > ff_hevc_unref_frame(frame, HEVC_FRAME_FLAG_OUTPUT | HEVC_FRAME_FLAG_BUMPING); > else > @@ -219,12 +220,6 @@ int ff_hevc_output_frame(HEVCContext *s, AVFrame *out, int flush) > if (ret < 0) > return ret; > > - if (frame->needs_fg && (ret = av_frame_copy_props(out, frame->f)) < 0) > - return ret; > - > - if (!(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) > - av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS); > - > av_log(s->avctx, AV_LOG_DEBUG, > "Output frame with POC %d.\n", frame->poc); > return 1;
Quoting Andreas Rheinhardt (2024-08-10 01:52:36) > Anton Khirnov: > > @@ -3442,12 +3448,15 @@ static int hevc_receive_frame(AVCodecContext *avctx, AVFrame *frame) > > if (ret < 0) > > return ret; > > > > - if (s->output_frame->buf[0]) { > > - av_frame_move_ref(frame, s->output_frame); > > +do_output: > > + if (ff_container_fifo_read(s->output_fifo, frame) >= 0) { > > This looks like it adds a (several) av_frame_move_ref() for the common > case in which we only have one output frame. Can't this be avoided? Does it? Current code has av_frame_ref() into HEVCContext.output_frame, which is then av_frame_move_ref()'d into the actual output frame. New code does the same, except with a few more wrappers. > > diff --git a/libavcodec/hevc/hevcdec.h b/libavcodec/hevc/hevcdec.h > > index da4d83e661..f2705b8de2 100644 > > --- a/libavcodec/hevc/hevcdec.h > > +++ b/libavcodec/hevc/hevcdec.h > > @@ -31,6 +31,7 @@ > > #include "libavcodec/avcodec.h" > > #include "libavcodec/bswapdsp.h" > > #include "libavcodec/cabac.h" > > +#include "libavcodec/container_fifo.h" > > Unnecessary. Just use struct ContainerFifo* below. Sure, changed locally.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 771e2b597e..e67b729bd3 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -431,7 +431,7 @@ OBJS-$(CONFIG_HCA_DECODER) += hcadec.o OBJS-$(CONFIG_HCOM_DECODER) += hcom.o OBJS-$(CONFIG_HDR_DECODER) += hdrdec.o OBJS-$(CONFIG_HDR_ENCODER) += hdrenc.o -OBJS-$(CONFIG_HEVC_DECODER) += aom_film_grain.o h274.o +OBJS-$(CONFIG_HEVC_DECODER) += aom_film_grain.o h274.o container_fifo.o OBJS-$(CONFIG_HEVC_AMF_ENCODER) += amfenc_hevc.o OBJS-$(CONFIG_HEVC_CUVID_DECODER) += cuviddec.o OBJS-$(CONFIG_HEVC_D3D12VA_ENCODER) += d3d12va_encode_hevc.o h265_profile_level.o \ diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c index fc87169a1a..5e28bfe54e 100644 --- a/libavcodec/hevc/hevcdec.c +++ b/libavcodec/hevc/hevcdec.c @@ -3006,6 +3006,10 @@ static int hevc_frame_start(HEVCContext *s) s->cur_frame->frame_grain->height = s->cur_frame->f->height; if ((ret = ff_thread_get_buffer(s->avctx, s->cur_frame->frame_grain, 0)) < 0) goto fail; + + ret = av_frame_copy_props(s->cur_frame->frame_grain, s->cur_frame->f); + if (ret < 0) + goto fail; } s->cur_frame->f->pict_type = 3 - s->sh.slice_type; @@ -3013,8 +3017,7 @@ static int hevc_frame_start(HEVCContext *s) if (!IS_IRAP(s)) ff_hevc_bump_frame(s); - av_frame_unref(s->output_frame); - ret = ff_hevc_output_frame(s, s->output_frame, 0); + ret = ff_hevc_output_frame(s, 0); if (ret < 0) goto fail; @@ -3411,13 +3414,16 @@ static int hevc_receive_frame(AVCodecContext *avctx, AVFrame *frame) uint8_t *sd; size_t sd_size; + if (ff_container_fifo_can_read(s->output_fifo)) + goto do_output; + av_packet_unref(avpkt); ret = ff_decode_get_packet(avctx, avpkt); if (ret == AVERROR_EOF) { - ret = ff_hevc_output_frame(s, frame, 1); + ret = ff_hevc_output_frame(s, 1); if (ret < 0) return ret; - return (ret > 0) ? 0 : AVERROR_EOF; + goto do_output; } else if (ret < 0) return ret; @@ -3442,12 +3448,15 @@ static int hevc_receive_frame(AVCodecContext *avctx, AVFrame *frame) if (ret < 0) return ret; - if (s->output_frame->buf[0]) { - av_frame_move_ref(frame, s->output_frame); +do_output: + if (ff_container_fifo_read(s->output_fifo, frame) >= 0) { + if (!(avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) + av_frame_remove_side_data(frame, AV_FRAME_DATA_FILM_GRAIN_PARAMS); + return 0; } - return AVERROR(EAGAIN); + return avci->draining ? AVERROR_EOF : AVERROR(EAGAIN); } static int hevc_ref_frame(HEVCFrame *dst, const HEVCFrame *src) @@ -3499,7 +3508,8 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx) av_freep(&s->sao_pixel_buffer_h[i]); av_freep(&s->sao_pixel_buffer_v[i]); } - av_frame_free(&s->output_frame); + + ff_container_fifo_free(&s->output_fifo); for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) { ff_hevc_unref_frame(&s->DPB[i], ~0); @@ -3537,8 +3547,8 @@ static av_cold int hevc_init_context(AVCodecContext *avctx) s->local_ctx[0].logctx = avctx; s->local_ctx[0].common_cabac_state = &s->cabac; - s->output_frame = av_frame_alloc(); - if (!s->output_frame) + s->output_fifo = ff_container_fifo_alloc_avframe(0); + if (!s->output_fifo) return AVERROR(ENOMEM); for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) { diff --git a/libavcodec/hevc/hevcdec.h b/libavcodec/hevc/hevcdec.h index da4d83e661..f2705b8de2 100644 --- a/libavcodec/hevc/hevcdec.h +++ b/libavcodec/hevc/hevcdec.h @@ -31,6 +31,7 @@ #include "libavcodec/avcodec.h" #include "libavcodec/bswapdsp.h" #include "libavcodec/cabac.h" +#include "libavcodec/container_fifo.h" #include "libavcodec/dovi_rpu.h" #include "libavcodec/get_bits.h" #include "libavcodec/h2645_parse.h" @@ -457,7 +458,7 @@ typedef struct HEVCContext { /** 1 if the independent slice segment header was successfully parsed */ uint8_t slice_initialized; - AVFrame *output_frame; + ContainerFifo *output_fifo; uint8_t *sao_pixel_buffer_h[3]; uint8_t *sao_pixel_buffer_v[3]; @@ -636,7 +637,7 @@ static av_always_inline int ff_hevc_nal_is_nonref(enum HEVCNALUnitType type) * Find next frame in output order and put a reference to it in frame. * @return 1 if a frame was output, 0 otherwise */ -int ff_hevc_output_frame(HEVCContext *s, AVFrame *frame, int flush); +int ff_hevc_output_frame(HEVCContext *s, int flush); void ff_hevc_bump_frame(HEVCContext *s); diff --git a/libavcodec/hevc/refs.c b/libavcodec/hevc/refs.c index 48fc4d27da..44ee7c837a 100644 --- a/libavcodec/hevc/refs.c +++ b/libavcodec/hevc/refs.c @@ -171,7 +171,7 @@ static void unref_missing_refs(HEVCContext *s) } } -int ff_hevc_output_frame(HEVCContext *s, AVFrame *out, int flush) +int ff_hevc_output_frame(HEVCContext *s, int flush) { if (IS_IRAP(s) && s->no_rasl_output_flag == 1) { const static int mask = HEVC_FRAME_FLAG_BUMPING | HEVC_FRAME_FLAG_OUTPUT; @@ -211,7 +211,8 @@ int ff_hevc_output_frame(HEVCContext *s, AVFrame *out, int flush) if (nb_output) { HEVCFrame *frame = &s->DPB[min_idx]; - ret = av_frame_ref(out, frame->needs_fg ? frame->frame_grain : frame->f); + ret = ff_container_fifo_write(s->output_fifo, + frame->needs_fg ? frame->frame_grain : frame->f); if (frame->flags & HEVC_FRAME_FLAG_BUMPING) ff_hevc_unref_frame(frame, HEVC_FRAME_FLAG_OUTPUT | HEVC_FRAME_FLAG_BUMPING); else @@ -219,12 +220,6 @@ int ff_hevc_output_frame(HEVCContext *s, AVFrame *out, int flush) if (ret < 0) return ret; - if (frame->needs_fg && (ret = av_frame_copy_props(out, frame->f)) < 0) - return ret; - - if (!(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN)) - av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS); - av_log(s->avctx, AV_LOG_DEBUG, "Output frame with POC %d.\n", frame->poc); return 1;