Hi Benjamin,
On 22/07/2024 17:06, Benjamin Gaignard wrote:
Add a test to check if V4L2_FMT_FLAG_ENUM_ALL is supported
or not by drivers.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
---
include/linux/videodev2.h | 3 ++
utils/common/v4l2-info.cpp | 1 +
utils/v4l2-compliance/v4l2-compliance.cpp | 1 +
utils/v4l2-compliance/v4l2-compliance.h | 1 +
utils/v4l2-compliance/v4l2-test-formats.cpp | 60 +++++++++++++++++++--
5 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index f18a40d4..8e2a8b36 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -864,6 +864,9 @@ struct v4l2_fmtdesc {
#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0100
#define V4L2_FMT_FLAG_META_LINE_BASED 0x0200
+/* Format description flag, to be ORed with the index */
+#define V4L2_FMT_FLAG_ENUM_ALL 0x80000000
+
/* Frame Size and frame rate enumeration */
/*
* F R A M E S I Z E E N U M E R A T I O N
diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp
index aaf7b0b5..c2c49ad0 100644
--- a/utils/common/v4l2-info.cpp
+++ b/utils/common/v4l2-info.cpp
@@ -383,6 +383,7 @@ static constexpr flag_def fmtdesc_ ## enc_type ## _def[] = { \
{ V4L2_FMT_FLAG_CSC_QUANTIZATION, "csc-quantization" }, \
{ V4L2_FMT_FLAG_CSC_XFER_FUNC, "csc-xfer-func" }, \
{ V4L2_FMT_FLAG_META_LINE_BASED, "meta-line-based" }, \
+ { V4L2_FMT_FLAG_ENUM_ALL, "enum-all-formats" }, \
{ 0, NULL } \
};
diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
index 144f9618..3446f66f 100644
--- a/utils/v4l2-compliance/v4l2-compliance.cpp
+++ b/utils/v4l2-compliance/v4l2-compliance.cpp
@@ -1444,6 +1444,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_
printf("Format ioctls%s:\n", suffix);
printf("\ttest VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: %s\n", ok(testEnumFormats(&node)));
+ printf("\ttest VIDIOC_ENUM_FMT ALL FORMATS: %s\n", ok(testEnumAllFormats(&node)));
This shouldn't be a new high-level test, instead it should be part of
testEnumFormats().
printf("\ttest VIDIOC_G/S_PARM: %s\n", ok(testParm(&node)));
printf("\ttest VIDIOC_G_FBUF: %s\n", ok(testFBuf(&node)));
printf("\ttest VIDIOC_G_FMT: %s\n", ok(testGetFormats(&node)));
diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h
index 948b62fd..be72590f 100644
--- a/utils/v4l2-compliance/v4l2-compliance.h
+++ b/utils/v4l2-compliance/v4l2-compliance.h
@@ -366,6 +366,7 @@ int testEdid(struct node *node);
// Format ioctl tests
int testEnumFormats(struct node *node);
+int testEnumAllFormats(struct node *node);
int testParm(struct node *node);
int testFBuf(struct node *node);
int testGetFormats(struct node *node);
diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp b/utils/v4l2-compliance/v4l2-test-formats.cpp
index fc16ad39..2b9b00ae 100644
--- a/utils/v4l2-compliance/v4l2-test-formats.cpp
+++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
@@ -221,7 +221,7 @@ static int testEnumFrameSizes(struct node *node, __u32 pixfmt)
return 0;
}
-static int testEnumFormatsType(struct node *node, unsigned type)
+static int _testEnumFormatsType(struct node *node, unsigned type, bool enum_all_formats)
{
pixfmt_map &map = node->buftype_pixfmts[type];
struct v4l2_fmtdesc fmtdesc;
@@ -234,6 +234,9 @@ static int testEnumFormatsType(struct node *node, unsigned type)
fmtdesc.index = f;
fmtdesc.mbus_code = 0;
+ if (enum_all_formats)
+ fmtdesc.index |= V4L2_FMT_FLAG_ENUM_ALL;
+
ret = doioctl(node, VIDIOC_ENUM_FMT, &fmtdesc);
if (ret == ENOTTY)
return ret;
@@ -246,7 +249,7 @@ static int testEnumFormatsType(struct node *node, unsigned type)
ret = check_0(fmtdesc.reserved, sizeof(fmtdesc.reserved));
if (ret)
return fail("fmtdesc.reserved not zeroed\n");
- if (fmtdesc.index != f)
+ if ((fmtdesc.index & ~V4L2_FMT_FLAG_ENUM_ALL) != f)
return fail("fmtdesc.index was modified\n");
if (fmtdesc.type != type)
return fail("fmtdesc.type was modified\n");
@@ -312,7 +315,7 @@ static int testEnumFormatsType(struct node *node, unsigned type)
continue;
// Update define in v4l2-compliance.h if new buffer types are added
assert(type <= V4L2_BUF_TYPE_LAST);
- if (map.find(fmtdesc.pixelformat) != map.end())
+ if (map.find(fmtdesc.pixelformat) != map.end() && !enum_all_formats)
return fail("duplicate format %08x (%s)\n",
fmtdesc.pixelformat, fcc2s(fmtdesc.pixelformat).c_str());
map[fmtdesc.pixelformat] = fmtdesc.flags;
@@ -321,6 +324,16 @@ static int testEnumFormatsType(struct node *node, unsigned type)
return 0;
}
+static int testEnumFormatsType(struct node *node, unsigned type)
+{
+ return _testEnumFormatsType(node, type, false);
+}
+
+static int testEnumAllFormatsType(struct node *node, unsigned type)
+{
+ return _testEnumFormatsType(node, type, true);
+}
+
int testEnumFormats(struct node *node)
{
bool supported = false;
@@ -372,6 +385,47 @@ int testEnumFormats(struct node *node)
return supported ? 0 : ENOTTY;
}
+int testEnumAllFormats(struct node *node)
+{
+ bool supported = false;
+ unsigned type;
+ int ret;
+
+ for (type = 0; type <= V4L2_BUF_TYPE_LAST; type++) {
+ ret = testEnumAllFormatsType(node, type);
+ if (ret && ret != ENOTTY)
+ return ret;
+ if (!ret)
+ supported = true;
+ switch (type) {
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
+ case V4L2_BUF_TYPE_VIDEO_OVERLAY:
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+ case V4L2_BUF_TYPE_SDR_CAPTURE:
+ case V4L2_BUF_TYPE_SDR_OUTPUT:
+ if (supported && ret && (node->g_caps() & buftype2cap[type]))
+ return fail("%s cap set, but no %s formats defined\n",
+ buftype2s(type).c_str(), buftype2s(type).c_str());
+ if (supported && !ret && !(node->g_caps() & buftype2cap[type]))
+ return fail("%s cap not set, but %s formats defined\n",
+ buftype2s(type).c_str(), buftype2s(type).c_str());
+ break;
+ case V4L2_BUF_TYPE_META_CAPTURE:
+ case V4L2_BUF_TYPE_META_OUTPUT:
+ /* Metadata formats need not be present for the current input/output */
+ break;
+ default:
+ if (!ret)
+ return fail("Buffer type %s not allowed!\n", buftype2s(type).c_str());
+ break;
+ }
+ }
+
+ return supported ? 0 : ENOTTY;
This does not test that the set of formats returned with this flag must contain all formats
returned when ENUM_FMT is called without this flag. I.e., it must be a superset of that.
Also note that testEnumFormatsType() calls testEnumFrameSizes which in turn will call
testEnumFrameIntervals. So the question is: if ENUM_FMT called with the new flag returns a
format that would normally be suppressed, and you pass that to VIDIOC_ENUM_FRAMESIZES/INTERVALS,
what should those ioctls do? Return -EINVAL? Or should it just work? Or leave it up to the
driver? Shouldn't this be documented?
