On Tue, May 7, 2024 at 10:01 PM Mark Thompson <s...@jkqxz.net> wrote: > > On 28/04/2024 08:26, David Rosca wrote: > > When there are multiple tiles in one slice buffer, use multiple slice > > params to avoid sending the same slice buffer multiple times and thus > > increasing the bitstream size the driver will need to upload to hw. > > --- > > libavcodec/vaapi_av1.c | 37 +++++++++++++++++++++++-------------- > > 1 file changed, 23 insertions(+), 14 deletions(-) > > Can you confirm that this works on at least iHD (Intel) and Mesa (AMD)? (I > don't expect any issue, but it's good to check in case of something strange > going on matching up to what this was previously doing.)
I've tested with Mesa and it works correctly. Gstreamer also does the same. https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/da35ed69164ba8c8599d337b00ba54074215f7e7/subprojects/gst-plugins-bad/sys/va/gstvaav1dec.c#L891-L912 > > > diff --git a/libavcodec/vaapi_av1.c b/libavcodec/vaapi_av1.c > > index 4a90db1e09..567f505fbd 100644 > > --- a/libavcodec/vaapi_av1.c > > +++ b/libavcodec/vaapi_av1.c > > @@ -19,6 +19,7 @@ > > */ > > > > #include "libavutil/frame.h" > > +#include "libavutil/mem.h" > > #include "hwaccel_internal.h" > > #include "vaapi_decode.h" > > #include "internal.h" > > @@ -393,13 +394,17 @@ static int vaapi_av1_decode_slice(AVCodecContext > > *avctx, > > { > > const AV1DecContext *s = avctx->priv_data; > > VAAPIDecodePicture *pic = s->cur_frame.hwaccel_picture_private; > > - VASliceParameterBufferAV1 slice_param; > > - int err = 0; > > + VASliceParameterBufferAV1 *slice_params; > > + int err = 0, nb_params = 0; > > Remove the spurious initialisation on err? Done in v2. > > > > > - for (int i = s->tg_start; i <= s->tg_end; i++) { > > - memset(&slice_param, 0, sizeof(VASliceParameterBufferAV1)); > > + slice_params = av_calloc(s->tg_end - s->tg_start + 1, > > sizeof(*slice_params)); > > I suggest allocating this into VAAPIAV1DecContext to avoid the alloc/free on > every call. (Only reallocate if it needs to be bigger than the previous > maximum.) Done in v2. Thanks, David > > > + if (!slice_params) { > > + err = AVERROR(ENOMEM); > > + goto fail; > > + } > > > > - slice_param = (VASliceParameterBufferAV1) { > > + for (int i = s->tg_start; i <= s->tg_end; i++) { > > + slice_params[nb_params++] = (VASliceParameterBufferAV1) { > > .slice_data_size = s->tile_group_info[i].tile_size, > > .slice_data_offset = s->tile_group_info[i].tile_offset, > > .slice_data_flag = VA_SLICE_DATA_FLAG_ALL, > > @@ -408,18 +413,22 @@ static int vaapi_av1_decode_slice(AVCodecContext > > *avctx, > > .tg_start = s->tg_start, > > .tg_end = s->tg_end, > > }; > > - > > - err = ff_vaapi_decode_make_slice_buffer(avctx, pic, &slice_param, > > 1, > > - > > sizeof(VASliceParameterBufferAV1), > > - buffer, > > - size); > > - if (err) { > > - ff_vaapi_decode_cancel(avctx, pic); > > - return err; > > - } > > } > > > > + err = ff_vaapi_decode_make_slice_buffer(avctx, pic, slice_params, > > nb_params, > > + > > sizeof(VASliceParameterBufferAV1), > > + buffer, > > + size); > > + av_free(slice_params); > > + > > + if (err) > > + goto fail; > > + > > return 0; > > + > > +fail: > > + ff_vaapi_decode_cancel(avctx, pic); > > + return err; > > } > > > > const FFHWAccel ff_av1_vaapi_hwaccel = { > > It's amusing that this quadratic behaviour was around for so long! (I guess > people don't use many tiles.) > > Thanks, > > - Mark > _______________________________________________ > 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".