On 21/01/2024 14:18, jianfeng.zheng wrote:
see https://github.com/intel/libva/pull/738
Signed-off-by: jianfeng.zheng <jianfeng.zh...@mthreads.com>
---
configure | 14 ++++
libavcodec/Makefile | 1 +
libavcodec/cavs.h | 4 +
libavcodec/cavsdec.c | 101 +++++++++++++++++++++--
libavcodec/hwaccels.h | 1 +
libavcodec/vaapi_cavs.c | 164 ++++++++++++++++++++++++++++++++++++++
libavcodec/vaapi_decode.c | 4 +
7 files changed, 284 insertions(+), 5 deletions(-)
create mode 100644 libavcodec/vaapi_cavs.c
I suggest splitting this patch into two: add hwaccel hooks to AVS decoder, then
add VAAPI implementation for it.
diff --git a/configure b/configure
index c8ae0a061d..89759eda5d 100755
--- a/configure
+++ b/configure
...
@@ -7175,6 +7177,18 @@ if enabled vaapi; then
check_type "va/va.h va/va_enc_vp8.h" "VAEncPictureParameterBufferVP8"
check_type "va/va.h va/va_enc_vp9.h" "VAEncPictureParameterBufferVP9"
check_type "va/va.h va/va_enc_av1.h" "VAEncPictureParameterBufferAV1"
+
+ #
+ # Using 'VA_CHECK_VERSION' in source codes make things easy. But we have
to wait
+ # until newly added VAProfile being distributed by VAAPI released version.
+ #
+ # Before or after that, we can use auto-detection to keep version
compatibility.
+ # It always works.
+ #
+ disable va_profile_avs &&
+ test_code cc va/va.h "VAProfile p1 = VAProfileAVSJizhun, p2 =
VAProfileAVSGuangdian;" &&
+ enable va_profile_avs
+ enabled va_profile_avs && check_type "va/va.h va/va_dec_avs.h"
"VAPictureParameterBufferAVS"
fi
Why can't this be done with check_type for the decode structure like the other
VAAPI codecs?
(I don't think this should be applied to ffmpeg mainline until finalised in
libva to avoid any incompatibility, but that doesn't require a released version
of libva.)
...
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 5036ef50f7..5ca021c098 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -25,11 +25,14 @@
* @author Stefan Gehrer <stefan.geh...@gmx.de>
*/
+#include "config_components.h"
#include "libavutil/avassert.h"
#include "libavutil/emms.h"
#include "avcodec.h"
#include "get_bits.h"
#include "golomb.h"
+#include "hwaccel_internal.h"
+#include "hwconfig.h"
#include "profiles.h"
#include "cavs.h"
#include "codec_internal.h"
@@ -1002,9 +1005,9 @@ static inline int decode_slice_header(AVSContext *h,
GetBitContext *gb)
}
h->mb_weight_pred_flag = get_bits1(gb);
if (!h->avctx->hwaccel) {
- av_log(h->avctx, AV_LOG_ERROR,
- "weighted prediction not yet supported\n");
- }
+ av_log(h->avctx, AV_LOG_ERROR,
+ "weighted prediction not yet supported\n");
+ }
Unrelated whitespace change?
}
}
if (h->aec_flag) {
@@ -1115,6 +1118,46 @@ static inline int check_for_slice(AVSContext *h)
* frame level
*
****************************************************************************/
+static int hwaccel_pic(AVSContext *h)
+{
+ int ret = 0;
+ int stc = -1;
+ const uint8_t *frm_start = align_get_bits(&h->gb);
+ const uint8_t *frm_end = h->gb.buffer_end;
+ const uint8_t *slc_start = frm_start;
+ const uint8_t *slc_end = frm_end;
+ GetBitContext gb = h->gb;
+ const FFHWAccel *hwaccel = ffhwaccel(h->avctx->hwaccel);
+
+ ret = hwaccel->start_frame(h->avctx, NULL, 0);
+ if (ret < 0)
+ return ret;
+
+ for (slc_start = frm_start; slc_start + 4 < frm_end; slc_start = slc_end) {
+ slc_end = avpriv_find_start_code(slc_start + 4, frm_end, &stc);
+ if (slc_end < frm_end) {
+ slc_end -= 4;
+ }
+
+ init_get_bits(&h->gb, slc_start, (slc_end - slc_start) * 8);
init_get_bits8 avoids the unlikely-but-possible overflow case here.
+ if (!check_for_slice(h)) {
+ break;
+ }
+
+ ret = hwaccel->decode_slice(h->avctx, slc_start, slc_end - slc_start);
+ if (ret < 0) {
+ break;
+ }
+ }
+
+ h->gb = gb;
+ skip_bits(&h->gb, (slc_start - frm_start) * 8);
This is skipping to the start of the error in error cases? Is that intended?
+
+ if (ret < 0)
+ return ret;
+
+ return hwaccel->end_frame(h->avctx);
Are there any bad consequences if you call this with zero slices in the frame,
since the loop above appears to allow that? (If yes, maybe only call
start_frame once you know you have some slices.)
+}
/**
* @brief remove frame out of dpb
@@ -1125,6 +1168,9 @@ static void cavs_frame_unref(AVSFrame *frame)
if (!frame->f || !frame->f->buf[0])
return;
+ av_buffer_unref(&frame->hwaccel_priv_buf);
+ frame->hwaccel_picture_private = NULL;
+
av_frame_unref(frame->f);
}
@@ -1219,6 +1265,17 @@ static int decode_pic(AVSContext *h)
if (ret < 0)
return ret;
+ if (h->avctx->hwaccel) {
+ const FFHWAccel *hwaccel = ffhwaccel(h->avctx->hwaccel);
+ av_assert0(!h->cur.hwaccel_picture_private);
+ if (hwaccel->frame_priv_data_size) {
+ h->cur.hwaccel_priv_buf =
av_buffer_allocz(hwaccel->frame_priv_data_size);
+ if (!h->cur.hwaccel_priv_buf)
+ return AVERROR(ENOMEM);
+ h->cur.hwaccel_picture_private = h->cur.hwaccel_priv_buf->data;
+ }
+ }
+
if (!h->edge_emu_buffer) {
int alloc_size = FFALIGN(FFABS(h->cur.f->linesize[0]) + 32, 32);
h->edge_emu_buffer = av_mallocz(alloc_size * 2 * 24);
@@ -1247,7 +1304,9 @@ static int decode_pic(AVSContext *h)
av_log(h->avctx, AV_LOG_ERROR, "poc=%d/%d/%d, dist=%d/%d\n",
h->DPB[1].poc, h->DPB[0].poc, h->cur.poc, h->dist[0],
h->dist[1]);
av_log(h->avctx, AV_LOG_ERROR, "sym_factor %d too large\n",
h->sym_factor);
- return AVERROR_INVALIDDATA;
+
+ if (!h->avctx->hwaccel)
+ return AVERROR_INVALIDDATA;
Why is this not an error in hw cases?
}
} else {
h->direct_den[0] = h->dist[0] ? 16384 / h->dist[0] : 0;
@@ -1332,7 +1391,9 @@ static int decode_pic(AVSContext *h)
}
ret = 0;
- if (h->cur.f->pict_type == AV_PICTURE_TYPE_I) {
+ if (h->avctx->hwaccel) {
+ ret = hwaccel_pic(h);
+ } else if (h->cur.f->pict_type == AV_PICTURE_TYPE_I) {
do {
check_for_slice(h);
ret = decode_mb_i(h, 0);
@@ -1503,6 +1564,20 @@ static int cavs_decode_frame(AVCodecContext *avctx,
AVFrame *rframe,
return ret;
avctx->profile = h->profile;
avctx->level = h->level;
+ if (!h->got_pix_fmt) {
+ h->got_pix_fmt = 1;
+ ret = ff_get_format(avctx, avctx->codec->pix_fmts);
+ if (ret < 0)
+ return ret;
+
+ avctx->pix_fmt = ret;
+
+ if (h->profile == AV_PROFILE_CAVS_GUANGDIAN &&
!avctx->hwaccel) {
+ av_log(avctx, AV_LOG_ERROR, "Your platform doesn't suppport
hardware"
+ " accelerated for CAVS Guangdian Profile
decoding.\n");
This message will also appear in a confusing way in software-only cases with no
hardware at all. Can it be clearer that it's really a software feature that's
missing?
+ return AVERROR(ENOTSUP);
+ }
+ }
break;
case PIC_I_START_CODE:
if (!h->got_keyframe) {
@@ -1577,6 +1652,14 @@ static int cavs_decode_frame(AVCodecContext *avctx,
AVFrame *rframe,
return (buf_ptr - avpkt->data);
}
+static const enum AVPixelFormat cavs_hwaccel_pixfmt_list_420[] = {
+#if CONFIG_CAVS_VAAPI_HWACCEL
+ AV_PIX_FMT_VAAPI,
+#endif
+ AV_PIX_FMT_YUV420P,
+ AV_PIX_FMT_NONE
+};
+
const FFCodec ff_cavs_decoder = {
.p.name = "cavs",
CODEC_LONG_NAME("Chinese AVS (Audio Video Standard) (AVS1-P2, JiZhun
profile)"),
This description probably isn't right any more!
@@ -1589,4 +1672,12 @@ const FFCodec ff_cavs_decoder = {
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
.flush = cavs_flush,
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
+ .p.pix_fmts = cavs_hwaccel_pixfmt_list_420,
+ .hw_configs = (const AVCodecHWConfigInternal *const []) {
+#if CONFIG_CAVS_VAAPI_HWACCEL
+ HWACCEL_VAAPI(cavs),
+#endif
+ NULL
+ },
+ .p.profiles = NULL_IF_CONFIG_SMALL(ff_cavs_profiles),
};
diff --git a/libavcodec/hwaccels.h b/libavcodec/hwaccels.h
index 5171e4c7d7..a1a973b460 100644
--- a/libavcodec/hwaccels.h
+++ b/libavcodec/hwaccels.h
@@ -89,5 +89,6 @@ extern const struct FFHWAccel ff_wmv3_dxva2_hwaccel;
extern const struct FFHWAccel ff_wmv3_nvdec_hwaccel;
extern const struct FFHWAccel ff_wmv3_vaapi_hwaccel;
extern const struct FFHWAccel ff_wmv3_vdpau_hwaccel;
+extern const struct FFHWAccel ff_cavs_vaapi_hwaccel;
#endif /* AVCODEC_HWACCELS_H */
diff --git a/libavcodec/vaapi_cavs.c b/libavcodec/vaapi_cavs.c
new file mode 100644
index 0000000000..4a7a9b95ad
--- /dev/null
+++ b/libavcodec/vaapi_cavs.c
@@ -0,0 +1,164 @@
+/*
+ * AVS (Chinese GY/T 257.1—2012) HW decode acceleration through VA-API
+ * Copyright (c) 2022 JianfengZheng <jianfeng.zh...@mthreads.com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "hwconfig.h"
+#include "hwaccel_internal.h"
+#include "vaapi_decode.h"
+#include "cavs.h"
+
+/**
+ * @file
+ * This file implements the glue code between FFmpeg's and VA-API's
+ * structures for AVS (Chinese GY/T 257.1—2012) decoding.
+ */
+
+static int vaapi_avs_pic_type_cvt(int pict_type)
+{
+ switch (pict_type)
+ {
+ case AV_PICTURE_TYPE_I: return VA_AVS_I_IMG;
+ case AV_PICTURE_TYPE_P: return VA_AVS_P_IMG;
+ case AV_PICTURE_TYPE_B: return VA_AVS_B_IMG;
+ default: return VA_AVS_I_IMG;
+ }
+}
+
+static void vaapi_avs_fill_pic(VAPictureAVS *va_pic, const AVSFrame *frame)
+{
+ va_pic->surface_id = ff_vaapi_get_surface_id(frame->f);
+ va_pic->poc = frame->poc / 2;
This / 2 makes me suspicious. Does the interface actually support interlacing,
and does this code?
+}
+
+/** Initialize and start decoding a frame with VA API. */
+static int vaapi_avs_start_frame(AVCodecContext *avctx,
+ av_unused const uint8_t *buffer,
+ av_unused uint32_t size)
+{
+ int i, err;
+ AVSContext *h = avctx->priv_data;
+ VAPictureParameterBufferAVS pic_param = {};
+ VAAPIDecodePicture *vapic = h->cur.hwaccel_picture_private;
+ vapic->output_surface = ff_vaapi_get_surface_id(h->cur.f);
+
+ pic_param = (VAPictureParameterBufferAVS) {
+ .width = h->width,
+ .height = h->height,
+ .picture_type = vaapi_avs_pic_type_cvt(h->cur.f->pict_type),
Going via the pic type with a separate enum seems unnecessarily confusing here?
I think this is just the value read from the bitstream, so could it be stored
in the context?
+ .progressive_seq_flag = h->progressive_seq,
+ .progressive_frame_flag = h->progressive_frame,
+ .picture_structure_flag = h->pic_structure,
+ .fixed_pic_qp_flag = h->qp_fixed,
+ .picture_qp = h->qp,
+ .loop_filter_disable_flag = h->loop_filter_disable,
+ .alpha_c_offset = h->alpha_offset,
+ .beta_offset = h->beta_offset,
+ .skip_mode_flag_flag = h->skip_mode_flag,
+ .picture_reference_flag = h->ref_flag,
+ };
+
+ if (h->profile == 0x48) {
You already added a nice name for this magic number in the first patch!
+ pic_param.guangdian_fields.guangdian_flag = 1;
+ pic_param.guangdian_fields.aec_flag = h->aec_flag;
+ pic_param.guangdian_fields.weight_quant_flag = h->weight_quant_flag;
+ pic_param.guangdian_fields.chroma_quant_param_delta_cb =
h->chroma_quant_param_delta_cb;
+ pic_param.guangdian_fields.chroma_quant_param_delta_cr =
h->chroma_quant_param_delta_cr;
+ memcpy(pic_param.guangdian_fields.wqm_8x8, h->wqm_8x8, 64);
+ }
+
+ vaapi_avs_fill_pic(&pic_param.curr_pic, &h->cur);
+ for (i = 0; i < 2; i++) {
+ vaapi_avs_fill_pic(&pic_param.ref_list[i], &h->DPB[i]);
+ }
+
+ err = ff_vaapi_decode_make_param_buffer(avctx, vapic,
+ VAPictureParameterBufferType,
+ &pic_param, sizeof(pic_param));
+ if (err < 0)
+ goto fail;
+
+ return 0;
+fail:
+ ff_vaapi_decode_cancel(avctx, vapic);
+ return err;
+}
+
+/** End a hardware decoding based frame. */
+static int vaapi_avs_end_frame(AVCodecContext *avctx)
+{
+ AVSContext *h = avctx->priv_data;
+ VAAPIDecodePicture *vapic = h->cur.hwaccel_picture_private;
+ return ff_vaapi_decode_issue(avctx, vapic);
+}
+
+/** Decode the given H.264 slice with VA API. */
The comment is betraying where the code was copied from...
+static int vaapi_avs_decode_slice(AVCodecContext *avctx,
+ const uint8_t *buffer,
+ uint32_t size)
+{
+ int err;
+ AVSContext *h = avctx->priv_data;
+ VAAPIDecodePicture *vapic = h->cur.hwaccel_picture_private;
+ VASliceParameterBufferAVS slice_param;
+ slice_param = (VASliceParameterBufferAVS) {
+ .slice_data_size = size,
+ .slice_data_offset = 0,
+ .slice_data_flag = VA_SLICE_DATA_FLAG_ALL,
+ .mb_data_bit_offset = get_bits_count(&h->gb),
+ .slice_vertical_pos = h->stc,
+ .fixed_slice_qp_flag = h->qp_fixed,
+ .slice_qp = h->qp,
+ .slice_weight_pred_flag = h->slice_weight_pred_flag,
+ .mb_weight_pred_flag = h->mb_weight_pred_flag,
+ };
+
+ *((uint32_t *)slice_param.luma_scale) = *((uint32_t *)h->luma_scale);
+ *((uint32_t *)slice_param.luma_shift) = *((uint32_t *)h->luma_shift);
+ *((uint32_t *)slice_param.chroma_scale) = *((uint32_t *)h->chroma_scale);
+ *((uint32_t *)slice_param.chroma_shift) = *((uint32_t *)h->chroma_shift);
Please don't type-pun like this - just loop to copy each of the four bytes
normally.
+
+ err = ff_vaapi_decode_make_slice_buffer(avctx, vapic,
+ &slice_param, sizeof(slice_param),
+ buffer, size);
+ if (err < 0)
+ goto fail;
+
+ return 0;
+
+fail:
+ ff_vaapi_decode_cancel(avctx, vapic);
+ return err;
+}
+
+const FFHWAccel ff_cavs_vaapi_hwaccel = {
+ .p.name = "cavs_vaapi",
+ .p.type = AVMEDIA_TYPE_VIDEO,
+ .p.id = AV_CODEC_ID_CAVS,
+ .p.pix_fmt = AV_PIX_FMT_VAAPI,
+ .start_frame = &vaapi_avs_start_frame,
+ .end_frame = &vaapi_avs_end_frame,
+ .decode_slice = &vaapi_avs_decode_slice,
+ .frame_priv_data_size = sizeof(VAAPIDecodePicture),
+ .init = &ff_vaapi_decode_init,
+ .uninit = &ff_vaapi_decode_uninit,
+ .frame_params = &ff_vaapi_common_frame_params,
+ .priv_data_size = sizeof(VAAPIDecodeContext),
+ .caps_internal = HWACCEL_CAP_ASYNC_SAFE,
+};
diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index ceac769c52..13a3f6aa42 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -408,6 +408,10 @@ static const struct {
H264ConstrainedBaseline),
MAP(H264, H264_MAIN, H264Main ),
MAP(H264, H264_HIGH, H264High ),
+#if HAVE_VA_PROFILE_AVS
+ MAP(CAVS, CAVS_JIZHUN, AVSJizhun ),
+ MAP(CAVS, CAVS_GUANGDIAN, AVSGuangdian),
+#endif
#if VA_CHECK_VERSION(0, 37, 0)
MAP(HEVC, HEVC_MAIN, HEVCMain ),
MAP(HEVC, HEVC_MAIN_10, HEVCMain10 ),
Some questions:
* What hardware do you need to run this?
* I'm guessing you have a matching driver for the other side of the VAAPI
interface, can you give a link to that? (Is it going to be in Mesa, given that
your company's products are primarily GPUs?)
* How much effort would it be to add support for the Guangdian profile to the
software decoder?
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".
