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".

Reply via email to