[FFmpeg-devel] [PATCH] libavutil/video_enc_params: add block type

[Mark Thompson]

On 15/07/2020 18:43, Yongle Lin wrote:
> add block type field to AVVideoBlockParams so we could either export or visualize it later.
> ---
>   libavutil/video_enc_params.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/libavutil/video_enc_params.h b/libavutil/video_enc_params.h
> index 43fa443154..8bf5f240c9 100644
> --- a/libavutil/video_enc_params.h
> +++ b/libavutil/video_enc_params.h
> @@ -57,6 +57,11 @@ enum AVVideoEncParamsType {
>       AV_VIDEO_ENC_PARAMS_H264,
>   };
>   
> +enum AVVideoBlockFlags {
> +    AV_VIDEO_ENC_BLOCK_INTRA = 1ULL <<  0,  /* Indicates block uses intra prediction */

The ULL is only confusing matters here - standard-conforming enum values have type int.  Some compilers allow it to be a larger integer type, but I don't think we can rely on that extension.

> +    AV_VIDEO_ENC_BLOCK_SKIP = 1ULL <<  1,   /* Indicates block is not coded (skipped) */

Can you define more precisely what you mean by "not coded"?  Is that just that it has no residuals, or does it also indicate no motion vector, or no coded motion vector relative to prediction?

> +};
> +
>   /**
>    * Video encoding parameters for a given frame. This struct is allocated along
>    * with an optional array of per-block AVVideoBlockParams descriptors.
> @@ -126,6 +131,20 @@ typedef struct AVVideoBlockParams {
>        * corresponding per-frame value.
>        */
>       int32_t delta_qp;
> +
> +    /**
> +     * Type flag of the block
> +     * Each bit field indicates a type flag
> +     */
> +    enum AVVideoBlockFlags flags;

Don't make this an enum, since you aren't using it as an enum - you're going to assign combinations of flags.  (Also, the size of the field may change as more flags are added.)

> +
> +    /**
> +     * Reference frames used for prediction
> +     * Each entry specifies the first/second/third/etc. reference frame the current frame uses.
> +     * The value at each entry specifies the index inside the reference frame array for that current frame.

You'll need to define more carefully what "the reference frame array" means.  I can guess that it's the ref_frame_idx[] number for VP9, but it's not at all obvious what it would mean for H.264.

> +     * Any entry that is unused will be set to -1
> +     */
> +    int8_t ref[8];
>   } AVVideoBlockParams;
>   
>   /*
> 

- Mark