Re: [FFmpeg-devel] [PATCH 31/92] Vulkan patchset part 1 - common code changes

From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 31/92] Vulkan patchset part 1 - common code changes
Date: Sat, 22 Apr 2023 14:38:02 +0200
Message-ID: <168216708203.3843.7636239663100197135@lain.khirnov.net> (raw)
In-Reply-To: <NTOyxSn--3-9@lynne.ee>

Quoting Lynne (2023-04-19 16:47:11)
> Apr 17, 2023, 11:07 by anton@khirnov.net:
> 
> > Quoting Anton Khirnov (2023-04-16 22:38:16)
> >
> >> Quoting Lynne (2023-03-14 07:33:43)
> >> > From 61f412eea3fbcb1e2a8625796760c0e24fa3fb83 Mon Sep 17 00:00:00 2001
> >> > From: Lynne <dev@lynne.ee>
> >> > Date: Tue, 17 Jan 2023 05:01:45 +0100
> >> > Subject: [PATCH 27/92] h264dec: add hwaccel_params_buf
> >> > 
> >> > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> >> > index 0aee724c0d..6559593195 100644
> >> > --- a/libavcodec/h264dec.c
> >> > +++ b/libavcodec/h264dec.c
> >> > @@ -341,6 +341,7 @@ static av_cold int h264_decode_end(AVCodecContext *avctx)
> >> >      H264Context *h = avctx->priv_data;
> >> >      int i;
> >> > 
> >> > +    av_buffer_unref(&h->hwaccel_params_buf);
> >> >      ff_h264_remove_all_refs(h);
> >> >      ff_h264_free_tables(h);
> >> > 
> >> > @@ -470,6 +471,7 @@ static void h264_decode_flush(AVCodecContext *avctx)
> >> > 
> >> >      ff_h264_flush_change(h);
> >> >      ff_h264_sei_uninit(&h->sei);
> >> > +    av_buffer_unref(&h->hwaccel_params_buf);
> >> > 
> >> >      for (i = 0; i < H264_MAX_PICTURE_COUNT; i++)
> >> >          ff_h264_unref_picture(h, &h->DPB[i]);
> >> > @@ -669,6 +671,7 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
> >> >              avpriv_request_sample(avctx, "data partitioning");
> >> >              break;
> >> >          case H264_NAL_SEI:
> >> > +            av_buffer_unref(&h->hwaccel_params_buf);
> >> >              if (h->setup_finished) {
> >> >                  avpriv_request_sample(avctx, "Late SEI");
> >> >                  break;
> >> > @@ -682,6 +685,7 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
> >> >              break;
> >> >          case H264_NAL_SPS: {
> >> >              GetBitContext tmp_gb = nal->gb;
> >> > +            av_buffer_unref(&h->hwaccel_params_buf);
> >> >              if (avctx->hwaccel && avctx->hwaccel->decode_params) {
> >> >                  ret = avctx->hwaccel->decode_params(avctx,
> >> >                                                      nal->type,
> >>
> >> Removed on SEI and SPS? That seems bizarre and highly arbitrary. Why
> >> specifically those two?
> >>
> >
> > Found the explanation in the cover letter - I assume SEI is a mistake
> > and should really be PPS. The points where it is reset still seem highly
> > Vulkan-specific, so I think it'd be better for the decoder to set a flag
> > on seeing new parameter sets. Vulkan could would then check and clear
> > the flag as needed.
> >
> 
> It was a typo, changed to remove it on SPS and PPS.
> We still have to store the compiled bufferref somewhere, and synchronize
> it between frames, and having it in the frame context itself solves
> those issues.

Except the way it is handled makes no sense for anything other than
vulkan.

Having per-thread hwaccel contexts with a callback for propagating the
data would solve this more cleanly.

-- 
Anton Khirnov
_______________________________________________
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".