1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
|
From 7580dc7daa2ac363c20674545be200802e4f6dce Mon Sep 17 00:00:00 2001
From: John Cox <jc@kynesim.co.uk>
Date: Wed, 1 Sep 2021 16:34:50 +0100
Subject: [PATCH] media: rpivid: Avoid returning EINVAL to a G_FMT
ioctl
V4L2 spec says that G/S/TRY_FMT IOCTLs should never return errors for
anything other than wrong buffer types. Improve the capture format
function such that this is so and unsupported values get converted
to supported ones properly.
Signed-off-by: John Cox <jc@kynesim.co.uk>
---
drivers/staging/media/rpivid/rpivid.c | 1 -
drivers/staging/media/rpivid/rpivid.h | 2 -
drivers/staging/media/rpivid/rpivid_video.c | 99 +++++++++++----------
drivers/staging/media/rpivid/rpivid_video.h | 3 +-
4 files changed, 54 insertions(+), 51 deletions(-)
--- a/drivers/staging/media/rpivid/rpivid.c
+++ b/drivers/staging/media/rpivid/rpivid.c
@@ -249,7 +249,6 @@ static int rpivid_open(struct file *file
/* The only bit of format info that we can guess now is H265 src
* Everything else we need more info for
*/
- ctx->src_fmt.pixelformat = RPIVID_SRC_PIXELFORMAT_DEFAULT;
rpivid_prepare_src_format(&ctx->src_fmt);
v4l2_fh_add(&ctx->fh);
--- a/drivers/staging/media/rpivid/rpivid.h
+++ b/drivers/staging/media/rpivid/rpivid.h
@@ -35,8 +35,6 @@
#define RPIVID_QUIRK_NO_DMA_OFFSET BIT(0)
-#define RPIVID_SRC_PIXELFORMAT_DEFAULT V4L2_PIX_FMT_HEVC_SLICE
-
enum rpivid_irq_status {
RPIVID_IRQ_NONE,
RPIVID_IRQ_ERROR,
--- a/drivers/staging/media/rpivid/rpivid_video.c
+++ b/drivers/staging/media/rpivid/rpivid_video.c
@@ -27,6 +27,8 @@
#define RPIVID_MIN_WIDTH 16U
#define RPIVID_MIN_HEIGHT 16U
+#define RPIVID_DEFAULT_WIDTH 1920U
+#define RPIVID_DEFAULT_HEIGHT 1088U
#define RPIVID_MAX_WIDTH 4096U
#define RPIVID_MAX_HEIGHT 4096U
@@ -70,25 +72,22 @@ size_t rpivid_bit_buf_size(unsigned int
return rpivid_round_up_size(bits_alloc);
}
-int rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt)
+void rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt)
{
size_t size;
u32 w;
u32 h;
- if (pix_fmt->pixelformat != V4L2_PIX_FMT_HEVC_SLICE)
- return -EINVAL;
-
w = pix_fmt->width;
h = pix_fmt->height;
if (!w || !h) {
- w = 1920;
- h = 1080;
+ w = RPIVID_DEFAULT_WIDTH;
+ h = RPIVID_DEFAULT_HEIGHT;
}
- if (w > 4096)
- w = 4096;
- if (h > 4096)
- h = 4096;
+ if (w > RPIVID_MAX_WIDTH)
+ w = RPIVID_MAX_WIDTH;
+ if (h > RPIVID_MAX_HEIGHT)
+ h = RPIVID_MAX_HEIGHT;
if (!pix_fmt->plane_fmt[0].sizeimage ||
pix_fmt->plane_fmt[0].sizeimage > SZ_32M) {
@@ -98,6 +97,7 @@ int rpivid_prepare_src_format(struct v4l
/* Set a minimum */
size = max_t(u32, SZ_4K, pix_fmt->plane_fmt[0].sizeimage);
+ pix_fmt->pixelformat = V4L2_PIX_FMT_HEVC_SLICE;
pix_fmt->width = w;
pix_fmt->height = h;
pix_fmt->num_planes = 1;
@@ -105,22 +105,33 @@ int rpivid_prepare_src_format(struct v4l
/* Zero bytes per line for encoded source. */
pix_fmt->plane_fmt[0].bytesperline = 0;
pix_fmt->plane_fmt[0].sizeimage = size;
-
- return 0;
}
-int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
+/* Take any pix_format and make it valid */
+static void rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
{
unsigned int width = pix_fmt->width;
unsigned int height = pix_fmt->height;
unsigned int sizeimage = pix_fmt->plane_fmt[0].sizeimage;
unsigned int bytesperline = pix_fmt->plane_fmt[0].bytesperline;
- switch (pix_fmt->pixelformat) {
+ if (!width)
+ width = RPIVID_DEFAULT_WIDTH;
+ if (width > RPIVID_MAX_WIDTH)
+ width = RPIVID_MAX_WIDTH;
+ if (!height)
+ height = RPIVID_DEFAULT_HEIGHT;
+ if (height > RPIVID_MAX_HEIGHT)
+ height = RPIVID_MAX_HEIGHT;
+
/* For column formats set bytesperline to column height (stride2) */
+ switch (pix_fmt->pixelformat) {
+ default:
+ pix_fmt->pixelformat = V4L2_PIX_FMT_NV12_COL128;
+ fallthrough;
case V4L2_PIX_FMT_NV12_COL128:
/* Width rounds up to columns */
- width = ALIGN(min(width, RPIVID_MAX_WIDTH), 128);
+ width = ALIGN(width, 128);
/* 16 aligned height - not sure we even need that */
height = ALIGN(height, 16);
@@ -140,7 +151,7 @@ int rpivid_prepare_dst_format(struct v4l
/* width in pixels (3 pels = 4 bytes) rounded to 128 byte
* columns
*/
- width = ALIGN(((min(width, RPIVID_MAX_WIDTH) + 2) / 3), 32) * 3;
+ width = ALIGN(((width + 2) / 3), 32) * 3;
/* 16-aligned height. */
height = ALIGN(height, 16);
@@ -157,9 +168,6 @@ int rpivid_prepare_dst_format(struct v4l
sizeimage = constrain2x(sizeimage,
bytesperline * width * 4 / 3);
break;
-
- default:
- return -EINVAL;
}
pix_fmt->width = width;
@@ -169,7 +177,6 @@ int rpivid_prepare_dst_format(struct v4l
pix_fmt->plane_fmt[0].bytesperline = bytesperline;
pix_fmt->plane_fmt[0].sizeimage = sizeimage;
pix_fmt->num_planes = 1;
- return 0;
}
static int rpivid_querycap(struct file *file, void *priv,
@@ -260,14 +267,13 @@ static u32 pixelformat_from_sps(const st
{
u32 pf = 0;
- // Use width 0 as a signifier of unsetness
- if (!is_sps_set(sps)) {
+ if (!is_sps_set(sps) || !rpivid_hevc_validate_sps(sps)) {
/* Treat this as an error? For now return both */
if (index == 0)
pf = V4L2_PIX_FMT_NV12_COL128;
else if (index == 1)
pf = V4L2_PIX_FMT_NV12_10_COL128;
- } else if (index == 0 && rpivid_hevc_validate_sps(sps)) {
+ } else if (index == 0) {
if (sps->bit_depth_luma_minus8 == 0)
pf = V4L2_PIX_FMT_NV12_COL128;
else if (sps->bit_depth_luma_minus8 == 2)
@@ -282,11 +288,14 @@ rpivid_hevc_default_dst_fmt(struct rpivi
{
const struct v4l2_ctrl_hevc_sps * const sps =
rpivid_find_control_data(ctx, V4L2_CID_MPEG_VIDEO_HEVC_SPS);
- struct v4l2_pix_format_mplane pix_fmt = {
- .width = sps->pic_width_in_luma_samples,
- .height = sps->pic_height_in_luma_samples,
- .pixelformat = pixelformat_from_sps(sps, 0)
- };
+ struct v4l2_pix_format_mplane pix_fmt;
+
+ memset(&pix_fmt, 0, sizeof(pix_fmt));
+ if (is_sps_set(sps)) {
+ pix_fmt.width = sps->pic_width_in_luma_samples;
+ pix_fmt.height = sps->pic_height_in_luma_samples;
+ pix_fmt.pixelformat = pixelformat_from_sps(sps, 0);
+ }
rpivid_prepare_dst_format(&pix_fmt);
return pix_fmt;
@@ -315,14 +324,23 @@ static int rpivid_enum_fmt_vid_cap(struc
return 0;
}
+/*
+ * get dst format - sets it to default if otherwise unset
+ * returns a pointer to the struct as a convienience
+ */
+static struct v4l2_pix_format_mplane *get_dst_fmt(struct rpivid_ctx *const ctx)
+{
+ if (!ctx->dst_fmt_set)
+ ctx->dst_fmt = rpivid_hevc_default_dst_fmt(ctx);
+ return &ctx->dst_fmt;
+}
+
static int rpivid_g_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_format *f)
{
struct rpivid_ctx *ctx = rpivid_file2ctx(file);
- if (!ctx->dst_fmt_set)
- ctx->dst_fmt = rpivid_hevc_default_dst_fmt(ctx);
- f->fmt.pix_mp = ctx->dst_fmt;
+ f->fmt.pix_mp = *get_dst_fmt(ctx);
return 0;
}
@@ -358,31 +376,20 @@ static int rpivid_try_fmt_vid_cap(struct
break;
}
- // If we can't use requested fmt then set to default
- if (pixelformat == 0) {
- pixelformat = pixelformat_from_sps(sps, 0);
- // If we don't have a default then give up
- if (pixelformat == 0)
- return -EINVAL;
- }
-
// We don't have any way of finding out colourspace so believe
// anything we are told - take anything set in src as a default
if (f->fmt.pix_mp.colorspace == V4L2_COLORSPACE_DEFAULT)
copy_color(&f->fmt.pix_mp, &ctx->src_fmt);
f->fmt.pix_mp.pixelformat = pixelformat;
- return rpivid_prepare_dst_format(&f->fmt.pix_mp);
+ rpivid_prepare_dst_format(&f->fmt.pix_mp);
+ return 0;
}
static int rpivid_try_fmt_vid_out(struct file *file, void *priv,
struct v4l2_format *f)
{
- if (rpivid_prepare_src_format(&f->fmt.pix_mp)) {
- // Set default src format
- f->fmt.pix_mp.pixelformat = RPIVID_SRC_PIXELFORMAT_DEFAULT;
- rpivid_prepare_src_format(&f->fmt.pix_mp);
- }
+ rpivid_prepare_src_format(&f->fmt.pix_mp);
return 0;
}
@@ -474,7 +481,7 @@ static int rpivid_queue_setup(struct vb2
if (V4L2_TYPE_IS_OUTPUT(vq->type))
pix_fmt = &ctx->src_fmt;
else
- pix_fmt = &ctx->dst_fmt;
+ pix_fmt = get_dst_fmt(ctx);
if (*nplanes) {
if (sizes[0] < pix_fmt->plane_fmt[0].sizeimage)
--- a/drivers/staging/media/rpivid/rpivid_video.h
+++ b/drivers/staging/media/rpivid/rpivid_video.h
@@ -28,7 +28,6 @@ int rpivid_queue_init(void *priv, struct
size_t rpivid_bit_buf_size(unsigned int w, unsigned int h, unsigned int bits_minus8);
size_t rpivid_round_up_size(const size_t x);
-int rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt);
-int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt);
+void rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt);
#endif
|
