> >Feb 29, 2024, 06:35 by tong1.wu-at-intel....@ffmpeg.org: > >> From: Tong Wu <tong1...@intel.com> >> >> Since VAAPI and future D3D12VA implementation may share the same dpb >> logic and some other functions. A base layer encode context is introduced >> as vaapi context's base and extract the related funtions to a common >> file such as receive_packet, init, close, etc. >> >> Signed-off-by: Tong Wu <tong1...@intel.com> >> --- >> libavcodec/Makefile | 2 +- >> libavcodec/hw_base_encode.c | 644 ++++++++++++++++++++++ >> libavcodec/hw_base_encode.h | 252 +++++++++ >> libavcodec/vaapi_encode.c | 927 ++++++-------------------------- >> libavcodec/vaapi_encode.h | 202 +------ >> libavcodec/vaapi_encode_av1.c | 71 +-- >> libavcodec/vaapi_encode_h264.c | 199 +++---- >> libavcodec/vaapi_encode_h265.c | 161 +++--- >> libavcodec/vaapi_encode_mjpeg.c | 22 +- >> libavcodec/vaapi_encode_mpeg2.c | 51 +- >> libavcodec/vaapi_encode_vp8.c | 26 +- >> libavcodec/vaapi_encode_vp9.c | 68 +-- >> 12 files changed, 1400 insertions(+), 1225 deletions(-) >> create mode 100644 libavcodec/hw_base_encode.c >> create mode 100644 libavcodec/hw_base_encode.h >> >> +#define AVCODEC_HW_BASE_ENCODE_H >> + >> +#include "libavutil/hwcontext.h" >> +#include "libavutil/fifo.h" >> + >> +#include "avcodec.h" >> + >> +static inline const char *ff_hw_base_encode_get_pictype_name(const int >type) { >> + const char * const picture_type_name[] = { "IDR", "I", "P", "B" }; >> + return picture_type_name[type]; >> +} >> + >> +enum { >> + MAX_DPB_SIZE = 16, >> + MAX_PICTURE_REFERENCES = 2, >> > >Only two max refs per picture?
For current VAAPI implementation yes. > > >> + MAX_REORDER_DELAY = 16, >> + MAX_ASYNC_DEPTH = 64, >> + MAX_REFERENCE_LIST_NUM = 2, >> +}; >> > >Define these, don't enum them unnecessarily. Sure. > > >> +enum { >> + PICTURE_TYPE_IDR = 0, >> + PICTURE_TYPE_I = 1, >> + PICTURE_TYPE_P = 2, >> + PICTURE_TYPE_B = 3, >> +}; >> + >> +enum { >> + // Codec supports controlling the subdivision of pictures into slices. >> + FLAG_SLICE_CONTROL = 1 << 0, >> + // Codec only supports constant quality (no rate control). >> + FLAG_CONSTANT_QUALITY_ONLY = 1 << 1, >> + // Codec is intra-only. >> + FLAG_INTRA_ONLY = 1 << 2, >> + // Codec supports B-pictures. >> + FLAG_B_PICTURES = 1 << 3, >> + // Codec supports referencing B-pictures. >> + FLAG_B_PICTURE_REFERENCES = 1 << 4, >> + // Codec supports non-IDR key pictures (that is, key pictures do >> + // not necessarily empty the DPB). >> + FLAG_NON_IDR_KEY_PICTURES = 1 << 5, >> + // Codec output packet without timestamp delay, which means the >> + // output packet has same PTS and DTS. >> + FLAG_TIMESTAMP_NO_DELAY = 1 << 6, >> > >This is a low-level API. Concepts such as delay don't exist >if you're the one setting up references. I'll remove it from here and define this flag in vaapi_encode.h then. > > >> +}; >> + >> +typedef struct HWBaseEncodePicture { >> + struct HWBaseEncodePicture *next; >> + >> + int64_t display_order; >> + int64_t encode_order; >> + int64_t pts; >> + int64_t duration; >> + int force_idr; >> + >> + void *opaque; >> + AVBufferRef *opaque_ref; >> + >> + int type; >> + int b_depth; >> + int encode_issued; >> + int encode_complete; >> + >> + AVFrame *input_image; >> + AVFrame *recon_image; >> + >> + void *priv_data; >> + >> + // Whether this picture is a reference picture. >> + int is_reference; >> + >> + // The contents of the DPB after this picture has been decoded. >> + // This will contain the picture itself if it is a reference picture, >> + // but not if it isn't. >> + int nb_dpb_pics; >> + struct HWBaseEncodePicture *dpb[MAX_DPB_SIZE]; >> + // The reference pictures used in decoding this picture. If they are >> + // used by later pictures they will also appear in the DPB. ref[0][] for >> + // previous reference frames. ref[1][] for future reference frames. >> + int nb_refs[MAX_REFERENCE_LIST_NUM]; >> + struct HWBaseEncodePicture >*refs[MAX_REFERENCE_LIST_NUM][MAX_PICTURE_REFERENCES]; >> + // The previous reference picture in encode order. Must be in at least >> + // one of the reference list and DPB list. >> + struct HWBaseEncodePicture *prev; >> + // Reference count for other pictures referring to this one through >> + // the above pointers, directly from incomplete pictures and indirectly >> + // through completed pictures. >> + int ref_count[2]; >> + int ref_removed[2]; >> +} HWBaseEncodePicture; >> + >> +typedef struct HWEncodePictureOperation { >> + // Alloc memory for the picture structure. >> + HWBaseEncodePicture * (*alloc)(AVCodecContext *avctx, const AVFrame >*frame); >> + // Issue the picture structure, which will send the frame surface to HW >Encode API. >> + int (*issue)(AVCodecContext *avctx, const HWBaseEncodePicture >*base_pic); >> + // Get the output AVPacket. >> + int (*output)(AVCodecContext *avctx, const HWBaseEncodePicture >*base_pic, AVPacket *pkt); >> + // Free the picture structure. >> + int (*free)(AVCodecContext *avctx, HWBaseEncodePicture *base_pic); >> +} HWEncodePictureOperation; >> + >> +typedef struct HWBaseEncodeContext { >> + const AVClass *class; >> + >> + // Hardware-specific hooks. >> + const struct HWEncodePictureOperation *op; >> + >> + // Global options. >> + >> + // Number of I frames between IDR frames. >> + int idr_interval; >> + >> + // Desired B frame reference depth. >> + int desired_b_depth; >> + >> + // Explicitly set RC mode (otherwise attempt to pick from >> + // available modes). >> + int explicit_rc_mode; >> + >> + // Explicitly-set QP, for use with the "qp" options. >> + // (Forces CQP mode when set, overriding everything else.) >> + int explicit_qp; >> + >> + // The required size of surfaces. This is probably the input >> + // size (AVCodecContext.width|height) aligned up to whatever >> + // block size is required by the codec. >> + int surface_width; >> + int surface_height; >> + >> + // The block size for slice calculations. >> + int slice_block_width; >> + int slice_block_height; >> + >> + // RC quality level - meaning depends on codec and RC mode. >> + // In CQP mode this sets the fixed quantiser value. >> + int rc_quality; >> + >> + AVBufferRef *device_ref; >> + AVHWDeviceContext *device; >> + >> + // The hardware frame context containing the input frames. >> + AVBufferRef *input_frames_ref; >> + AVHWFramesContext *input_frames; >> + >> + // The hardware frame context containing the reconstructed frames. >> + AVBufferRef *recon_frames_ref; >> + AVHWFramesContext *recon_frames; >> + >> + // Current encoding window, in display (input) order. >> + HWBaseEncodePicture *pic_start, *pic_end; >> + // The next picture to use as the previous reference picture in >> + // encoding order. Order from small to large in encoding order. >> + HWBaseEncodePicture *next_prev[MAX_PICTURE_REFERENCES]; >> + int nb_next_prev; >> + >> + // Next input order index (display order). >> + int64_t input_order; >> + // Number of frames that output is behind input. >> + int64_t output_delay; >> + // Next encode order index. >> + int64_t encode_order; >> + // Number of frames decode output will need to be delayed. >> + int64_t decode_delay; >> + // Next output order index (in encode order). >> + int64_t output_order; >> + >> + // Timestamp handling. >> + int64_t first_pts; >> + int64_t dts_pts_diff; >> + int64_t ts_ring[MAX_REORDER_DELAY * 3 + >> + MAX_ASYNC_DEPTH]; >> + >> + // Frame type decision. >> + int gop_size; >> + int closed_gop; >> + int gop_per_idr; >> + int p_per_i; >> + int max_b_depth; >> + int b_per_p; >> + int force_idr; >> + int idr_counter; >> + int gop_counter; >> + int end_of_stream; >> + int p_to_gpb; >> + >> + // Whether the driver supports ROI at all. >> + int roi_allowed; >> + >> + // The encoder does not support cropping information, so warn about >> + // it the first time we encounter any nonzero crop fields. >> + int crop_warned; >> + // If the driver does not support ROI then warn the first time we >> + // encounter a frame with ROI side data. >> + int roi_warned; >> + >> + AVFrame *frame; >> + >> + // Whether the HW supports sync buffer function. >> + // If supported, encode_fifo/async_depth will be used together. >> + // Used for output buffer synchronization. >> + int async_encode; >> + >> + // Store buffered pic >> + AVFifo *encode_fifo; >> + // Max number of frame buffered in encoder. >> + int async_depth; >> + >> + /** Tail data of a pic, now only used for av1 repeat frame header. */ >> + AVPacket *tail_pkt; >> +} HWBaseEncodeContext; >> + >> +int ff_hw_base_encode_receive_packet(AVCodecContext *avctx, AVPacket >*pkt); >> + >> +int ff_hw_base_encode_init(AVCodecContext *avctx); >> + >> +int ff_hw_base_encode_close(AVCodecContext *avctx); >> + >> +#define HW_BASE_ENCODE_COMMON_OPTIONS \ >> + { "idr_interval", \ >> + "Distance (in I-frames) between IDR frames", \ >> + OFFSET(common.base.idr_interval), AV_OPT_TYPE_INT, \ >> > >Call them keyframes. Don't hardcode MPEGese. This part is just a copy from VAAPI. I can change the explanation from IDR frames to key frames. But if change the option name idr_interval it might bring compatibility issue. > > >> + { .i64 = 0 }, 0, INT_MAX, FLAGS }, \ >> + { "b_depth", \ >> + "Maximum B-frame reference depth", \ >> + OFFSET(common.base.desired_b_depth), AV_OPT_TYPE_INT, \ >> + { .i64 = 1 }, 1, INT_MAX, FLAGS }, \ >> + { "async_depth", "Maximum processing parallelism. " \ >> + "Increase this to improve single channel performance.", \ >> + OFFSET(common.base.async_depth), AV_OPT_TYPE_INT, \ >> + { .i64 = 2 }, 1, MAX_ASYNC_DEPTH, FLAGS } >> + >> +#endif /* AVCODEC_HW_BASE_ENCODE_H */ >> > >No namespacing of any structs or functions. I don't really understand. Could you explain a little more? >Split this into multiple commits rather than converting VAAPI in the same >commit. Actually I have split some common functions into the following 4 patches. I can split more in the next version. Thanks. > >This looks like a copy of VAAPI's logic, with all specific limitations >to VAAPI, but able to be wrapped on top of D3D12. I'd like to see something >more generic. >This doesn't look like it would be that useful for Vulkan as-is. Yes since D3D12 APIs are similar to VAAPI and are at the same level as VAAPI. That's why it's possible to share the common part and reduce code redundancy. The idea is just extract the common logic to a base part, especially DPB management logics. If Vulkan encoder in the future does not rely on that, which I'm not pretty sure, it's not necessary to be useful for Vulkan. We could just keep it only for VAAPI and D3D12. Thanks, Tong >_______________________________________________ >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".