Skip to contentSkip to conversation

em28xx: solve several issues pointed by v4l2-compliance

PATCH In review Mauro Carvalho Chehab 1 participant 6 messages Sep 14, 2018 – Sep 15, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover em28xx: solve several issues pointed by v4l2-compliance

There are several non-compliance issues on em28xx. Fix those that I can hit with a simple grabber board like Terratec AV 350.

I also tested it with a WinTV USB2. There, I got several other compliants all related to msp3400 driver and step size. Fixing those is more complex, as it would require some non-trivial changes. So, for now, let's do just the ones that aren't related to msp3400.

Mauro Carvalho Chehab (4):
media: em28xx: fix handler for vidioc_s_input()
media: em28xx: use a default format if TRY_FMT fails
media: em28xx: fix input name for Terratec AV 350
media: em28xx: make v4l2-compliance happier by starting sequence on
zero

 drivers/media/usb/em28xx/em28xx-cards.c | 33 ++++++++-
 drivers/media/usb/em28xx/em28xx-video.c | 91 ++++++++++++++++++++++---
 drivers/media/usb/em28xx/em28xx.h       |  8 ++-
 3 files changed, 118 insertions(+), 14 deletions(-)
Signature · 1 line
2.17.1
#1 lore raw

PATCH 1/4 media: em28xx: fix handler for vidioc_s_input()

The a->index is not the name of the internal amux entry, but, instead a value from zero to the maximum number of audio inputs.

As the actual available inputs depend on each board, build it dynamically.

---
 drivers/media/usb/em28xx/em28xx-cards.c | 29 +++++++++
 drivers/media/usb/em28xx/em28xx-video.c | 83 ++++++++++++++++++++++---
 drivers/media/usb/em28xx/em28xx.h       |  8 ++-
 3 files changed, 111 insertions(+), 9 deletions(-)
drivers/media/usb/em28xx/em28xx-cards.c +29 -0
index 71c829f31d3b..e9ab1fbc8f0d 100644
@@ -3039,6 +3039,9 @@ static int em28xx_hint_board(struct em28xx *dev)
 
 static void em28xx_card_setup(struct em28xx *dev)
 {
+	int i, j, idx;
+	bool duplicate_entry;
+
 	/*
 	 * If the device can be a webcam, seek for a sensor.
 	 * If sensor is not found, then it isn't a webcam.
@@ -3195,6 +3198,32 @@ static void em28xx_card_setup(struct em28xx *dev)
 	/* Allow override tuner type by a module parameter */
 	if (tuner >= 0)
 		dev->tuner_type = tuner;
+
+	/*
+	 * Dynamically generate a list of valid audio inputs for this
+	 * specific board, mapping them via enum em28xx_amux.
+	 */
+
+	idx = 0;
+	for (i = 0; i < MAX_EM28XX_INPUT; i++) {
+		if (!INPUT(i)->type)
+			continue;
+
+		/* Skip already mapped audio inputs */
+		duplicate_entry = false;
+		for (j = 0; j < idx; j++) {
+			if (INPUT(i)->amux == dev->amux_map[j]) {
+				duplicate_entry = true;
+				break;
+			}
+		}
+		if (duplicate_entry)
+			continue;
+
+		dev->amux_map[idx++] = INPUT(i)->amux;
+	}
+	for (;idx < MAX_EM28XX_INPUT; idx++)
+		dev->amux_map[idx] = EM28XX_AMUX_UNUSED;
 }
 
 void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
drivers/media/usb/em28xx/em28xx-video.c +75 -8

136 lines not on this page · raw message

drivers/media/usb/em28xx/em28xx.h +7 -1
index 953caac025f2..a551072e62ed 100644
@@ -335,6 +335,9 @@ enum em28xx_usb_audio_type {
 /**
  * em28xx_amux - describes the type of audio input used by em28xx
  *
+ * @EM28XX_AMUX_UNUSED:
+ *	Used only on em28xx dev->map field, in order to mark an entry
+ *	as unused.
  * @EM28XX_AMUX_VIDEO:
  *	On devices without AC97, this is the only value that it is currently
  *	allowed.
@@ -369,7 +372,8 @@ enum em28xx_usb_audio_type {
  * same time, via the alsa mux.
  */
 enum em28xx_amux {
-	EM28XX_AMUX_VIDEO,
+	EM28XX_AMUX_UNUSED = -1,
+	EM28XX_AMUX_VIDEO = 0,
 	EM28XX_AMUX_LINE_IN,
 
 	/* Some less-common mixer setups */
@@ -692,6 +696,8 @@ struct em28xx {
 	unsigned int ctl_input;	// selected input
 	unsigned int ctl_ainput;// selected audio input
 	unsigned int ctl_aoutput;// selected audio output
+	enum em28xx_amux amux_map[MAX_EM28XX_INPUT];
+
 	int mute;
 	int volume;
 
Signature · 1 line
2.17.1
#2 lore raw

PATCH 2/4 media: em28xx: use a default format if TRY_FMT fails

In reply to Mauro Carvalho Chehab

Follow the V4L2 spec, as warned by v4l2-compliance:

warn: v4l2-test-formats.cpp(732): TRY_FMT cannot handle an invalid pixelformat.
warn: v4l2-test-formats.cpp(733): This may or may not be a problem. For more information see:

warn: v4l2-test-formats.cpp(734): http://www.mail-archive.com/[email protected]/msg56550.html

---
 drivers/media/usb/em28xx/em28xx-video.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/media/usb/em28xx/em28xx-video.c +3 -3
index fcb5c1b25ab3..41ac47f1589c 100644
@@ -1471,9 +1471,9 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 
 	fmt = format_by_fourcc(f->fmt.pix.pixelformat);
 	if (!fmt) {
-		em28xx_videodbg("Fourcc format (%08x) invalid.\n",
-				f->fmt.pix.pixelformat);
-		return -EINVAL;
+		fmt = &format[0];
+		em28xx_videodbg("Fourcc format (%08x) invalid. Using default (%08x).\n",
+				f->fmt.pix.pixelformat, fmt->fourcc);
 	}
 
 	if (dev->board.is_em2800) {
Signature · 1 line
2.17.1
#3 lore raw

PATCH 3/4 media: em28xx: fix input name for Terratec AV 350

In reply to Mauro Carvalho Chehab

Instead of using a register value, use an AMUX name, as otherwise VIDIOC_G_AUDIO would fail.

---
 drivers/media/usb/em28xx/em28xx-cards.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/usb/em28xx/em28xx-cards.c +2 -2
index e9ab1fbc8f0d..2a3f3e237d05 100644
@@ -2141,13 +2141,13 @@ const struct em28xx_board em28xx_boards[] = {
 		.input           = { {
 			.type     = EM28XX_VMUX_COMPOSITE,
 			.vmux     = TVP5150_COMPOSITE1,
-			.amux     = EM28XX_AUDIO_SRC_LINE,
+			.amux     = EM28XX_AMUX_LINE_IN,
 			.gpio     = terratec_av350_unmute_gpio,
 
 		}, {
 			.type     = EM28XX_VMUX_SVIDEO,
 			.vmux     = TVP5150_SVIDEO,
-			.amux     = EM28XX_AUDIO_SRC_LINE,
+			.amux     = EM28XX_AMUX_LINE_IN,
 			.gpio     = terratec_av350_unmute_gpio,
 		} },
 	},
Signature · 1 line
2.17.1
#4 lore raw

PATCH 4/4 media: em28xx: make v4l2-compliance happier by starting sequence on zero

In reply to Mauro Carvalho Chehab

The v4l2-compliance tool complains if a video doesn't start with a zero sequence number.

While this shouldn't cause any real problem for apps, let's make it happier, in order to better check the v4l2-compliance differences before and after patchsets.

---
 drivers/media/usb/em28xx/em28xx-video.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/media/usb/em28xx/em28xx-video.c +2 -0
index 41ac47f1589c..26859aaf5c93 100644
@@ -1093,6 +1093,8 @@ int em28xx_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 
 	em28xx_videodbg("%s\n", __func__);
 
+	dev->v4l2->field_count = 0;
+
 	/*
 	 * Make sure streaming is not already in progress for this type
 	 * of filehandle (e.g. video, vbi)
Signature · 1 line
2.17.1
#5 lore raw
In reply to Mauro Carvalho Chehab

Em Fri, 14 Sep 2018 15:22:30 -0300
Mauro Carvalho Chehab <[email protected]> escreveu:

Quoted 15 lines

There are several non-compliance issues on em28xx. Fix those that I can hit with a simple grabber board like Terratec AV 350.

I also tested it with a WinTV USB2. There, I got several other compliants all related to msp3400 driver and step size. Fixing those is more complex, as it would require some non-trivial changes. So, for now, let's do just the ones that aren't related to msp3400.

Mauro Carvalho Chehab (4):
media: em28xx: fix handler for vidioc_s_input()
media: em28xx: use a default format if TRY_FMT fails
media: em28xx: fix input name for Terratec AV 350
media: em28xx: make v4l2-compliance happier by starting sequence on
zero

Quoted 5 lines
 drivers/media/usb/em28xx/em28xx-cards.c | 33 ++++++++-
 drivers/media/usb/em28xx/em28xx-video.c | 91 ++++++++++++++++++++++---
 drivers/media/usb/em28xx/em28xx.h       |  8 ++-
 3 files changed, 118 insertions(+), 14 deletions(-)

Btw, if one wants to see the results for v4l2-compliance, I applied those together with my tvp5150-4 test git branch:

	https://git.linuxtv.org/mchehab/experimental.git/log/?h=tvp5150-4

Results enclosed.

Thanks,
Mauro

$ v4l2-compliance -s
v4l2-compliance SHA: bc71e4a67c6fbc5940062843bc41e7c8679634ce, 64 bits

Compliance test for device /dev/video0:

Driver Info:

	Driver name      : em28xx
	Card type        : Terratec AV350
	Bus info         : usb-0000:00:14.0-2
	Driver version   : 4.19.0
	Capabilities     : 0x85220011

Video Capture
VBI Capture
Audio
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x05220001
Video Capture
Audio
Read/Write
Streaming
Extended Pix Format

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second /dev/video0 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK
Inputs: 2 Audio Inputs: 1 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 13 Private Controls: 0

Format ioctls (Input 0):
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK

Codec ioctls (Input 0):
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK (Not Supported)

Control ioctls (Input 1):
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 13 Private Controls: 0

Format ioctls (Input 1):
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK

Codec ioctls (Input 1):
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 1):
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK (Not Supported)

Test input 0:

Streaming ioctls:
test read/write: OK
test blocking wait: OK
test MMAP: OK
test USERPTR: OK
test DMABUF: Cannot test, specify --expbuf-device

Total: 68, Succeeded: 68, Failed: 0, Warnings: 0