From 47bfa71530c90c7490f60789ff64e4d6d0eabfd9 Mon Sep 17 00:00:00 2001 From: Nicolas Dufresne Date: Fri, 28 Jan 2022 14:39:35 -0500 Subject: [PATCH] v4l2codecs: h264: Improve ABI check This moves the ABI check to the registration, so we don't expose decoders with the wrong ABI or that are just broken somehow. It also makes few enhancement: - Handle missing, but required controls - Prints the controls macro name instead of id This should fix RK3399 support with a currently release minor regression in the Hantro driver that cause errors. Part-of: --- .../sys/v4l2codecs/gstv4l2codech264dec.c | 67 +++++++++++-------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/v4l2codecs/gstv4l2codech264dec.c b/subprojects/gst-plugins-bad/sys/v4l2codecs/gstv4l2codech264dec.c index 13505d4c6c..9c7260b85a 100644 --- a/subprojects/gst-plugins-bad/sys/v4l2codecs/gstv4l2codech264dec.c +++ b/subprojects/gst-plugins-bad/sys/v4l2codecs/gstv4l2codech264dec.c @@ -118,37 +118,42 @@ needs_start_codes (GstV4l2CodecH264Dec * self) return self->start_code == V4L2_STATELESS_H264_START_CODE_ANNEX_B; } - static gboolean -gst_v4l2_decoder_h264_api_check (GstV4l2CodecH264Dec * self) +gst_v4l2_decoder_h264_api_check (GstV4l2Decoder * decoder) { guint i, ret_size; /* *INDENT-OFF* */ + #define SET_ID(cid) .id = (cid), .name = #cid struct { + const gchar *name; unsigned int id; unsigned int size; + gboolean optional; } controls[] = { { - .id = V4L2_CID_STATELESS_H264_SPS, + SET_ID (V4L2_CID_STATELESS_H264_SPS), .size = sizeof(struct v4l2_ctrl_h264_sps), }, { - .id = V4L2_CID_STATELESS_H264_PPS, + SET_ID (V4L2_CID_STATELESS_H264_PPS), .size = sizeof(struct v4l2_ctrl_h264_pps), }, { - .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX, + SET_ID (V4L2_CID_STATELESS_H264_SCALING_MATRIX), .size = sizeof(struct v4l2_ctrl_h264_scaling_matrix), }, { - .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS, + SET_ID (V4L2_CID_STATELESS_H264_DECODE_PARAMS), .size = sizeof(struct v4l2_ctrl_h264_decode_params), }, { - .id = V4L2_CID_STATELESS_H264_SLICE_PARAMS, + SET_ID (V4L2_CID_STATELESS_H264_SLICE_PARAMS), .size = sizeof(struct v4l2_ctrl_h264_slice_params), + .optional = TRUE, }, { - .id = V4L2_CID_STATELESS_H264_PRED_WEIGHTS, + SET_ID (V4L2_CID_STATELESS_H264_PRED_WEIGHTS), .size = sizeof(struct v4l2_ctrl_h264_pred_weights), + .optional = TRUE, } }; + #undef SET_ID /* *INDENT-ON* */ /* @@ -156,12 +161,19 @@ gst_v4l2_decoder_h264_api_check (GstV4l2CodecH264Dec * self) * the right size. */ for (i = 0; i < G_N_ELEMENTS (controls); i++) { - if (gst_v4l2_decoder_query_control_size (self->decoder, controls[i].id, - &ret_size) && ret_size != controls[i].size) { - GST_ELEMENT_ERROR (self, RESOURCE, OPEN_READ_WRITE, - ("H264 API mismatch!"), - ("%d control size mismatch: got %d bytes but %d expected.", - controls[i].id, ret_size, controls[i].size)); + gboolean control_found; + + control_found = gst_v4l2_decoder_query_control_size (decoder, + controls[i].id, &ret_size); + + if (!controls[i].optional && !control_found) { + GST_WARNING ("Driver is missing %s support.", controls[i].name); + return FALSE; + } + + if (control_found && ret_size != controls[i].size) { + GST_WARNING ("%s control size mismatch: got %d bytes but %d expected.", + controls[i].name, ret_size, controls[i].size); return FALSE; } } @@ -173,7 +185,6 @@ static gboolean gst_v4l2_codec_h264_dec_open (GstVideoDecoder * decoder) { GstV4l2CodecH264Dec *self = GST_V4L2_CODEC_H264_DEC (decoder); - guint version; /* *INDENT-OFF* */ struct v4l2_ext_control control[] = { @@ -193,20 +204,6 @@ gst_v4l2_codec_h264_dec_open (GstVideoDecoder * decoder) return FALSE; } - version = gst_v4l2_decoder_get_version (self->decoder); - if (version < V4L2_MIN_KERNEL_VERSION) - GST_WARNING_OBJECT (self, - "V4L2 API v%u.%u too old, at least v%u.%u required", - (version >> 16) & 0xff, (version >> 8) & 0xff, - V4L2_MIN_KERNEL_VER_MAJOR, V4L2_MIN_KERNEL_VER_MINOR); - - if (!gst_v4l2_decoder_h264_api_check (self)) { - GST_ELEMENT_ERROR (self, RESOURCE, OPEN_READ_WRITE, - ("Failed to open H264 decoder"), - ("gst_v4l2_decoder_h264_api_check() failed")); - return FALSE; - } - if (!gst_v4l2_decoder_get_controls (self->decoder, control, G_N_ELEMENTS (control))) { GST_ELEMENT_ERROR (self, RESOURCE, OPEN_READ_WRITE, @@ -1520,6 +1517,7 @@ gst_v4l2_codec_h264_dec_register (GstPlugin * plugin, GstV4l2Decoder * decoder, GstV4l2CodecDevice * device, guint rank) { GstCaps *src_caps; + guint version; GST_DEBUG_CATEGORY_INIT (v4l2_h264dec_debug, "v4l2codecs-h264dec", 0, "V4L2 stateless h264 decoder"); @@ -1535,6 +1533,17 @@ gst_v4l2_codec_h264_dec_register (GstPlugin * plugin, GstV4l2Decoder * decoder, goto done; } + version = gst_v4l2_decoder_get_version (decoder); + if (version < V4L2_MIN_KERNEL_VERSION) + GST_WARNING ("V4L2 API v%u.%u too old, at least v%u.%u required", + (version >> 16) & 0xff, (version >> 8) & 0xff, + V4L2_MIN_KERNEL_VER_MAJOR, V4L2_MIN_KERNEL_VER_MINOR); + + if (!gst_v4l2_decoder_h264_api_check (decoder)) { + GST_WARNING ("Not registering H264 decoder as it failed ABI check."); + goto done; + } + gst_v4l2_decoder_register (plugin, GST_TYPE_V4L2_CODEC_H264_DEC, (GClassInitFunc) gst_v4l2_codec_h264_dec_subclass_init,