mbox series

[v10,0/4] media: vidtv: Implement a virtual DVB driver

Message ID 20200821125848.1092958-1-dwlsalmeida@gmail.com (mailing list archive)
Headers show
Series media: vidtv: Implement a virtual DVB driver | expand

Message

Daniel Almeida Aug. 21, 2020, 12:58 p.m. UTC
From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>

This series is work in progress. It represents the current work done on a
virtual DVB driver for the Linux media subsystem. I am new to the media
subsystem and to kernel development in general.

This driver aims to:
	- Serve as template for new DVB driver writers
	- Help userspace application writers in general
	- Push fake audio/video to userspace for testing
	purposes
	- Push debug information to userspace via debugfs

Current state for this driver:
	- Driver generates PSI information (PAT, PMT, SDT)
	- Driver generates PCR packets
	- Driver generates NULL packets for padding
	- PCM audio stream is decoded by ffmpeg, but no audio is heard yet.


changes in v10:
	s302m encoder got reworked

The Virtual Digital TV Driver (vidtv)

Background
----------

Vidtv is a virtual DVB driver that aims to serve as a reference for driver
writers by serving as a template. It also validates the existing media DVB
APIs, thus helping userspace application writers.

Currently, it consists of:

- A fake tuner driver, which will report a bad signal quality if the chosen
  frequency is too far away from a table of valid frequencies for a
  particular delivery system.

- A fake demod driver, which will constantly poll the fake signal quality
  returned by the tuner, simulating a device that can lose/reacquire a lock
  on the signal depending on the CNR levels.

- A fake bridge driver, which is the module responsible for modprobing the
  fake tuner and demod modules and implementing the demux logic. This module
  takes parameters at initialization that will dictate how the simulation
  behaves.

- Code reponsible for encoding a valid MPEG Transport Stream, which is then
  passed to the bridge driver. This fake stream contains some hardcoded content.
  For now, we have a single, audio-only channel containing a single MPEG
  Elementary Stream, which in turn contains a SMPTE 302m encoded sine-wave.
  Note that this particular encoder was chosen because it is the easiest
  way to encode PCM audio data in a MPEG Transport Stream.

Building vidtv
--------------
vidtv is a test driver and thus is **not** enabled by default when
compiling the kernel.

In order to enable compilation of vidtv:

- Enable **DVB_TEST_DRIVERS**, then
- Enable **DVB_VIDTV**

When compiled as a module, expect the following .ko files:

- dvb_vidtv_tuner.ko

- dvb_vidtv_demod.ko

- dvb_vidtv_bridge.ko

Running vidtv
-------------
When compiled as a module, run::

	modprobe dvb_vidtv_bridge

That's it! The bridge driver will initialize the tuner and demod drivers as
part of its own initialization.

You can optionally define some command-line arguments to vidtv, see the
documentation for more info.

Testing vidtv with v4l-utils
============================

Start by installing v4l-utils and then modprobing vidtv::

	modprobe dvb_vidtv_bridge

If the driver is OK, it should load and its probing code will run. This will
pull in the tuner and demod drivers.

Using dvb-fe-tool
-----------------

The first step to check whether the demod loaded successfully is to run::

	$ dvb-fe-tool

This should return what is currently set up at the demod struct, i.e.::

	static const struct dvb_frontend_ops vidtv_demod_ops = {
		.delsys = {
			SYS_DVBT,
			SYS_DVBT2,
			SYS_DVBC_ANNEX_A,
			SYS_DVBS,
			SYS_DVBS2,
		},

		.info = {
			.name                   = "Dummy demod for DVB-T/T2/C/S/S2",
			.frequency_min_hz       = 51 * MHz,
			.frequency_max_hz       = 2150 * MHz,
			.frequency_stepsize_hz  = 62500,
			.frequency_tolerance_hz = 29500 * kHz,
			.symbol_rate_min        = 1000000,
			.symbol_rate_max        = 45000000,

			.caps = FE_CAN_FEC_1_2 |
				FE_CAN_FEC_2_3 |
				FE_CAN_FEC_3_4 |
				FE_CAN_FEC_4_5 |
				FE_CAN_FEC_5_6 |
				FE_CAN_FEC_6_7 |
				FE_CAN_FEC_7_8 |
				FE_CAN_FEC_8_9 |
				FE_CAN_QAM_16 |
				FE_CAN_QAM_64 |
				FE_CAN_QAM_32 |
				FE_CAN_QAM_128 |
				FE_CAN_QAM_256 |
				FE_CAN_QAM_AUTO |
				FE_CAN_QPSK |
				FE_CAN_FEC_AUTO |
				FE_CAN_INVERSION_AUTO |
				FE_CAN_TRANSMISSION_MODE_AUTO |
				FE_CAN_GUARD_INTERVAL_AUTO |
				FE_CAN_HIERARCHY_AUTO,
		}

		....

Using dvb-scan
--------------

In order to tune into a channel and read the PSI tables, we can use dvb-scan.

For this, one should provide a configuration file known as a 'scan file',
here's an example::

	[Channel]
	FREQUENCY = 330000000
	MODULATION = QAM/AUTO
	SYMBOL_RATE = 6940000
	INNER_FEC = AUTO
	DELIVERY_SYSTEM = DVBC/ANNEX_A

	NOTE:
	The parameters depend on the video standard you're testing.

	NOTE:
	Vidtv is a fake driver and does not validate much of the information
	in the scan file. Just specifying 'FREQUENCY' and 'DELIVERY_SYSTEM'
	should be enough for DVB-T/DVB-T2. For DVB-S/DVB-C however, you
	should also provide 'SYMBOL_RATE'.

Assuming this channel is named 'channel.conf', you can then run::

        $ dvbv5-scan dresden_dvbc_channel.conf

Using dvb-zap
-------------

dvbv5-zap is a command line tool that can be used to record MPEG-TS to disk. The
typical use is to tune into a channel and put it into record mode. The example
below - which is taken from the documentation - illustrates that::

        $ dvbv5-zap -c dvb_channel.conf "trilhas sonoras" -r
        using demux '/dev/dvb/adapter0/demux0'
        reading channels from file 'dvb_channel.conf'
        service has pid type 05:  204
        tuning to 573000000 Hz
        audio pid 104
          dvb_set_pesfilter 104
        Lock   (0x1f) Quality= Good Signal= 100.00% C/N= -13.80dB UCB= 70 postBER= 3.14x10^-3 PER= 0
        DVR interface '/dev/dvb/adapter0/dvr0' can now be opened

The channel can be watched by playing the contents of the DVR interface, with
some player that recognizes the MPEG-TS format, such as *mplayer* or *vlc*.

By playing the contents of the stream one can visually inspect the workings of
vidtv, e.g.::

	$ mplayer /dev/dvb/adapter0/dvr0


Daniel W. S. Almeida (4):
  media: vidtv: implement a tuner driver
  media: vidtv: implement a demodulator driver
  media: vidtv: add a bridge driver
  media: Documentation: vidtv: Add ReST documentation for vidtv

 .../driver-api/media/drivers/index.rst        |    1 +
 .../driver-api/media/drivers/vidtv.rst        |  417 +++++
 MAINTAINERS                                   |    8 +
 drivers/media/test-drivers/Kconfig            |   16 +
 drivers/media/test-drivers/Makefile           |    1 +
 drivers/media/test-drivers/vidtv/Kconfig      |   11 +
 drivers/media/test-drivers/vidtv/Makefile     |    9 +
 .../media/test-drivers/vidtv/vidtv_bridge.c   |  546 +++++++
 .../media/test-drivers/vidtv/vidtv_bridge.h   |   60 +
 .../media/test-drivers/vidtv/vidtv_channel.c  |  306 ++++
 .../media/test-drivers/vidtv/vidtv_channel.h  |   76 +
 .../media/test-drivers/vidtv/vidtv_common.c   |   89 ++
 .../media/test-drivers/vidtv/vidtv_common.h   |   33 +
 .../media/test-drivers/vidtv/vidtv_demod.c    |  440 ++++++
 .../media/test-drivers/vidtv/vidtv_demod.h    |   73 +
 .../media/test-drivers/vidtv/vidtv_encoder.h  |   96 ++
 drivers/media/test-drivers/vidtv/vidtv_mux.c  |  479 ++++++
 drivers/media/test-drivers/vidtv/vidtv_mux.h  |  160 ++
 drivers/media/test-drivers/vidtv/vidtv_pes.c  |  398 +++++
 drivers/media/test-drivers/vidtv/vidtv_pes.h  |  189 +++
 drivers/media/test-drivers/vidtv/vidtv_psi.c  | 1352 +++++++++++++++++
 drivers/media/test-drivers/vidtv/vidtv_psi.h  |  593 ++++++++
 .../media/test-drivers/vidtv/vidtv_s302m.c    |  552 +++++++
 .../media/test-drivers/vidtv/vidtv_s302m.h    |   90 ++
 drivers/media/test-drivers/vidtv/vidtv_ts.c   |  137 ++
 drivers/media/test-drivers/vidtv/vidtv_ts.h   |  130 ++
 .../media/test-drivers/vidtv/vidtv_tuner.c    |  427 ++++++
 .../media/test-drivers/vidtv/vidtv_tuner.h    |   43 +
 28 files changed, 6732 insertions(+)
 create mode 100644 Documentation/driver-api/media/drivers/vidtv.rst
 create mode 100644 drivers/media/test-drivers/vidtv/Kconfig
 create mode 100644 drivers/media/test-drivers/vidtv/Makefile
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_encoder.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.h
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.c
 create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.h

Comments

Mauro Carvalho Chehab Sept. 11, 2020, 8:02 a.m. UTC | #1
Hi Daniel,

Em Fri, 21 Aug 2020 09:58:44 -0300
"Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:

> From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
> 
> This series is work in progress. It represents the current work done on a
> virtual DVB driver for the Linux media subsystem. I am new to the media
> subsystem and to kernel development in general.
> 
> This driver aims to:
> 	- Serve as template for new DVB driver writers
> 	- Help userspace application writers in general
> 	- Push fake audio/video to userspace for testing
> 	purposes
> 	- Push debug information to userspace via debugfs
> 
> Current state for this driver:
> 	- Driver generates PSI information (PAT, PMT, SDT)
> 	- Driver generates PCR packets
> 	- Driver generates NULL packets for padding
> 	- PCM audio stream is decoded by ffmpeg, but no audio is heard yet.
> 
> changes in v10:
> 	s302m encoder got reworked

Thanks for all the hard work on it. Very much appreciated!

I finally found some time to test it. For now, just a quick
test from my side, without passing any arguments to the
driver.

See enclosed.

While there are still a few issues there, I'm considering applying
this series for Kernel 5.10.

My plan is to write some patches on the top of yours, in order to
address the problems I'll find on it. If not something more critical
won't be solved in time, we may still add it at staging/media. 
Let's see.

The problems I noticed so far are:

	1. rmmod doesn't work;
	2. dvbv5-stats doesn't seem to be working properly, as
	   it is reporting signal strengh with a percentage
	   (same for Carrier S/N ratio);
	3. dvbv5-zap wrote an empty audio file (without -P flag).
	   Probably there are still some issues at the program
	   channel descriptor or service;
	4. The provider service field is null. Perhaps we could
	   add some string there, like "linuxtv.org".
	5. Maybe we could also add a simple NIT table, just to
	   avoid dvbv5-scan to wait for it until timeout.

Also, it probably makes sense to add a debugfs interface in
order to allow injecting errors at the stream at runtime.

Regards,
Mauro

---

	$ dvbv5-scan -v foobar
	using demux 'dvb0.demux0'
	Device Dummy demod for DVB-T/T2/C/S/S2 (/dev/dvb/adapter0/frontend0) capabilities:
	     CAN_FEC_1_2
	     CAN_FEC_2_3
	     CAN_FEC_3_4
	     CAN_FEC_4_5
	     CAN_FEC_5_6
	     CAN_FEC_6_7
	     CAN_FEC_7_8
	     CAN_FEC_8_9
	     CAN_FEC_AUTO
	     CAN_GUARD_INTERVAL_AUTO
	     CAN_HIERARCHY_AUTO
	     CAN_INVERSION_AUTO
	     CAN_QAM_16
	     CAN_QAM_32
	     CAN_QAM_64
	     CAN_QAM_128
	     CAN_QAM_256
	     CAN_QAM_AUTO
	     CAN_QPSK
	     CAN_TRANSMISSION_MODE_AUTO
	DVB API Version 5.11, Current v5 delivery system: DVBC/ANNEX_A
	Supported delivery systems: 
	     DVBT
	     DVBT2
	    [DVBC/ANNEX_A]
	     DVBS
	     DVBS2
	Frequency range for the current standard: 
	From:            51.0 MHz
	To:              2.15 GHz
	Step:            62.5 kHz
	Tolerance:       29.5 MHz
	Symbol rate ranges for the current standard: 
	From:            1.00 MBauds
	To:              45.0 MBauds
	Failed to guess country from the current locale setting.
	
	ERROR    command BANDWIDTH_HZ (5) not found during retrieve
	Cannot calc frequency shift. Either bandwidth/symbol-rate is unavailable (yet).
	Scanning frequency #1 330000000
	FREQUENCY = 330000000
	MODULATION = QAM/AUTO
	INVERSION = AUTO
	SYMBOL_RATE = 6940000
	INNER_FEC = AUTO
	DELIVERY_SYSTEM = DVBC/ANNEX_A
	Got parameters for DVBC/ANNEX_A:
	FREQUENCY = 330000000
	MODULATION = QAM/AUTO
	INVERSION = AUTO
	SYMBOL_RATE = 6940000
	INNER_FEC = AUTO
	DELIVERY_SYSTEM = DVBC/ANNEX_A
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	dvb_read_sections: waiting for table ID 0x00, program ID 0x00
	dvb_parse_section: received table 0x00, extension ID 0x0744, section 0/0
	dvb_parse_section: table 0x00, extension ID 0x0744: done
	PAT
	| table_id         0x00
	| section_length      13
	| one                 2
	| zero                0
	| syntax              1
	| transport_stream_id 1860
	| current_next        1
	| version             1
	| one2                3
	| section_number      0
	| last_section_number 0
	|\ 1 program pid
	|  pid 0x0101: service 0x0880
	Program #0 ID 0x0101, service ID 0x0880
	dvb_read_sections: waiting for table ID 0x02, program ID 0x101
	dvb_parse_section: received table 0x02, extension ID 0x0880, section 0/0
	dvb_parse_section: table 0x02, extension ID 0x0880: done
	PMT
	| table_id         0x02
	| section_length      24
	| one                 2
	| zero                0
	| syntax              1
	| transport_stream_id 2176
	| current_next        1
	| version             1
	| one2                3
	| section_number      0
	| last_section_number 0
	|- pcr_pid          0200
	|  reserved2           7
	|  descriptor length   0
	|  zero3               0
	|  reserved3          15
	|\
	|- stream 0x0111: ISO/IEC 13818-1 Private Data (6)
	|    descriptor length   6
	|        0x05: registration_descriptor
	|           42 53 53 44                                        BSSD
	|_  1 streams
	dvb_read_sections: waiting for table ID 0x40, program ID 0x10
	ERROR    dvb_read_sections: no data read on section filter
	ERROR    error while reading the NIT table
	dvb_read_sections: waiting for table ID 0x42, program ID 0x11
	dvb_parse_section: received table 0x42, extension ID 0x0744, section 0/0
	dvb_parse_section: table 0x42, extension ID 0x0744: done
	SDT
	| table_id         0x42
	| section_length      48
	| one                 2
	| zero                1
	| syntax              1
	| transport_stream_id 1860
	| current_next        1
	| version             1
	| one2                3
	| section_number      0
	| last_section_number 0
	| network_id          1860
	| reserved            255
	|\
	|- service 0x0880
	|   EIT schedule          0
	|   EIT present following 0
	|   free CA mode          0
	|   running status        4
	|   descriptor length     31
	|        0x48: service_descriptor
	|           service type  1
	|           provider      '(null)'
	|           name          'S302m: Sine Wave PCM Audio'
	|_  1 services
	Service S302m: Sine Wave PCM Audio, provider (null): digital television
	Storing as channel S302m: Sine Wave PCM Audio
	
	$ dvbv5-zap -c dvb_channel.conf "S302m: Sine Wave PCM Audio" -m
...
	 PID           FREQ         SPEED       TOTAL
	    0     25.07 p/s   36.826 Kbps       55 KB
	   17     25.07 p/s   36.826 Kbps       55 KB
	  257     25.07 p/s   36.826 Kbps       55 KB
	  273   1335.19 p/s    1.915 Mbps     2943 KB
	  512     25.07 p/s   36.826 Kbps       55 KB
	 8191  16775.59 p/s   24.062 Mbps    36974 KB
	TOT    18211.08 p/s   26.121 Mbps    40138 KB
	
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	
	
	$ dvbv5-zap -c dvb_channel.conf "S302m: Sine Wave PCM Audio" -t 30 -o pcm_audio.ts
	using demux 'dvb0.demux0'
	reading channels from file 'dvb_channel.conf'
	service has pid type 06:  273
	tuning to 330000000 Hz
	       (0x00) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	Record to file 'pcm_audio.ts' started
	Forcing program stop due to timeout or terminate signal
	
	$ ls -l pcm_audio.ts
	-rw-r--r-- 1 mchehab mchehab 0 Sep 11 09:46 pcm_audio.ts

	$ dvbv5-zap -c dvb_channel.conf "S302m: Sine Wave PCM Audio" -t 30 -o pcm_audio.ts -P
	using demux 'dvb0.demux0'
	reading channels from file 'dvb_channel.conf'
	service has pid type 06:  273
	tuning to 330000000 Hz
	pass all PID's to TS
	  dvb_set_pesfilter 8192
	dvb_dev_set_bufsize: buffer set to 6160384
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0
	Record to file 'pcm_audio.ts' started
	received 103809840 bytes (3379 Kbytes/sec)
	Lock   (0x1f) Signal= 0.00% C/N= 0.00% UCB= 0 postBER= 0

	$ ls -lh pcm_audio.ts
	-rw-r--r-- 1 mchehab mchehab 100M Sep 11 09:54 pcm_audio.ts
Daniel Almeida Sept. 11, 2020, 12:18 p.m. UTC | #2
Hey Mauro,

> Thanks for all the hard work on it. Very much appreciated!
> 
> I finally found some time to test it. For now, just a quick
> test from my side, without passing any arguments to the
> driver.
> 

That's nice!

> My plan is to write some patches on the top of yours, in order to
> address the problems I'll find on it. If not something more critical
> won't be solved in time, we may still add it at staging/media. 
> Let's see.

OK

> 	3. dvbv5-zap wrote an empty audio file (without -P flag).
> 	   Probably there are still some issues at the program
> 	   channel descriptor or service;

I don't remember whether I tried this. I tried dumping the stream to a
file with dvbzap, which should work. By the way, I guess we should be
comparing the output to this

$ ffmpeg -f lavfi -i sine=frequency=1000:duration=5 -ac 2 -c:a s302m
-strict -2 out.ts

Since it produces a playable transport stream file that actually sounds
like a sine tone.

Inspecting ffmpeg & vidtv output side by side in dvbinspector, you'll
see that they're mostly the same. I have a separate PID for the PCR and
some other minor differences.



> 	4. The provider service field is null. Perhaps we could
> 	   add some string there, like "linuxtv.org".
> 	5. Maybe we could also add a simple NIT table, just to
> 	   avoid dvbv5-scan to wait for it until timeout.
> 
> Also, it probably makes sense to add a debugfs interface in
> order to allow injecting errors at the stream at runtime.

Sure. This is fun, sign me up for it.



As I said in a previous email, I think the buffer in vidtv_s302m.c is
not exactly what we want. It sounds like noise.

I got it from here:
https://www.daycounter.com/Calculators/Sine-Generator-Calculator.phtml

I will just copy what I had in a previous email here:

>How do I compute the values for a sine wave such that it can be played indefinitely?
>
>I was thinking about this: (just spitballing here..)
>
>sampling_rate_khz = 48000
>buffer_size = sampling_rate_khz * seconds
>i = 0
>
>for i < buffer_size:
>buffer[i] = 32767 * sin( 2 * PI * freq / (sampling_rate_khz * i) )
>
>
>but rather I want to precompute a single period for a given frequency
>such that I can loop on the array indefinitely



By the way, after some time trying out stuff, I guess this is actually
what we need in vidtv_s302m_write_frame:

static u32 vidtv_s302m_write_frame(struct vidtv_encoder *e,
				   u16 sample)
{
	u32 nbytes = 0;
	struct vidtv_s302m_frame_16 f = {};
	struct vidtv_s302m_ctx *ctx = e->ctx;

	/* from ffmpeg: see s302enc.c */

	u8 vucf = ctx->frame_index == 0 ? 1 : 0;

	f.data[0] = reverse[sample & 0xff];
	f.data[1] = reverse[(sample & 0xff00) >>  8];
	f.data[2] = (vucf << 4)  | (reverse[(sample & 0x0f)] >> 4);
	f.data[3] = reverse[(sample & 0x0ff0) >>  4];
	f.data[4] = reverse[(sample & 0xf000) >> 12] >> 4;

	nbytes += vidtv_memcpy(e->encoder_buf,
			       e->encoder_buf_offset,
			       VIDTV_S302M_BUF_SZ,
			       &f,
			       sizeof(f));

	e->encoder_buf_offset += nbytes;

	ctx->frame_index++;
	if (ctx->frame_index >= S302M_BLOCK_SZ)
		ctx->frame_index = 0;

	return nbytes;
}


i.e. see: https://docplayer.net/37828038-For-television-mapping-of-aes3-data-into-an-mpeg-2-transport-stream.html
Figure 5, page 4.




Lastly, I am somewhat new to C, so a few things might look odd here and
there. If you come across something that's too verbose tell me and I
will try to tidy it up.


--thanks
-- Daniel
Hans Verkuil Sept. 12, 2020, 8:21 a.m. UTC | #3
Hi Daniel,

First of all, thank you for all your work on this. I see that Mauro merged
this v10, and after testing I posted a patch for the Kconfig, since that
seems to be wrong.

I have some more questions below, my apologies if that's been asked before.

On 21/08/2020 14:58, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
> 
> This series is work in progress. It represents the current work done on a
> virtual DVB driver for the Linux media subsystem. I am new to the media
> subsystem and to kernel development in general.
> 
> This driver aims to:
> 	- Serve as template for new DVB driver writers
> 	- Help userspace application writers in general
> 	- Push fake audio/video to userspace for testing
> 	purposes
> 	- Push debug information to userspace via debugfs
> 
> Current state for this driver:
> 	- Driver generates PSI information (PAT, PMT, SDT)
> 	- Driver generates PCR packets
> 	- Driver generates NULL packets for padding
> 	- PCM audio stream is decoded by ffmpeg, but no audio is heard yet.
> 
> 
> changes in v10:
> 	s302m encoder got reworked
> 
> The Virtual Digital TV Driver (vidtv)
> 
> Background
> ----------
> 
> Vidtv is a virtual DVB driver that aims to serve as a reference for driver
> writers by serving as a template. It also validates the existing media DVB
> APIs, thus helping userspace application writers.
> 
> Currently, it consists of:
> 
> - A fake tuner driver, which will report a bad signal quality if the chosen
>   frequency is too far away from a table of valid frequencies for a
>   particular delivery system.
> 
> - A fake demod driver, which will constantly poll the fake signal quality
>   returned by the tuner, simulating a device that can lose/reacquire a lock
>   on the signal depending on the CNR levels.
> 
> - A fake bridge driver, which is the module responsible for modprobing the
>   fake tuner and demod modules and implementing the demux logic. This module
>   takes parameters at initialization that will dictate how the simulation
>   behaves.
> 
> - Code reponsible for encoding a valid MPEG Transport Stream, which is then
>   passed to the bridge driver. This fake stream contains some hardcoded content.
>   For now, we have a single, audio-only channel containing a single MPEG
>   Elementary Stream, which in turn contains a SMPTE 302m encoded sine-wave.
>   Note that this particular encoder was chosen because it is the easiest
>   way to encode PCM audio data in a MPEG Transport Stream.
> 
> Building vidtv
> --------------
> vidtv is a test driver and thus is **not** enabled by default when
> compiling the kernel.
> 
> In order to enable compilation of vidtv:
> 
> - Enable **DVB_TEST_DRIVERS**, then
> - Enable **DVB_VIDTV**
> 
> When compiled as a module, expect the following .ko files:
> 
> - dvb_vidtv_tuner.ko
> 
> - dvb_vidtv_demod.ko
> 
> - dvb_vidtv_bridge.ko

Why the dvb_ prefix? All virtual drivers just start with 'vi'.

And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko?
Just like the other virtual media drivers?

> 
> Running vidtv
> -------------
> When compiled as a module, run::
> 
> 	modprobe dvb_vidtv_bridge
> 
> That's it! The bridge driver will initialize the tuner and demod drivers as
> part of its own initialization.
> 
> You can optionally define some command-line arguments to vidtv, see the
> documentation for more info.
> 
> Testing vidtv with v4l-utils
> ============================

For regression testing of vidtv during the daily build it would be great if
the contrib/test/test-media script can be enhanced to include vidtv.

This is run during the daily build with a kernel that has lockdep and many
other checks enabled, so it is very helpful to verify that no regressions
happened.

Note that this script relies on the /dev/mediaX devices to run the tests. I
noticed that vidtv doesn't appear to create a /dev/mediaX device, even though
CONFIG_MEDIA_CONTROLLER_DVB=y. This is definitely something that would be good
to support in vidtv.

Regards,

	Hans

> 
> Start by installing v4l-utils and then modprobing vidtv::
> 
> 	modprobe dvb_vidtv_bridge
> 
> If the driver is OK, it should load and its probing code will run. This will
> pull in the tuner and demod drivers.
> 
> Using dvb-fe-tool
> -----------------
> 
> The first step to check whether the demod loaded successfully is to run::
> 
> 	$ dvb-fe-tool
> 
> This should return what is currently set up at the demod struct, i.e.::
> 
> 	static const struct dvb_frontend_ops vidtv_demod_ops = {
> 		.delsys = {
> 			SYS_DVBT,
> 			SYS_DVBT2,
> 			SYS_DVBC_ANNEX_A,
> 			SYS_DVBS,
> 			SYS_DVBS2,
> 		},
> 
> 		.info = {
> 			.name                   = "Dummy demod for DVB-T/T2/C/S/S2",
> 			.frequency_min_hz       = 51 * MHz,
> 			.frequency_max_hz       = 2150 * MHz,
> 			.frequency_stepsize_hz  = 62500,
> 			.frequency_tolerance_hz = 29500 * kHz,
> 			.symbol_rate_min        = 1000000,
> 			.symbol_rate_max        = 45000000,
> 
> 			.caps = FE_CAN_FEC_1_2 |
> 				FE_CAN_FEC_2_3 |
> 				FE_CAN_FEC_3_4 |
> 				FE_CAN_FEC_4_5 |
> 				FE_CAN_FEC_5_6 |
> 				FE_CAN_FEC_6_7 |
> 				FE_CAN_FEC_7_8 |
> 				FE_CAN_FEC_8_9 |
> 				FE_CAN_QAM_16 |
> 				FE_CAN_QAM_64 |
> 				FE_CAN_QAM_32 |
> 				FE_CAN_QAM_128 |
> 				FE_CAN_QAM_256 |
> 				FE_CAN_QAM_AUTO |
> 				FE_CAN_QPSK |
> 				FE_CAN_FEC_AUTO |
> 				FE_CAN_INVERSION_AUTO |
> 				FE_CAN_TRANSMISSION_MODE_AUTO |
> 				FE_CAN_GUARD_INTERVAL_AUTO |
> 				FE_CAN_HIERARCHY_AUTO,
> 		}
> 
> 		....
> 
> Using dvb-scan
> --------------
> 
> In order to tune into a channel and read the PSI tables, we can use dvb-scan.
> 
> For this, one should provide a configuration file known as a 'scan file',
> here's an example::
> 
> 	[Channel]
> 	FREQUENCY = 330000000
> 	MODULATION = QAM/AUTO
> 	SYMBOL_RATE = 6940000
> 	INNER_FEC = AUTO
> 	DELIVERY_SYSTEM = DVBC/ANNEX_A
> 
> 	NOTE:
> 	The parameters depend on the video standard you're testing.
> 
> 	NOTE:
> 	Vidtv is a fake driver and does not validate much of the information
> 	in the scan file. Just specifying 'FREQUENCY' and 'DELIVERY_SYSTEM'
> 	should be enough for DVB-T/DVB-T2. For DVB-S/DVB-C however, you
> 	should also provide 'SYMBOL_RATE'.
> 
> Assuming this channel is named 'channel.conf', you can then run::
> 
>         $ dvbv5-scan dresden_dvbc_channel.conf
> 
> Using dvb-zap
> -------------
> 
> dvbv5-zap is a command line tool that can be used to record MPEG-TS to disk. The
> typical use is to tune into a channel and put it into record mode. The example
> below - which is taken from the documentation - illustrates that::
> 
>         $ dvbv5-zap -c dvb_channel.conf "trilhas sonoras" -r
>         using demux '/dev/dvb/adapter0/demux0'
>         reading channels from file 'dvb_channel.conf'
>         service has pid type 05:  204
>         tuning to 573000000 Hz
>         audio pid 104
>           dvb_set_pesfilter 104
>         Lock   (0x1f) Quality= Good Signal= 100.00% C/N= -13.80dB UCB= 70 postBER= 3.14x10^-3 PER= 0
>         DVR interface '/dev/dvb/adapter0/dvr0' can now be opened
> 
> The channel can be watched by playing the contents of the DVR interface, with
> some player that recognizes the MPEG-TS format, such as *mplayer* or *vlc*.
> 
> By playing the contents of the stream one can visually inspect the workings of
> vidtv, e.g.::
> 
> 	$ mplayer /dev/dvb/adapter0/dvr0
> 
> 
> Daniel W. S. Almeida (4):
>   media: vidtv: implement a tuner driver
>   media: vidtv: implement a demodulator driver
>   media: vidtv: add a bridge driver
>   media: Documentation: vidtv: Add ReST documentation for vidtv
> 
>  .../driver-api/media/drivers/index.rst        |    1 +
>  .../driver-api/media/drivers/vidtv.rst        |  417 +++++
>  MAINTAINERS                                   |    8 +
>  drivers/media/test-drivers/Kconfig            |   16 +
>  drivers/media/test-drivers/Makefile           |    1 +
>  drivers/media/test-drivers/vidtv/Kconfig      |   11 +
>  drivers/media/test-drivers/vidtv/Makefile     |    9 +
>  .../media/test-drivers/vidtv/vidtv_bridge.c   |  546 +++++++
>  .../media/test-drivers/vidtv/vidtv_bridge.h   |   60 +
>  .../media/test-drivers/vidtv/vidtv_channel.c  |  306 ++++
>  .../media/test-drivers/vidtv/vidtv_channel.h  |   76 +
>  .../media/test-drivers/vidtv/vidtv_common.c   |   89 ++
>  .../media/test-drivers/vidtv/vidtv_common.h   |   33 +
>  .../media/test-drivers/vidtv/vidtv_demod.c    |  440 ++++++
>  .../media/test-drivers/vidtv/vidtv_demod.h    |   73 +
>  .../media/test-drivers/vidtv/vidtv_encoder.h  |   96 ++
>  drivers/media/test-drivers/vidtv/vidtv_mux.c  |  479 ++++++
>  drivers/media/test-drivers/vidtv/vidtv_mux.h  |  160 ++
>  drivers/media/test-drivers/vidtv/vidtv_pes.c  |  398 +++++
>  drivers/media/test-drivers/vidtv/vidtv_pes.h  |  189 +++
>  drivers/media/test-drivers/vidtv/vidtv_psi.c  | 1352 +++++++++++++++++
>  drivers/media/test-drivers/vidtv/vidtv_psi.h  |  593 ++++++++
>  .../media/test-drivers/vidtv/vidtv_s302m.c    |  552 +++++++
>  .../media/test-drivers/vidtv/vidtv_s302m.h    |   90 ++
>  drivers/media/test-drivers/vidtv/vidtv_ts.c   |  137 ++
>  drivers/media/test-drivers/vidtv/vidtv_ts.h   |  130 ++
>  .../media/test-drivers/vidtv/vidtv_tuner.c    |  427 ++++++
>  .../media/test-drivers/vidtv/vidtv_tuner.h    |   43 +
>  28 files changed, 6732 insertions(+)
>  create mode 100644 Documentation/driver-api/media/drivers/vidtv.rst
>  create mode 100644 drivers/media/test-drivers/vidtv/Kconfig
>  create mode 100644 drivers/media/test-drivers/vidtv/Makefile
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_encoder.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.h
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.c
>  create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.h
>
Hans Verkuil Sept. 12, 2020, 8:35 a.m. UTC | #4
On 21/08/2020 14:58, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>
> 
> This series is work in progress. It represents the current work done on a
> virtual DVB driver for the Linux media subsystem. I am new to the media
> subsystem and to kernel development in general.
> 
> This driver aims to:
> 	- Serve as template for new DVB driver writers
> 	- Help userspace application writers in general
> 	- Push fake audio/video to userspace for testing
> 	purposes
> 	- Push debug information to userspace via debugfs
> 
> Current state for this driver:
> 	- Driver generates PSI information (PAT, PMT, SDT)
> 	- Driver generates PCR packets
> 	- Driver generates NULL packets for padding
> 	- PCM audio stream is decoded by ffmpeg, but no audio is heard yet.
> 
> 
> changes in v10:
> 	s302m encoder got reworked

Hi Daniel, Mauro,

When compiling for a 32 bit system I get these warnings:

linux-5.9-rc1-i686: WARNINGS

In file included from ./include/linux/kernel.h:15,
                 from ./include/asm-generic/bug.h:20,
                 from ./arch/x86/include/asm/bug.h:93,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from /home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/config-compat.h:12,
                 from /home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/compat.h:10,
                 from <command-line>:
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_common.c: In function 'vidtv_memcpy':
./include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
    5 | #define KERN_SOH "\001"  /* ASCII Start Of Header */
      |                  ^~~~~~
./include/linux/printk.h:508:10: note: in definition of macro 'printk_ratelimited'
  508 |   printk(fmt, ##__VA_ARGS__);    \
      |          ^~~
./include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
   11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
      |                  ^~~~~~~~
./include/linux/printk.h:522:21: note: in expansion of macro 'KERN_ERR'
  522 |  printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
      |                     ^~~~~~~~
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_common.c:46:3: note: in expansion of macro 'pr_err_ratelimited'
   46 |   pr_err_ratelimited("overflow detected, skipping. Try increasing the buffer size. Needed %lu, had %zu\n",
      |   ^~~~~~~~~~~~~~~~~~
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_common.c: In function 'vidtv_memset':
./include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
    5 | #define KERN_SOH "\001"  /* ASCII Start Of Header */
      |                  ^~~~~~
./include/linux/printk.h:508:10: note: in definition of macro 'printk_ratelimited'
  508 |   printk(fmt, ##__VA_ARGS__);    \
      |          ^~~
./include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
   11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
      |                  ^~~~~~~~
./include/linux/printk.h:522:21: note: in expansion of macro 'KERN_ERR'
  522 |  printk_ratelimited(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
      |                     ^~~~~~~~
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_common.c:82:3: note: in expansion of macro 'pr_err_ratelimited'
   82 |   pr_err_ratelimited("overflow detected, skipping. Try increasing the buffer size. Needed %lu, had %zu\n",
      |   ^~~~~~~~~~~~~~~~~~
In file included from ./include/linux/kernel.h:15,
                 from ./include/asm-generic/bug.h:20,
                 from ./arch/x86/include/asm/bug.h:93,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from /home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/config-compat.h:12,
                 from /home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/compat.h:10,
                 from <command-line>:
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_s302m.c: In function 'vidtv_s302m_write_frames':
./include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'u32' {aka 'unsigned int'} [-Wformat=]
    5 | #define KERN_SOH "\001"  /* ASCII Start Of Header */
      |                  ^~~~~~
./include/linux/printk.h:508:10: note: in definition of macro 'printk_ratelimited'
  508 |   printk(fmt, ##__VA_ARGS__);    \
      |          ^~~
./include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
   12 | #define KERN_WARNING KERN_SOH "4" /* warning conditions */
      |                      ^~~~~~~~
./include/linux/printk.h:524:21: note: in expansion of macro 'KERN_WARNING'
  524 |  printk_ratelimited(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
      |                     ^~~~~~~~~~~~
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_s302m.c:426:4: note: in expansion of macro 'pr_warn_ratelimited'
  426 |    pr_warn_ratelimited("write size was %d, expected %lu\n",
      |    ^~~~~~~~~~~~~~~~~~~
In file included from ./include/linux/bitops.h:5,
                 from ./include/linux/kernel.h:12,
                 from ./include/asm-generic/bug.h:20,
                 from ./arch/x86/include/asm/bug.h:93,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from /home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/config-compat.h:12,
                 from /home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/compat.h:10,
                 from <command-line>:
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_pes.c: In function 'vidtv_pes_write_pts_dts':
./include/linux/bits.h:36:11: warning: right shift count is negative [-Wshift-count-negative]
   36 |   (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
      |           ^~
./include/linux/bits.h:38:31: note: in expansion of macro '__GENMASK'
   38 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
      |                               ^~~~~~~~~
/home/hans/work/build/trees/i686/linux-5.9-rc1/media_build/v4l/vidtv_pes.c:99:10: note: in expansion of macro 'GENMASK'
   99 |  mask1 = GENMASK(32, 30);
      |          ^~~~~~~

Please take a look at this.

Regards,

	Hans
Mauro Carvalho Chehab Sept. 12, 2020, 9:15 a.m. UTC | #5
Hi Hans/Daniel,

Em Sat, 12 Sep 2020 10:21:59 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> > Building vidtv
> > --------------
> > vidtv is a test driver and thus is **not** enabled by default when
> > compiling the kernel.
> > 
> > In order to enable compilation of vidtv:
> > 
> > - Enable **DVB_TEST_DRIVERS**, then
> > - Enable **DVB_VIDTV**
> > 
> > When compiled as a module, expect the following .ko files:
> > 
> > - dvb_vidtv_tuner.ko
> > 
> > - dvb_vidtv_demod.ko
> > 
> > - dvb_vidtv_bridge.ko  
> 
> Why the dvb_ prefix? All virtual drivers just start with 'vi'.
> 
> And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko?
> Just like the other virtual media drivers?

It is a sort of standard to use dvb_* for pure digital tv
bridge drivers.

Yet, I agree with you that at least an alias is needed.
earlier today, I wrote a patch with such purpose:

	https://patchwork.linuxtv.org/project/linux-media/patch/fccf254e33bdd169dde6bdd6f941cf773c82a1c0.1599901499.git.mchehab+huawei@kernel.org/

Still, maybe we should move dvb_vidtv_tuner.ko and dvb_vidtv_demod.ko
to dvb-frontends and rename them. Not 100% sure about that, though.

I would also rename vidtv_bridge.c to just vidtv.c.


Thanks,
Mauro
Daniel Almeida Sept. 12, 2020, 2:49 p.m. UTC | #6
Hi Hans and Mauro & all


>Why the dvb_ prefix? All virtual drivers just start with 'vi'.
>
>And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko?
>Just like the other virtual media drivers?

I guess Mauro was the one to come up with the dvb_* prefix for the
kernel modules for the reasons he explicited up in this thread. 

As far as dvb_vidtv_bridge.ko and vidtv_bridge.c, I just wanted to be
verbose so that people would look at this and see that it is the code
for a bridge driver, since this is also supposed to be a template. 

Also because I had some trouble myself figuring out what was what when
first browsing through other dvb drivers. That said, I am 100% onboard
with renaming this to vidtv.ko or whatever seems more appropiate :)


>Yet, I agree with you that at least an alias is needed.
>earlier today, I wrote a patch with such purpose:

If you all would like to just leave this at that ^ I am also ok with it.

>For regression testing of vidtv during the daily build it would be
great if
>the contrib/test/test-media script can be enhanced to include vidtv.

Sure, I can do that if you'd like. Can you provide some tips on how to
get started? :)

>Note that this script relies on the /dev/mediaX devices to run the
tests. I
>noticed that vidtv doesn't appear to create a /dev/mediaX device, even though
>CONFIG_MEDIA_CONTROLLER_DVB=y. This is definitely something that would
be good
>to support in vidtv.

I didn't write any code for this specifically. I was under the
impression that the dvb core would create any/all the necessary files. Mauro?


>I'm thinking on something like:
>
>echo 1 >/sys/kernel/debug/vidtv/discontinuity
>
>to generate a frame numbering discontinuity
>
>and other things like that, changing S/N ratio and other parameters
>and injecting errors, in order to test the effects on user apps.

Can you provide an initial list of things you would like to see? I can
then implement these and we can go from there

When you say 'frame numbering discontinuity', do you mean having a gap
in the the TS continuity counter for a given pid?

For the s302m encoder, maybe we can add some noise to the samples?

> changing S/N ratio

Btw 'lose lock if signal goes bad' code in vidtv_tuner.c and
vidtv_demod.c does not work currently. Mainly because they expect an
array of valid frequencies to compare to and as of now there's no default.

I mean these, just to be clear:

static unsigned int vidtv_valid_dvb_t_freqs[NUM_VALID_TUNER_FREQS];
module_param_array(vidtv_valid_dvb_t_freqs, uint, NULL, 0);
MODULE_PARM_DESC(vidtv_valid_dvb_t_freqs,
  		 "Valid DVB-T frequencies to simulate");

static unsigned int vidtv_valid_dvb_c_freqs[NUM_VALID_TUNER_FREQS];
module_param_array(vidtv_valid_dvb_c_freqs, uint, NULL, 0);
MODULE_PARM_DESC(vidtv_valid_dvb_c_freqs,
		 "Valid DVB-C frequencies to simulate");

static unsigned int vidtv_valid_dvb_s_freqs[NUM_VALID_TUNER_FREQS];
module_param_array(vidtv_valid_dvb_s_freqs, uint, NULL, 0);
MODULE_PARM_DESC(vidtv_valid_dvb_s_freqs,
		 "Valid DVB-C frequencies to simulate");

static unsigned int max_frequency_shift_hz;
module_param(max_frequency_shift_hz, uint, 0);
MODULE_PARM_DESC(max_frequency_shift_hz,
		 "Maximum shift in HZ allowed when tuning in a channel")






> No. I meant the audio was not a sinusoidal wave after such change.

This encoder is dead simple and yet I have been struggling :/

Here is the relevant decoding code in libavcodec/s302m.c:

        for (; buf_size > 4; buf_size -= 5) {
            *o++ = (ff_reverse[buf[1]]        <<  8) |
                    ff_reverse[buf[0]];
            *o++ = (ff_reverse[buf[4] & 0xf0] << 12) |
                   (ff_reverse[buf[3]]        <<  4) |
                   (ff_reverse[buf[2]]        >>  4);
            buf += 5;
        }

I am somewhat confident that the opposite to the above is precisely what
I sent you last:

> f.data[0] = reverse[sample & 0xff];
> f.data[1] = reverse[(sample & 0xff00) >> 8];
> f.data[2] = (vucf << 4) | (reverse[(sample & 0x0f)] >> 4);
> f.data[3] = reverse[(sample & 0x0ff0) >> 4];
> f.data[4] = reverse[(sample & 0xf000) >> 12] >> 4;


Their encoder, which works perfectly of course, is:

libavcodec/s302menc.c:

        for (c = 0; c < frame->nb_samples; c++) {
            uint8_t vucf = s->framing_index == 0 ? 0x10 : 0;

            for (channels = 0; channels < avctx->channels; channels +=
2) {
                o[0] = ff_reverse[ samples[0] & 0xFF];
                o[1] = ff_reverse[(samples[0] & 0xFF00) >>  8];
                o[2] = ff_reverse[(samples[1] & 0x0F)   <<  4] | vucf;
                o[3] = ff_reverse[(samples[1] & 0x0FF0) >>  4];
                o[4] = ff_reverse[(samples[1] & 0xF000) >> 12];
                o += 5;
                samples += 2;

            }

            s->framing_index++;
            if (s->framing_index >= 192)
                s->framing_index = 0;
        }

In which samples[0] and samples[1] are the same for stereo audio.

If you feel like we should only address this later I can drop this
subject but as we've both established, the encoder in v10 is flat out
wrong. Your call :)


-- thanks
-- Daniel
Mauro Carvalho Chehab Sept. 12, 2020, 5:57 p.m. UTC | #7
Em Sat, 12 Sep 2020 11:49:01 -0300
"Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:

> Hi Hans and Mauro & all
> 
> 
> >Why the dvb_ prefix? All virtual drivers just start with 'vi'.
> >
> >And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko?
> >Just like the other virtual media drivers?  
> 
> I guess Mauro was the one to come up with the dvb_* prefix for the
> kernel modules for the reasons he explicited up in this thread. 
> 
> As far as dvb_vidtv_bridge.ko and vidtv_bridge.c, I just wanted to be
> verbose so that people would look at this and see that it is the code
> for a bridge driver, since this is also supposed to be a template. 
> 
> Also because I had some trouble myself figuring out what was what when
> first browsing through other dvb drivers. That said, I am 100% onboard
> with renaming this to vidtv.ko or whatever seems more appropiate :)

Let us think a little bit about that. 

> 
> 
> >Yet, I agree with you that at least an alias is needed.
> >earlier today, I wrote a patch with such purpose:  
> 
> If you all would like to just leave this at that ^ I am also ok with it.
> 
> >For regression testing of vidtv during the daily build it would be  
> great if
> >the contrib/test/test-media script can be enhanced to include vidtv.  
> 
> Sure, I can do that if you'd like. Can you provide some tips on how to
> get started? :)

Hans can explain it better, but the hole idea is to have a set of
userspace apps that will ensure that drivers will properly implement
the DVB API.

I suspect that, before that (or together with such tooling), we need
to properly implement the frontend ioctl, validating the per delivery
system parameters, as, right now, it just accepts anything from
userspace. 

> >Note that this script relies on the /dev/mediaX devices to run the  
> tests. I
> >noticed that vidtv doesn't appear to create a /dev/mediaX device, even though
> >CONFIG_MEDIA_CONTROLLER_DVB=y. This is definitely something that would  
> be good
> >to support in vidtv.  
> 
> I didn't write any code for this specifically. I was under the
> impression that the dvb core would create any/all the necessary files. Mauro?

It should be easy to implement support for it. The core does almost
everything. Drivers just need to initialize the media controller.

See for example:
	drivers/media/usb/em28xx/em28xx-dvb.c


> >I'm thinking on something like:
> >
> >echo 1 >/sys/kernel/debug/vidtv/discontinuity
> >
> >to generate a frame numbering discontinuity
> >
> >and other things like that, changing S/N ratio and other parameters
> >and injecting errors, in order to test the effects on user apps.  
> 
> Can you provide an initial list of things you would like to see? I can
> then implement these and we can go from there

Well, things that could help identifying bugs on userspace...

> 
> When you say 'frame numbering discontinuity', do you mean having a gap
> in the the TS continuity counter for a given pid?

... like this. Yeah, adding a gap a TS continuity counter.

Other things could be to add noise to frames and increment the
dvbv5-stat numbers and shifting bits and/or dropping 188-byte frames.

With that regards, I'm planning to add support for dvb5-stat 
myself, as this should be trivial for me. Once support gets
added, changing the stats is just a matter of increment
some counters at fe cache.

> 
> For the s302m encoder, maybe we can add some noise to the samples?

Yes.

> 
> > changing S/N ratio  
> 
> Btw 'lose lock if signal goes bad' code in vidtv_tuner.c and
> vidtv_demod.c does not work currently. Mainly because they expect an
> array of valid frequencies to compare to and as of now there's no default.
> 
> I mean these, just to be clear:
> 
> static unsigned int vidtv_valid_dvb_t_freqs[NUM_VALID_TUNER_FREQS];
> module_param_array(vidtv_valid_dvb_t_freqs, uint, NULL, 0);
> MODULE_PARM_DESC(vidtv_valid_dvb_t_freqs,
>   		 "Valid DVB-T frequencies to simulate");
> 
> static unsigned int vidtv_valid_dvb_c_freqs[NUM_VALID_TUNER_FREQS];
> module_param_array(vidtv_valid_dvb_c_freqs, uint, NULL, 0);
> MODULE_PARM_DESC(vidtv_valid_dvb_c_freqs,
> 		 "Valid DVB-C frequencies to simulate");
> 
> static unsigned int vidtv_valid_dvb_s_freqs[NUM_VALID_TUNER_FREQS];
> module_param_array(vidtv_valid_dvb_s_freqs, uint, NULL, 0);
> MODULE_PARM_DESC(vidtv_valid_dvb_s_freqs,
> 		 "Valid DVB-C frequencies to simulate");
> 
> static unsigned int max_frequency_shift_hz;
> module_param(max_frequency_shift_hz, uint, 0);
> MODULE_PARM_DESC(max_frequency_shift_hz,
> 		 "Maximum shift in HZ allowed when tuning in a channel")

Yes, I know (btw, there's a bug on its implementation - I'm working
on a fix ATM).

There are two different things here. With the above, you're
simulating tuning at a wrong frequency.

Another thing would be to have a parameter forcing errors.

This can be helpful to dynamically inject errors on a tuned
channel, and see how apps will behave.

> > No. I meant the audio was not a sinusoidal wave after such change.  
> 
> This encoder is dead simple and yet I have been struggling :/

:-)

That happens. As I said, for now this is not too important.
The way it is, it is already possible to test userspace.

Yet, it would be nice to get it fixed, and maybe change
the code in the future to play some simple public
domain music (or maybe some random music using for example
a pentatonic scale). Avoiding a single periodic waveform
can help tracking other issues when there are package drops.

> 
> Here is the relevant decoding code in libavcodec/s302m.c:
> 
>         for (; buf_size > 4; buf_size -= 5) {
>             *o++ = (ff_reverse[buf[1]]        <<  8) |
>                     ff_reverse[buf[0]];
>             *o++ = (ff_reverse[buf[4] & 0xf0] << 12) |
>                    (ff_reverse[buf[3]]        <<  4) |
>                    (ff_reverse[buf[2]]        >>  4);
>             buf += 5;
>         }
> 
> I am somewhat confident that the opposite to the above is precisely what
> I sent you last:
> 
> > f.data[0] = reverse[sample & 0xff];
> > f.data[1] = reverse[(sample & 0xff00) >> 8];
> > f.data[2] = (vucf << 4) | (reverse[(sample & 0x0f)] >> 4);
> > f.data[3] = reverse[(sample & 0x0ff0) >> 4];
> > f.data[4] = reverse[(sample & 0xf000) >> 12] >> 4;  
> 
> 
> Their encoder, which works perfectly of course, is:
> 
> libavcodec/s302menc.c:
> 
>         for (c = 0; c < frame->nb_samples; c++) {
>             uint8_t vucf = s->framing_index == 0 ? 0x10 : 0;
> 
>             for (channels = 0; channels < avctx->channels; channels +=
> 2) {
>                 o[0] = ff_reverse[ samples[0] & 0xFF];
>                 o[1] = ff_reverse[(samples[0] & 0xFF00) >>  8];
>                 o[2] = ff_reverse[(samples[1] & 0x0F)   <<  4] | vucf;
>                 o[3] = ff_reverse[(samples[1] & 0x0FF0) >>  4];
>                 o[4] = ff_reverse[(samples[1] & 0xF000) >> 12];
>                 o += 5;
>                 samples += 2;
> 
>             }
> 
>             s->framing_index++;
>             if (s->framing_index >= 192)
>                 s->framing_index = 0;
>         }
> 
> In which samples[0] and samples[1] are the same for stereo audio.
> 
> If you feel like we should only address this later I can drop this
> subject but as we've both established, the encoder in v10 is flat out
> wrong. Your call :)

Well, if you're confident and had it tested, feel free to submit
a patch ;-)

Thanks,
Mauro
Hans Verkuil Sept. 14, 2020, 8:33 a.m. UTC | #8
On 12/09/2020 19:57, Mauro Carvalho Chehab wrote:
> Em Sat, 12 Sep 2020 11:49:01 -0300
> "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> escreveu:
> 
>> Hi Hans and Mauro & all
>>
>>
>>> Why the dvb_ prefix? All virtual drivers just start with 'vi'.
>>>
>>> And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko?
>>> Just like the other virtual media drivers?  
>>
>> I guess Mauro was the one to come up with the dvb_* prefix for the
>> kernel modules for the reasons he explicited up in this thread. 
>>
>> As far as dvb_vidtv_bridge.ko and vidtv_bridge.c, I just wanted to be
>> verbose so that people would look at this and see that it is the code
>> for a bridge driver, since this is also supposed to be a template. 
>>
>> Also because I had some trouble myself figuring out what was what when
>> first browsing through other dvb drivers. That said, I am 100% onboard
>> with renaming this to vidtv.ko or whatever seems more appropiate :)
> 
> Let us think a little bit about that. 
> 
>>
>>
>>> Yet, I agree with you that at least an alias is needed.
>>> earlier today, I wrote a patch with such purpose:  
>>
>> If you all would like to just leave this at that ^ I am also ok with it.
>>
>>> For regression testing of vidtv during the daily build it would be  
>> great if
>>> the contrib/test/test-media script can be enhanced to include vidtv.  
>>
>> Sure, I can do that if you'd like. Can you provide some tips on how to
>> get started? :)
> 
> Hans can explain it better, but the hole idea is to have a set of
> userspace apps that will ensure that drivers will properly implement
> the DVB API.
> 
> I suspect that, before that (or together with such tooling), we need
> to properly implement the frontend ioctl, validating the per delivery
> system parameters, as, right now, it just accepts anything from
> userspace. 

Daniel, if you look at the test-media script, then you'll see that it has
separate sections for each virtual driver. It's probably best to look at
the vim2m driver tests since that's the easiest.

It loads the module, then it starts v4l2-compliance to test the API. This
utility basically tries all V4L2 APIs and checks that the driver conforms to
the spec.

Note that you see options like '-m platform:vim2m' that selects which /dev/media
device to use based on the name and v4l2-compliance (or v4l2-ctl with the -z option)
then walks the topology of the media device and tests all interfaces it finds.

Hence my question about media controller support in vidtv: this should be
supported there as well since it allows you to write these test sequences without
having to know which /dev/fooX device should be used.

After the compliance test is run the script tests unbind/bind (always a nasty test)
and checks for memory leaks (if enabled in the kernel).

Much of this test sequence can be copied for vidtv, but you need something else for
the compliance test. It would for now be enough to have some quick check with the
existing dvb utils from v4l-utils to see that the basics work.

IMHO I think a dtv-compliance utility along the lines of v4l2-compliance and
cec-compliance should be created.

I'm actually wondering whether the dtv compliance tests shouldn't be part of
v4l2-compliance (which would have to be renamed to media-compliance in that case)
since there are hybrid drivers supporting both in the same media topology.

This would make compliance tests possible where analog/digital TV mode handling
is tested (i.e. if analog TV is in use, then trying to use digital TV would return
EBUSY and vice versa).

It would require some work in v4l2-compliance.cpp to make this possible, but I
can do that myself.

Compliance tests have proven to be a great method of testing for regressions and
testing drivers in general.

Note that it takes a lot of time to create good compliance tests, but you just start
with some simple tests and expand it over time.

Regards,

	Hans