[FFmpeg-devel,31/92] Vulkan patchset part 1 - common code changes

The attached patchset is all the common code changes that my Vulkan patchset needs.

In total lines of code, this part has 425 additions and 131 deletions.
Most of that is additions to HEVC parsing. Excluding them, the patchset is
200 lines of code added, which is manageable.

Apart from the parser changes, the following other changes have been
made to the API:

AVHWAccel.free_frame_priv exists due to Vulkan's way of using VkImageView
objects to wrap VkImage objects, which we need to free once they're no longer
in use. Every other API uses the direct objects in decoding, but with Vulkan,
they have to be represented by other objects.
We also use it to free the slice offsets buffer.

AVHWAccel.flush exists due to Vulkan keeping decoder state, despite being
stateless in theory. The decoder has to be notified of flushes in order to reset
decoding slots and other data it needs, such as motion vectors and reference
lists for AV1. Otherwise, inferring whether a flush has happened can be codec
dependent, and hacky.

hwaccel_params_buf exists due to Vulkan's way of compiling SPS/PPS data
into objects, making updating expensive. The change allows for hardware
to only upload new parameters if they have been changed.
It's insignificant for H264 and AV1, but HEVC's structures can reach 114
megabytes of data that has to be uploaded, for a specially crafted input,
which is enough to DDOS an ingest.
The data is set and managed by the hwaccel, but does need to be synchronized
between different decoding threads, which this patch performs.

Finally, the HWACCEL_CAP_THREAD_SAFE flag is added due to Vulkan being
actually threadsafe, and requiring no serialization. It does work and it
does actually make a difference, on average, it can increase performance
by 20% for an average B-frame using HEVC stream, depending on the
number of threads and the number of decode queues.
While hardware decoders are fast in general, certain vendors such as AMD
can choke up while playing 8k video, and this patch can significantly help
increase throughput.

In context, the changes can be viewed here:
https://github.com/cyanreg/FFmpeg/tree/vulkan

The rest of the whole patchset is either rewrites, filter code, or
the actual hardware accel code.

The patchset will not be pushed standalone, but as part of the greater
Vulkan patchset.

31 patches attached.

On 3/14/2023 3:33 AM, Lynne wrote:
> The attached patchset is all the common code changes that my Vulkan patchset needs.
> 
> In total lines of code, this part has 425 additions and 131 deletions.
> Most of that is additions to HEVC parsing. Excluding them, the patchset is
> 200 lines of code added, which is manageable.
> 
> Apart from the parser changes, the following other changes have been
> made to the API:
> 
> AVHWAccel.free_frame_priv exists due to Vulkan's way of using VkImageView
> objects to wrap VkImage objects, which we need to free once they're no longer
> in use. Every other API uses the direct objects in decoding, but with Vulkan,
> they have to be represented by other objects.
> We also use it to free the slice offsets buffer.
> 
> AVHWAccel.flush exists due to Vulkan keeping decoder state, despite being
> stateless in theory. The decoder has to be notified of flushes in order to reset
> decoding slots and other data it needs, such as motion vectors and reference
> lists for AV1. Otherwise, inferring whether a flush has happened can be codec
> dependent, and hacky.
> 
> hwaccel_params_buf exists due to Vulkan's way of compiling SPS/PPS data
> into objects, making updating expensive. The change allows for hardware
> to only upload new parameters if they have been changed.
> It's insignificant for H264 and AV1, but HEVC's structures can reach 114
> megabytes of data that has to be uploaded, for a specially crafted input,
> which is enough to DDOS an ingest.
> The data is set and managed by the hwaccel, but does need to be synchronized
> between different decoding threads, which this patch performs.
> 
> Finally, the HWACCEL_CAP_THREAD_SAFE flag is added due to Vulkan being
> actually threadsafe, and requiring no serialization. It does work and it
> does actually make a difference, on average, it can increase performance
> by 20% for an average B-frame using HEVC stream, depending on the
> number of threads and the number of decode queues.
> While hardware decoders are fast in general, certain vendors such as AMD
> can choke up while playing 8k video, and this patch can significantly help
> increase throughput.
> 
> In context, the changes can be viewed here:
> https://github.com/cyanreg/FFmpeg/tree/vulkan
> 
> The rest of the whole patchset is either rewrites, filter code, or
> the actual hardware accel code.
> 
> The patchset will not be pushed standalone, but as part of the greater
> Vulkan patchset.
> 
> 31 patches attached.

[...]

> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index a80e37e33f..5a3c51e94a 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -591,6 +591,8 @@ static void av1_frame_unref(AVCodecContext *avctx, AV1Frame *f)
>      f->spatial_id = f->temporal_id = 0;
>      memset(f->skip_mode_frame_idx, 0,
>             2 * sizeof(uint8_t));
> +    memset(f->ref_order_hint, 0,
> +           7 * sizeof(uint8_t));
>      memset(&f->film_grain, 0, sizeof(f->film_grain));
>      f->coded_lossless = 0;
>  }
> @@ -633,6 +635,9 @@ static int av1_frame_ref(AVCodecContext *avctx, AV1Frame *dst, const AV1Frame *s
>      memcpy(dst->skip_mode_frame_idx,
>             src->skip_mode_frame_idx,
>             2 * sizeof(uint8_t));
> +    memcpy(dst->ref_order_hint,
> +           src->ref_order_hint,
> +           7 * sizeof(uint8_t));
>      memcpy(&dst->film_grain,
>             &src->film_grain,
>             sizeof(dst->film_grain));
> @@ -1267,6 +1272,10 @@ static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>              s->cur_frame.spatial_id  = header->spatial_id;
>              s->cur_frame.temporal_id = header->temporal_id;
>  
> +            for (int i = 0; i < 7; i++)
> +                s->cur_frame.ref_order_hint[i] =
> +                s->raw_frame_header->ref_order_hint[s->raw_frame_header->ref_frame_idx[i]];

Why do you need this in cur_frame? It's not a derived value, and the 
AV1RawFrameHeader struct is accessible in all AVHWaccel callbacks.

And i think you should be looking at 
s->ref[s->raw_frame_header->ref_frame_idx[i]].raw_frame_header->order_hint 
instead, too, which is the decoder state vs the raw values in the 
current frame header (Although they should match in theory).

> +
>              if (avctx->hwaccel && s->cur_frame.f->buf[0]) {
>                  ret = avctx->hwaccel->start_frame(avctx, unit->data,

Mar 14, 2023, 12:51 by jamrial@gmail.com:

> On 3/14/2023 3:33 AM, Lynne wrote:
>
>>  +            for (int i = 0; i < 7; i++)
>> +                s->cur_frame.ref_order_hint[i] =
>> +                s->raw_frame_header->ref_order_hint[s->raw_frame_header->ref_frame_idx[i]];
>>
>
> Why do you need this in cur_frame? It's not a derived value, and the AV1RawFrameHeader struct is accessible in all AVHWaccel callbacks.
>
> And i think you should be looking at s->ref[s->raw_frame_header->ref_frame_idx[i]].raw_frame_header->order_hint instead, too, which is the decoder state vs the raw values in the current frame header (Although they should match in theory).
>
>> +
>>  if (avctx->hwaccel && s->cur_frame.f->buf[0]) {
>>  ret = avctx->hwaccel->start_frame(avctx, unit->data,
>>

Fixed, removed the commit altogether and just used the value directly.
Thanks for the review