linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] vidtv: fix several things on it
@ 2020-09-21 11:43 Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 1/9] media: vidtv: simplify PCR logic to get jiffies Mauro Carvalho Chehab
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

There are still several issues at vidtv driver. This series address
the ones I found so far.

After this series, the generated TS output can now be decoded
by Kaffeine, and probably by other DVB players.

There is still something weird there, though: after playing some
notes, there's a long silence that happens only when played with
VLC.  I suspect that the package times are not ok yet, but further
investigation is required.

Mauro Carvalho Chehab (9):
  media: vidtv: simplify PCR logic to get jiffies
  media: vidtv: remove more ENDIAN_BITFIELD nonsense
  media: vidtv: fix initialization of the network_id field at SDT
  media: vidtv: cleanup PSI version numbers
  media: vidtv: cleanup SDT string identifiers
  media: vidtv: fix a typo
  media: vidtv: simplify parameters for vidtv_pes_write_stuffing()
  media: vidtv: rewrite the adaption field logic
  media: vidtv: fix decoding with gstreamer and Vlc

 .../media/test-drivers/vidtv/vidtv_channel.c  |   7 +-
 drivers/media/test-drivers/vidtv/vidtv_mux.c  |   8 +-
 drivers/media/test-drivers/vidtv/vidtv_pes.c  | 216 +++++++++++-------
 drivers/media/test-drivers/vidtv/vidtv_pes.h  |   4 +-
 drivers/media/test-drivers/vidtv/vidtv_psi.c  |  18 +-
 drivers/media/test-drivers/vidtv/vidtv_psi.h  |   8 -
 drivers/media/test-drivers/vidtv/vidtv_ts.h   |  13 --
 7 files changed, 153 insertions(+), 121 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/9] media: vidtv: simplify PCR logic to get jiffies
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 2/9] media: vidtv: remove more ENDIAN_BITFIELD nonsense Mauro Carvalho Chehab
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

There's no need to have a check when setting timing.past_jiffies,
as we can simply do the initialization earlier at vidtv_mux_init().

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_mux.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
index 43b13aa3a8bc..6a3123c51cc4 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
@@ -100,11 +100,7 @@ static void vidtv_mux_update_clk(struct vidtv_mux *m)
 	/* call this at every thread iteration */
 	u64 elapsed_time;
 
-	/* this will not hold a value yet if we have just started */
-	m->timing.past_jiffies = m->timing.current_jiffies ?
-				 m->timing.current_jiffies :
-				 get_jiffies_64();
-
+	m->timing.past_jiffies = m->timing.current_jiffies;
 	m->timing.current_jiffies = get_jiffies_64();
 
 	elapsed_time = jiffies_to_usecs(m->timing.current_jiffies -
@@ -449,6 +445,7 @@ struct vidtv_mux *vidtv_mux_init(struct dvb_frontend *fe,
 	m->pcr_pid = args.pcr_pid;
 	m->transport_stream_id = args.transport_stream_id;
 	m->priv = args.priv;
+	m->timing.current_jiffies = get_jiffies_64();
 
 	if (args.channels)
 		m->channels = args.channels;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/9] media: vidtv: remove more ENDIAN_BITFIELD nonsense
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 1/9] media: vidtv: simplify PCR logic to get jiffies Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 3/9] media: vidtv: fix initialization of the network_id field at SDT Mauro Carvalho Chehab
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

Changeset 870e350d4e39 ("media: vidtv: get rid of ENDIAN_BITFIELD nonsense")
was incomplete. There are still some wrong endannes logic at
the driver. Get rid of them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_psi.h |  8 --------
 drivers/media/test-drivers/vidtv/vidtv_ts.h  | 13 -------------
 2 files changed, 21 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_psi.h b/drivers/media/test-drivers/vidtv/vidtv_psi.h
index c9e059bfb6eb..3f962cc78278 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_psi.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_psi.h
@@ -132,17 +132,9 @@ struct vidtv_psi_table_pat {
  */
 struct vidtv_psi_table_sdt_service {
 	__be16 service_id;
-#if defined(__LITTLE_ENDIAN_BITFIELD)
 	u8 EIT_present_following:1;
 	u8 EIT_schedule:1;
 	u8 reserved:6;
-#elif defined(__BIG_ENDIAN_BITFIELD)
-	u8 reserved:6;
-	u8 EIT_schedule:1;
-	u8 EIT_present_following:1;
-#else
-#error  "Unknown bitfield ordering"
-#endif
 	__be16 bitfield; /* running_status: 3, free_ca:1, desc_loop_len:12 */
 	struct vidtv_psi_desc *descriptor;
 	struct vidtv_psi_table_sdt_service *next;
diff --git a/drivers/media/test-drivers/vidtv/vidtv_ts.h b/drivers/media/test-drivers/vidtv/vidtv_ts.h
index fab523723633..83dcc9183b45 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_ts.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_ts.h
@@ -24,7 +24,6 @@
 struct vidtv_mpeg_ts_adaption {
 	u8 length;
 	struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
 		u8 extension:1;
 		u8 private_data:1;
 		u8 splicing_point:1;
@@ -33,18 +32,6 @@ struct vidtv_mpeg_ts_adaption {
 		u8 priority:1;
 		u8 random_access:1;
 		u8 discontinued:1;
-#elif defined(__BIG_ENDIAN_BITFIELD)
-		u8 discontinued:1;
-		u8 random_access:1;
-		u8 priority:1;
-		u8 PCR:1;
-		u8 OPCR:1;
-		u8 splicing_point:1;
-		u8 private_data:1;
-		u8 extension:1;
-#else
-#error  "Unknown bitfield ordering"
-#endif
 	} __packed;
 	u8 data[];
 } __packed;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/9] media: vidtv: fix initialization of the network_id field at SDT
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 1/9] media: vidtv: simplify PCR logic to get jiffies Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 2/9] media: vidtv: remove more ENDIAN_BITFIELD nonsense Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 4/9] media: vidtv: cleanup PSI version numbers Mauro Carvalho Chehab
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

This field should point to the network ID, and has different
ranges for cable, terrestrial or satellite. It also has
an special range for temporary private usage.

For now, let's use the temporary private one. Once the
Network Information Table (NIT) gets added, this should be
reviewed.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_psi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_psi.c b/drivers/media/test-drivers/vidtv/vidtv_psi.c
index 8cdf645b4fdd..5fe3fbb765b8 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_psi.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_psi.c
@@ -1107,7 +1107,14 @@ struct vidtv_psi_table_sdt *vidtv_psi_sdt_table_init(u16 transport_stream_id)
 	sdt->header.section_id   = 0;
 	sdt->header.last_section = 0;
 
-	sdt->network_id = cpu_to_be16(transport_stream_id);
+	/*
+	 * FIXME: The network_id range from 0xFF01 to 0xFFFF is used to
+	 * indicate temporary private use. For now, let's use the first
+	 * value.
+	 * This can be changed to something more useful, when support for
+	 * NIT gets added
+	 */
+	sdt->network_id = cpu_to_be16(0xff01);
 	sdt->reserved = RESERVED;
 
 	vidtv_psi_sdt_table_update_sec_len(sdt);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/9] media: vidtv: cleanup PSI version numbers
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2020-09-21 11:43 ` [PATCH 3/9] media: vidtv: fix initialization of the network_id field at SDT Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 5/9] media: vidtv: cleanup SDT string identifiers Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

There's no reason to use static vars to store PSI version
numbers.

Also, currently, version numbers are starting with 0x01,
because there's a code being called that increases it to
1 for all table initializer code, as the code may support
dynamic changes at the PS tables on some future.

So, let's just initialize them to 0x1f, in order for the
versions to be reported as starting from 0.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_psi.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_psi.c b/drivers/media/test-drivers/vidtv/vidtv_psi.c
index 5fe3fbb765b8..82cf67dd27c0 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_psi.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_psi.c
@@ -756,7 +756,6 @@ vidtv_psi_pat_program_assign(struct vidtv_psi_table_pat *pat,
 struct vidtv_psi_table_pat *vidtv_psi_pat_table_init(u16 transport_stream_id)
 {
 	struct vidtv_psi_table_pat *pat = kzalloc(sizeof(*pat), GFP_KERNEL);
-	static u8 pat_version;
 	const u16 SYNTAX = 0x1;
 	const u16 ZERO = 0x0;
 	const u16 ONES = 0x03;
@@ -767,7 +766,7 @@ struct vidtv_psi_table_pat *vidtv_psi_pat_table_init(u16 transport_stream_id)
 	pat->header.id           = cpu_to_be16(transport_stream_id);
 	pat->header.current_next = 0x1;
 
-	pat->header.version = pat_version;
+	pat->header.version = 0x1f;
 
 	pat->header.one2         = 0x03;
 	pat->header.section_id   = 0x0;
@@ -935,7 +934,6 @@ struct vidtv_psi_table_pmt *vidtv_psi_pmt_table_init(u16 program_number,
 						     u16 pcr_pid)
 {
 	struct vidtv_psi_table_pmt *pmt = kzalloc(sizeof(*pmt), GFP_KERNEL);
-	static u8 pmt_version;
 	const u16 SYNTAX = 0x1;
 	const u16 ZERO = 0x0;
 	const u16 ONES = 0x03;
@@ -953,7 +951,7 @@ struct vidtv_psi_table_pmt *vidtv_psi_pmt_table_init(u16 program_number,
 	pmt->header.id = cpu_to_be16(program_number);
 	pmt->header.current_next = 0x1;
 
-	pmt->header.version = pmt_version;
+	pmt->header.version = 0x1f;
 
 	pmt->header.one2 = ONES;
 	pmt->header.section_id   = 0;
@@ -1083,7 +1081,6 @@ void vidtv_psi_pmt_table_destroy(struct vidtv_psi_table_pmt *pmt)
 struct vidtv_psi_table_sdt *vidtv_psi_sdt_table_init(u16 transport_stream_id)
 {
 	struct vidtv_psi_table_sdt *sdt = kzalloc(sizeof(*sdt), GFP_KERNEL);
-	static u8 sdt_version;
 	const u16 SYNTAX = 0x1;
 	const u16 ONE = 0x1;
 	const u16 ONES = 0x03;
@@ -1101,7 +1098,7 @@ struct vidtv_psi_table_sdt *vidtv_psi_sdt_table_init(u16 transport_stream_id)
 	sdt->header.id = cpu_to_be16(transport_stream_id);
 	sdt->header.current_next = ONE;
 
-	sdt->header.version = sdt_version;
+	sdt->header.version = 0x1f;
 
 	sdt->header.one2  = ONES;
 	sdt->header.section_id   = 0;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/9] media: vidtv: cleanup SDT string identifiers
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2020-09-21 11:43 ` [PATCH 4/9] media: vidtv: cleanup PSI version numbers Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 6/9] media: vidtv: fix a typo Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

In order to better test userspace applications, add an encoding
for the strings. The original channel string is also too big.
As we're just playing the first compasses of the 5th Beethoven
Symphony, change the service name to just "Beethoven". Also,
add a provider's name. For such purpose, let's point it to
the Linux media community website.

With such changes, the SDT table now looks more like one got
from a real digital TV channel:

SDT
| table_id         0x42
| section_length      44
| one                 2
| zero                1
| syntax              1
| transport_stream_id 1860
| current_next        1
| version             0
| one2                3
| section_number      0
| last_section_number 0
| network_id          65281
| reserved            255
|\
|- service 0x0880
|   EIT schedule          0
|   EIT present following 0
|   free CA mode          0
|   running status        4
|   descriptor length     27
|        0x48: service_descriptor
|           service type  1
|           provider      'LinuxTV.org'
|           name          'Beethoven'
|_  1 services

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_channel.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_channel.c b/drivers/media/test-drivers/vidtv/vidtv_channel.c
index 0a62b97e6a27..f2b97cf08e87 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_channel.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_channel.c
@@ -43,6 +43,8 @@ static void vidtv_channel_encoder_destroy(struct vidtv_encoder *e)
 	}
 }
 
+#define ENCODING_ISO8859_15 "\x0b"
+
 struct vidtv_channel
 *vidtv_channel_s302m_init(struct vidtv_channel *head, u16 transport_stream_id)
 {
@@ -55,7 +57,8 @@ struct vidtv_channel
 	const u16 s302m_es_pid              = 0x111; /* packet id for the ES */
 	const __be32 s302m_fid              = cpu_to_be32(VIDTV_S302M_FORMAT_IDENTIFIER);
 
-	char *name = "S302m: Sine Wave PCM Audio";
+	char *name = ENCODING_ISO8859_15 "Beethoven";
+	char *provider = ENCODING_ISO8859_15 "LinuxTV.org";
 
 	struct vidtv_channel *s302m = kzalloc(sizeof(*s302m), GFP_KERNEL);
 	struct vidtv_s302m_encoder_init_args encoder_args = {};
@@ -68,7 +71,7 @@ struct vidtv_channel
 				     vidtv_psi_service_desc_init(NULL,
 								 DIGITAL_TELEVISION_SERVICE,
 								 name,
-								 NULL);
+								 provider);
 
 	s302m->transport_stream_id = transport_stream_id;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/9] media: vidtv: fix a typo
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2020-09-21 11:43 ` [PATCH 5/9] media: vidtv: cleanup SDT string identifiers Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 7/9] media: vidtv: simplify parameters for vidtv_pes_write_stuffing() Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

optinal -> optional

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_pes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.h b/drivers/media/test-drivers/vidtv/vidtv_pes.h
index 36443a6e30d3..84628780d8c6 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_pes.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.h
@@ -44,7 +44,7 @@ struct vidtv_pes_optional_pts_dts {
 struct vidtv_pes_optional {
 	/*
 	 * These flags show which components are actually
-	 * present in the "optinal fields" in the optinal PES
+	 * present in the "optional fields" in the optional PES
 	 * header and which are not
 	 *
 	 * u16 two:2;  //0x2
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/9] media: vidtv: simplify parameters for vidtv_pes_write_stuffing()
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
                   ` (5 preceding siblings ...)
  2020-09-21 11:43 ` [PATCH 6/9] media: vidtv: fix a typo Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 8/9] media: vidtv: rewrite the adaption field logic Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 9/9] media: vidtv: fix decoding with gstreamer and Vlc Mauro Carvalho Chehab
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

Instead of passing struct pes_ts_header_write_args fields as
function parameters, just pass a pointer to the struct.

That would allow adding more args without needing to change
the function prototype.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_pes.c | 42 +++++++++-----------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.c b/drivers/media/test-drivers/vidtv/vidtv_pes.c
index 34518ef042f7..de0ce5529d06 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_pes.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.c
@@ -176,37 +176,34 @@ static u32 vidtv_pes_write_h(struct pes_header_write_args args)
 	return nbytes;
 }
 
-static u32 vidtv_pes_write_stuffing(void *dest_buf,
-				    u32 dest_offset,
-				    u32 n_stuffing_bytes,
-				    u32 buf_sz)
+static u32 vidtv_pes_write_stuffing(struct pes_ts_header_write_args *args,
+				    u32 dest_offset)
 {
 	u32 nbytes = 0;
 	struct vidtv_mpeg_ts_adaption ts_adap = {};
-	u32 stuff_nbytes = 0;
+	u32 stuff_nbytes;
 
-	if (!n_stuffing_bytes)
+	if (!args->n_stuffing_bytes)
 		goto out;
 
-	if (n_stuffing_bytes > PES_TS_HEADER_MAX_STUFFING_BYTES) {
+	if (args->n_stuffing_bytes > PES_TS_HEADER_MAX_STUFFING_BYTES) {
 		pr_warn_ratelimited("More than %d stuffing bytes for a PES packet!\n",
 				    PES_TS_HEADER_MAX_STUFFING_BYTES);
 
-		n_stuffing_bytes = PES_TS_HEADER_MAX_STUFFING_BYTES;
+		args->n_stuffing_bytes = PES_TS_HEADER_MAX_STUFFING_BYTES;
 	}
 
 	/* the AF will only be its 'length' field with a value of zero */
-	if (n_stuffing_bytes == 1) {
-		nbytes += vidtv_memset(dest_buf,
+	if (args->n_stuffing_bytes == 1) {
+		nbytes += vidtv_memset(args->dest_buf,
 				       dest_offset + nbytes,
-				       buf_sz,
+				       args->dest_buf_sz,
 				       0,
-				       n_stuffing_bytes);
-
+				       args->n_stuffing_bytes);
 		goto out;
 	}
 
-	stuff_nbytes = n_stuffing_bytes - sizeof(ts_adap);
+	stuff_nbytes = args->n_stuffing_bytes - sizeof(ts_adap);
 
 	/* length _immediately_ following 'adaptation_field_length' */
 	ts_adap.length = sizeof(ts_adap) -
@@ -214,24 +211,24 @@ static u32 vidtv_pes_write_stuffing(void *dest_buf,
 			 stuff_nbytes;
 
 	/* write the adap after the TS header */
-	nbytes += vidtv_memcpy(dest_buf,
+	nbytes += vidtv_memcpy(args->dest_buf,
 			       dest_offset + nbytes,
-			       buf_sz,
+			       args->dest_buf_sz,
 			       &ts_adap,
 			       sizeof(ts_adap));
 
 	/* write the stuffing bytes */
-	nbytes += vidtv_memset(dest_buf,
+	nbytes += vidtv_memset(args->dest_buf,
 			       dest_offset + nbytes,
-			       buf_sz,
+			       args->dest_buf_sz,
 			       TS_FILL_BYTE,
 			       stuff_nbytes);
 
 out:
-	if (nbytes != n_stuffing_bytes)
+	if (nbytes != args->n_stuffing_bytes)
 		pr_warn_ratelimited("write size was %d, expected %d\n",
 				    nbytes,
-				    n_stuffing_bytes);
+				    args->n_stuffing_bytes);
 
 	return nbytes;
 }
@@ -261,10 +258,7 @@ static u32 vidtv_pes_write_ts_h(struct pes_ts_header_write_args args)
 			       sizeof(ts_header));
 
 	/* write stuffing, if any */
-	nbytes += vidtv_pes_write_stuffing(args.dest_buf,
-					   args.dest_offset + nbytes,
-					   args.n_stuffing_bytes,
-					   args.dest_buf_sz);
+	nbytes += vidtv_pes_write_stuffing(&args, args.dest_offset + nbytes);
 
 	return nbytes;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/9] media: vidtv: rewrite the adaption field logic
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
                   ` (6 preceding siblings ...)
  2020-09-21 11:43 ` [PATCH 7/9] media: vidtv: simplify parameters for vidtv_pes_write_stuffing() Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  2020-09-21 11:43 ` [PATCH 9/9] media: vidtv: fix decoding with gstreamer and Vlc Mauro Carvalho Chehab
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

When compared with a stream generated via ffmpeg, it can be
noticed that the PES doesn't contain any PCR info. That could
cause problems with userspace decoding. So, rewrite the
logic that fills the adaptation info, in order to allow it
to add PCR frames without breaking frame alignment.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_mux.c |   1 +
 drivers/media/test-drivers/vidtv/vidtv_pes.c | 198 ++++++++++++-------
 drivers/media/test-drivers/vidtv/vidtv_pes.h |   2 +
 3 files changed, 128 insertions(+), 73 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
index 6a3123c51cc4..082740ae9d44 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
@@ -254,6 +254,7 @@ static u32 vidtv_mux_packetize_access_units(struct vidtv_mux *m,
 		args.access_unit_len = au->nbytes;
 		args.dest_offset     = m->mux_buf_offset;
 		args.pts             = au->pts;
+		args.pcr	     = m->timing.clk;
 
 		m->mux_buf_offset += vidtv_pes_write_into(args);
 
diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.c b/drivers/media/test-drivers/vidtv/vidtv_pes.c
index de0ce5529d06..57c7ed4fb3b5 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_pes.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.c
@@ -43,10 +43,13 @@ static u32 vidtv_pes_op_get_len(bool send_pts, bool send_dts)
 	return len;
 }
 
+#define SIZE_PCR (6 + sizeof(struct vidtv_mpeg_ts_adaption))
+
 static u32 vidtv_pes_h_get_len(bool send_pts, bool send_dts)
 {
+	u32 len = 0;
+
 	/* PES header length notwithstanding stuffing bytes */
-	u32 len = 0;
 
 	len += sizeof(struct vidtv_mpeg_pes);
 	len += vidtv_pes_op_get_len(send_pts, send_dts);
@@ -176,39 +179,58 @@ static u32 vidtv_pes_write_h(struct pes_header_write_args args)
 	return nbytes;
 }
 
+static u32 vidtv_pes_write_pcr_bits(u8 *to, u32 to_offset, u64 pcr)
+{
+	/* Exact same from ffmpeg. PCR is a counter driven by a 27Mhz clock */
+	u64 div;
+	u64 rem;
+	u8 *buf = to + to_offset;
+	u64 pcr_low;
+	u64 pcr_high;
+
+	div = div64_u64_rem(pcr, 300, &rem);
+
+	pcr_low = rem; /* pcr_low = pcr % 300 */
+	pcr_high = div; /* pcr_high = pcr / 300 */
+
+	*buf++ = pcr_high >> 25;
+	*buf++ = pcr_high >> 17;
+	*buf++ = pcr_high >>  9;
+	*buf++ = pcr_high >>  1;
+	*buf++ = pcr_high <<  7 | pcr_low >> 8 | 0x7e;
+	*buf++ = pcr_low;
+
+	return 6;
+}
+
 static u32 vidtv_pes_write_stuffing(struct pes_ts_header_write_args *args,
-				    u32 dest_offset)
+				    u32 dest_offset, bool need_pcr,
+				    u64 *last_pcr)
 {
-	u32 nbytes = 0;
 	struct vidtv_mpeg_ts_adaption ts_adap = {};
-	u32 stuff_nbytes;
+	int stuff_nbytes;
+	u32 nbytes = 0;
 
 	if (!args->n_stuffing_bytes)
-		goto out;
+		return 0;
 
-	if (args->n_stuffing_bytes > PES_TS_HEADER_MAX_STUFFING_BYTES) {
-		pr_warn_ratelimited("More than %d stuffing bytes for a PES packet!\n",
-				    PES_TS_HEADER_MAX_STUFFING_BYTES);
-
-		args->n_stuffing_bytes = PES_TS_HEADER_MAX_STUFFING_BYTES;
-	}
-
-	/* the AF will only be its 'length' field with a value of zero */
-	if (args->n_stuffing_bytes == 1) {
-		nbytes += vidtv_memset(args->dest_buf,
-				       dest_offset + nbytes,
-				       args->dest_buf_sz,
-				       0,
-				       args->n_stuffing_bytes);
-		goto out;
-	}
-
-	stuff_nbytes = args->n_stuffing_bytes - sizeof(ts_adap);
+	ts_adap.random_access = 1;
 
 	/* length _immediately_ following 'adaptation_field_length' */
-	ts_adap.length = sizeof(ts_adap) -
-			 sizeof(ts_adap.length) +
-			 stuff_nbytes;
+	if (need_pcr) {
+		ts_adap.PCR = 1;
+		ts_adap.length = SIZE_PCR;
+	} else {
+		ts_adap.length = sizeof(ts_adap);
+	}
+	stuff_nbytes = args->n_stuffing_bytes - ts_adap.length;
+
+	ts_adap.length -= sizeof(ts_adap.length);
+
+	if (unlikely(stuff_nbytes < 0))
+		stuff_nbytes = 0;
+
+	ts_adap.length += stuff_nbytes;
 
 	/* write the adap after the TS header */
 	nbytes += vidtv_memcpy(args->dest_buf,
@@ -217,23 +239,37 @@ static u32 vidtv_pes_write_stuffing(struct pes_ts_header_write_args *args,
 			       &ts_adap,
 			       sizeof(ts_adap));
 
-	/* write the stuffing bytes */
-	nbytes += vidtv_memset(args->dest_buf,
-			       dest_offset + nbytes,
-			       args->dest_buf_sz,
-			       TS_FILL_BYTE,
-			       stuff_nbytes);
+	/* write the optional PCR */
+	if (need_pcr) {
+		nbytes += vidtv_pes_write_pcr_bits(args->dest_buf,
+						   dest_offset + nbytes,
+						   args->pcr);
 
-out:
+		*last_pcr = args->pcr;
+	}
+
+	/* write the stuffing bytes, if are there anything left */
+	if (stuff_nbytes)
+		nbytes += vidtv_memset(args->dest_buf,
+				       dest_offset + nbytes,
+				       args->dest_buf_sz,
+				       TS_FILL_BYTE,
+				       stuff_nbytes);
+
+	/*
+	 * The n_stuffing_bytes contain a pre-calculated value of
+	 * the amount of data that this function would read, made from
+	 * vidtv_pes_h_get_len(). If something went wrong, print a warning
+	 */
 	if (nbytes != args->n_stuffing_bytes)
 		pr_warn_ratelimited("write size was %d, expected %d\n",
-				    nbytes,
-				    args->n_stuffing_bytes);
+				    nbytes, args->n_stuffing_bytes);
 
 	return nbytes;
 }
 
-static u32 vidtv_pes_write_ts_h(struct pes_ts_header_write_args args)
+static u32 vidtv_pes_write_ts_h(struct pes_ts_header_write_args args,
+				bool need_pcr, u64 *last_pcr)
 {
 	/* number of bytes written by this function */
 	u32 nbytes = 0;
@@ -258,43 +294,35 @@ static u32 vidtv_pes_write_ts_h(struct pes_ts_header_write_args args)
 			       sizeof(ts_header));
 
 	/* write stuffing, if any */
-	nbytes += vidtv_pes_write_stuffing(&args, args.dest_offset + nbytes);
+	nbytes += vidtv_pes_write_stuffing(&args, args.dest_offset + nbytes,
+					   need_pcr, last_pcr);
 
 	return nbytes;
 }
 
 u32 vidtv_pes_write_into(struct pes_write_args args)
 {
-	u32 nbytes_past_boundary = (args.dest_offset % TS_PACKET_LEN);
-	bool aligned = (nbytes_past_boundary == 0);
-
+	u32 unaligned_bytes = (args.dest_offset % TS_PACKET_LEN);
 	struct pes_ts_header_write_args ts_header_args = {};
-	struct pes_header_write_args pes_header_args   = {};
-
-	/* number of bytes written by this function */
-	u32 nbytes        = 0;
+	struct pes_header_write_args pes_header_args = {};
 	u32 remaining_len = args.access_unit_len;
-
 	bool wrote_pes_header = false;
+	u64 last_pcr = args.pcr;
+	bool need_pcr = true;
+	u32 available_space;
+	u32 payload_size;
+	u32 stuff_bytes;
+	u32 nbytes = 0;
 
-	/* whether we need to stuff the TS packet to align the buffer */
-	bool should_insert_stuffing_bytes = false;
-
-	u32 available_space    = 0;
-	u32 payload_write_len  = 0;
-	u32 num_stuffing_bytes = 0;
-
-	if (!aligned) {
-		pr_warn_ratelimited("Cannot start a PES packet in a misaligned buffer\n");
+	if (unaligned_bytes) {
+		pr_warn_ratelimited("buffer is misaligned, while starting PES\n");
 
 		/* forcibly align and hope for the best */
 		nbytes += vidtv_memset(args.dest_buf,
 				       args.dest_offset + nbytes,
 				       args.dest_buf_sz,
 				       TS_FILL_BYTE,
-				       TS_PACKET_LEN - nbytes_past_boundary);
-
-		aligned = true;
+				       TS_PACKET_LEN - unaligned_bytes);
 	}
 
 	if (args.send_dts && !args.send_pts) {
@@ -310,44 +338,68 @@ u32 vidtv_pes_write_into(struct pes_write_args args)
 	}
 
 	while (remaining_len) {
+		available_space = TS_PAYLOAD_LEN;
 		/*
 		 * The amount of space initially available in the TS packet.
 		 * if this is the beginning of the PES packet, take into account
 		 * the space needed for the TS header _and_ for the PES header
 		 */
-		available_space = (!wrote_pes_header) ?
-				  TS_PAYLOAD_LEN -
-				  vidtv_pes_h_get_len(args.send_pts, args.send_dts) :
-				  TS_PAYLOAD_LEN;
+		if (!wrote_pes_header)
+			available_space -= vidtv_pes_h_get_len(args.send_pts,
+							       args.send_dts);
 
-		/* if the encoder has inserted stuffing bytes in the PES
+		/*
+		 * if the encoder has inserted stuffing bytes in the PES
 		 * header, account for them.
 		 */
 		available_space -= args.n_pes_h_s_bytes;
 
-		/* whether stuffing bytes are needed to align the buffer */
-		should_insert_stuffing_bytes = remaining_len < available_space;
+		/* Take the extra adaptation into account if need to send PCR */
+		if (need_pcr) {
+			available_space -= SIZE_PCR;
+			stuff_bytes = SIZE_PCR;
+		} else {
+			stuff_bytes = 0;
+		}
 
 		/*
 		 * how much of the _actual_ payload should be written in this
 		 * packet.
 		 */
-		payload_write_len = (should_insert_stuffing_bytes) ?
-				    remaining_len :
-				    available_space;
+		if (remaining_len >= available_space) {
+			payload_size = available_space;
+		} else {
+			/* Last frame should ensure 188-bytes PS alignment */
+			payload_size = remaining_len;
+			stuff_bytes += available_space - payload_size;
 
-		num_stuffing_bytes = available_space - payload_write_len;
+			/*
+			* Ensure that the stuff bytes will be within the
+			* allowed range, decrementing the number of payload
+			* bytes to write if needed.
+			*/
+			if (stuff_bytes > PES_TS_HEADER_MAX_STUFFING_BYTES) {
+				u32 tmp = stuff_bytes - PES_TS_HEADER_MAX_STUFFING_BYTES;
+
+				stuff_bytes = PES_TS_HEADER_MAX_STUFFING_BYTES;
+				payload_size -= tmp;
+			}
+		}
 
 		/* write ts header */
 		ts_header_args.dest_buf           = args.dest_buf;
 		ts_header_args.dest_offset        = args.dest_offset + nbytes;
 		ts_header_args.dest_buf_sz        = args.dest_buf_sz;
 		ts_header_args.pid                = args.pid;
+		ts_header_args.pcr		  = args.pcr;
 		ts_header_args.continuity_counter = args.continuity_counter;
 		ts_header_args.wrote_pes_header   = wrote_pes_header;
-		ts_header_args.n_stuffing_bytes   = num_stuffing_bytes;
+		ts_header_args.n_stuffing_bytes   = stuff_bytes;
 
-		nbytes += vidtv_pes_write_ts_h(ts_header_args);
+		nbytes += vidtv_pes_write_ts_h(ts_header_args, need_pcr,
+					       &last_pcr);
+
+		need_pcr = false;
 
 		if (!wrote_pes_header) {
 			/* write the PES header only once */
@@ -375,11 +427,11 @@ u32 vidtv_pes_write_into(struct pes_write_args args)
 				       args.dest_offset + nbytes,
 				       args.dest_buf_sz,
 				       args.from,
-				       payload_write_len);
+				       payload_size);
 
-		args.from += payload_write_len;
+		args.from += payload_size;
 
-		remaining_len -= payload_write_len;
+		remaining_len -= payload_size;
 	}
 
 	return nbytes;
diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.h b/drivers/media/test-drivers/vidtv/vidtv_pes.h
index 84628780d8c6..0ea9e863024d 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_pes.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.h
@@ -125,6 +125,7 @@ struct pes_ts_header_write_args {
 	u8 *continuity_counter;
 	bool wrote_pes_header;
 	u32 n_stuffing_bytes;
+	u64 pcr;
 };
 
 /**
@@ -168,6 +169,7 @@ struct pes_write_args {
 	u64 dts;
 
 	u32 n_pes_h_s_bytes;
+	u64 pcr;
 };
 
 /**
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 9/9] media: vidtv: fix decoding with gstreamer and Vlc
  2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
                   ` (7 preceding siblings ...)
  2020-09-21 11:43 ` [PATCH 8/9] media: vidtv: rewrite the adaption field logic Mauro Carvalho Chehab
@ 2020-09-21 11:43 ` Mauro Carvalho Chehab
  8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-21 11:43 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Daniel W. S. Almeida,
	Mauro Carvalho Chehab, linux-kernel, linux-media

Neither Vlc nor Gstreamer likes the PES_scrambling_control bits.

In the case of GST, this can be seen with:

	$ GST_DEBUG=2 LANG=C gst-play-1.0 pcm_audio.ts
	...
	0:00:00.097973439 12308 0x55f7ddd155e0 WARN               pesparser pesparse.c:411:mpegts_parse_pes_header: Wrong '0x10' marker before PES_scrambling_control (0x40)
	0:00:00.097987026 12308 0x55f7ddd155e0 WARN                 tsdemux tsdemux.c:2314:gst_ts_demux_parse_pes_header: Error parsing PES header. pid: 0x111 stream_type: 0x6
	...

So, change, it. After such change, the stream now plays
fine with Vlc, Gstreamer, ffmpeg - and with programs
that use such libraries, like Kaffeine.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/media/test-drivers/vidtv/vidtv_pes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.c b/drivers/media/test-drivers/vidtv/vidtv_pes.c
index 57c7ed4fb3b5..2caad64349b3 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_pes.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.c
@@ -137,7 +137,7 @@ static u32 vidtv_pes_write_h(struct pes_header_write_args args)
 	struct vidtv_pes_optional pes_optional    = {};
 	struct pes_header_write_args pts_dts_args = args;
 	u32 stream_id = (args.encoder_id == S302M) ? PRIVATE_STREAM_1_ID : args.stream_id;
-	u16 pes_opt_bitfield = 0x2 << 13;
+	u16 pes_opt_bitfield = 0x01 << 15;
 
 	pes_header.bitfield = cpu_to_be32((PES_START_CODE_PREFIX << 8) | stream_id);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-09-21 11:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 11:43 [PATCH 0/9] vidtv: fix several things on it Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 1/9] media: vidtv: simplify PCR logic to get jiffies Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 2/9] media: vidtv: remove more ENDIAN_BITFIELD nonsense Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 3/9] media: vidtv: fix initialization of the network_id field at SDT Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 4/9] media: vidtv: cleanup PSI version numbers Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 5/9] media: vidtv: cleanup SDT string identifiers Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 6/9] media: vidtv: fix a typo Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 7/9] media: vidtv: simplify parameters for vidtv_pes_write_stuffing() Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 8/9] media: vidtv: rewrite the adaption field logic Mauro Carvalho Chehab
2020-09-21 11:43 ` [PATCH 9/9] media: vidtv: fix decoding with gstreamer and Vlc Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).