diff mbox

[RFC,1/2] app: kaffeine: Fix missing PCR on live streams.

Message ID 20170709094351.14642-1-tvboxspy@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Malcolm Priestley July 9, 2017, 9:43 a.m. UTC 
The ISO/IEC standard 13818-1 or ITU-T Rec. H.222.0 standard allow transport
vendors to place PCR (Program Clock Reference) on a different PID.

If the PCR is unset the value is 0x1fff, most vendors appear to set it the
same as video pid in which case it need not be set.

The PCR PID is at an offset of 8 in pmtSection structure.

Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
---
 src/dvb/dvbliveview.cpp | 8 ++++++++
 src/dvb/dvbsi.h         | 5 +++++
 2 files changed, 13 insertions(+)

Comments

Mauro Carvalho Chehab July 9, 2017, 11:14 a.m. UTC | #1 
Hi Malcolm,

Em Sun,  9 Jul 2017 10:43:50 +0100
Malcolm Priestley <tvboxspy@gmail.com> escreveu:

> The ISO/IEC standard 13818-1 or ITU-T Rec. H.222.0 standard allow transport
> vendors to place PCR (Program Clock Reference) on a different PID.
> 
> If the PCR is unset the value is 0x1fff, most vendors appear to set it the
> same as video pid in which case it need not be set.
> 
> The PCR PID is at an offset of 8 in pmtSection structure.

Thanks for the patches!

Patches look good, except for two things:

- we use camelCase at Kaffeine. So, the new field should be pcrPid ;)

- you didn't use dvbsi.xml. The way we usually update dvbsi.h and part of
  dvbsi.cpp is to add a field at dvbsi.xml and then run:

	$ tools/update_dvbsi.sh

  Kaffeine should be built with the optional BUILD_TOOLS feature, in order
  for it to build the tool that parses dvbsi.xml.

Anyway, I applied your patchset and added a few pathes afterwards 
adjusting it.

Regards,
Mauro

> 
> Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
> ---
>  src/dvb/dvbliveview.cpp | 8 ++++++++
>  src/dvb/dvbsi.h         | 5 +++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/src/dvb/dvbliveview.cpp b/src/dvb/dvbliveview.cpp
> index cfad892..3e92fa6 100644
> --- a/src/dvb/dvbliveview.cpp
> +++ b/src/dvb/dvbliveview.cpp
> @@ -518,6 +518,7 @@ void DvbLiveView::updatePids(bool forcePatPmtUpdate)
>  	DvbPmtSection pmtSection(internal->pmtSectionData);
>  	DvbPmtParser pmtParser(pmtSection);
>  	QSet<int> newPids;
> +	int pcr_pid = pmtSection.pcr_pid();
>  	bool updatePatPmt = forcePatPmtUpdate;
>  	bool isTimeShifting = internal->timeShiftFile.isOpen();
>  
> @@ -543,6 +544,13 @@ void DvbLiveView::updatePids(bool forcePatPmtUpdate)
>  		newPids.insert(pmtParser.teletextPid);
>  	}
>  
> +	/* check PCR PID is set */
> +	if (pcr_pid != 0x1fff) {
> +		/* Check not already in list */
> +		if (!newPids.contains(pcr_pid))
> +			newPids.insert(pcr_pid);
> +	}
> +
>  	for (int i = 0; i < pids.size(); ++i) {
>  		int pid = pids.at(i);
>  
> diff --git a/src/dvb/dvbsi.h b/src/dvb/dvbsi.h
> index 4d27252..9b4bbe0 100644
> --- a/src/dvb/dvbsi.h
> +++ b/src/dvb/dvbsi.h
> @@ -1098,6 +1098,11 @@ public:
>  		return (at(3) << 8) | at(4);
>  	}
>  
> +	int pcr_pid() const
> +	{
> +		return ((at(8) & 0x1f) << 8) | at(9);
> +	}
> +
>  	DvbDescriptor descriptors() const
>  	{
>  		return DvbDescriptor(getData() + 12, descriptorsLength);



Thanks,
Mauro
Malcolm Priestley July 9, 2017, 12:11 p.m. UTC | #2 
On 09/07/17 12:14, Mauro Carvalho Chehab wrote:
> Hi Malcolm,
> 
> Em Sun,  9 Jul 2017 10:43:50 +0100
> Malcolm Priestley <tvboxspy@gmail.com> escreveu:
> 
>> The ISO/IEC standard 13818-1 or ITU-T Rec. H.222.0 standard allow transport
>> vendors to place PCR (Program Clock Reference) on a different PID.
>>
>> If the PCR is unset the value is 0x1fff, most vendors appear to set it the
>> same as video pid in which case it need not be set.
>>
>> The PCR PID is at an offset of 8 in pmtSection structure.
> 
> Thanks for the patches!
> 
> Patches look good, except for two things:
> 
> - we use camelCase at Kaffeine. So, the new field should be pcrPid ;)
Ok, Wasn't sure

> 
> - you didn't use dvbsi.xml. The way we usually update dvbsi.h and part of
>    dvbsi.cpp is to add a field at dvbsi.xml and then run:
> 
> 	$ tools/update_dvbsi.sh
Oh I see.


> 
>    Kaffeine should be built with the optional BUILD_TOOLS feature, in order
>    for it to build the tool that parses dvbsi.xml.
> 
> Anyway, I applied your patchset and added a few pathes afterwards
> adjusting it.

Thanks

How do you turn off debug the spam from epg is horrendous.

Regards


Malcolm
Mauro Carvalho Chehab July 10, 2017, 12:38 p.m. UTC | #3 
Em Sun, 9 Jul 2017 13:11:36 +0100
Malcolm Priestley <tvboxspy@gmail.com> escreveu:

> On 09/07/17 12:14, Mauro Carvalho Chehab wrote:
> > Hi Malcolm,
> > 
> > Em Sun,  9 Jul 2017 10:43:50 +0100
> > Malcolm Priestley <tvboxspy@gmail.com> escreveu:
> >   
> >> The ISO/IEC standard 13818-1 or ITU-T Rec. H.222.0 standard allow transport
> >> vendors to place PCR (Program Clock Reference) on a different PID.
> >>
> >> If the PCR is unset the value is 0x1fff, most vendors appear to set it the
> >> same as video pid in which case it need not be set.
> >>
> >> The PCR PID is at an offset of 8 in pmtSection structure.  
> > 
> > Thanks for the patches!
> > 
> > Patches look good, except for two things:
> > 
> > - we use camelCase at Kaffeine. So, the new field should be pcrPid ;)  
> Ok, Wasn't sure
> 
> > 
> > - you didn't use dvbsi.xml. The way we usually update dvbsi.h and part of
> >    dvbsi.cpp is to add a field at dvbsi.xml and then run:
> > 
> > 	$ tools/update_dvbsi.sh  
> Oh I see.
> 
> 
> > 
> >    Kaffeine should be built with the optional BUILD_TOOLS feature, in order
> >    for it to build the tool that parses dvbsi.xml.
> > 
> > Anyway, I applied your patchset and added a few pathes afterwards
> > adjusting it.  
> 
> Thanks
> 
> How do you turn off debug the spam from epg is horrendous.

The default should have been to have those debug messages disabled.
I'm pretty sure I wrote some patches for it some time ago, but it
seems they got lost.

Anyway, I wrote them again. You should now see debug messages only
if kaffeine is called with --debug or using
	QT_LOGGING_RULES=kaffeine.category.debug=true

as stated on its help message.

> 
> Regards
> 
> 
> Malcolm
> 
> 



Thanks,
Mauro
diff mbox

Patch

diff --git a/src/dvb/dvbliveview.cpp b/src/dvb/dvbliveview.cpp
index cfad892..3e92fa6 100644
--- a/src/dvb/dvbliveview.cpp
+++ b/src/dvb/dvbliveview.cpp
@@ -518,6 +518,7 @@  void DvbLiveView::updatePids(bool forcePatPmtUpdate)
 	DvbPmtSection pmtSection(internal->pmtSectionData);
 	DvbPmtParser pmtParser(pmtSection);
 	QSet<int> newPids;
+	int pcr_pid = pmtSection.pcr_pid();
 	bool updatePatPmt = forcePatPmtUpdate;
 	bool isTimeShifting = internal->timeShiftFile.isOpen();
 
@@ -543,6 +544,13 @@  void DvbLiveView::updatePids(bool forcePatPmtUpdate)
 		newPids.insert(pmtParser.teletextPid);
 	}
 
+	/* check PCR PID is set */
+	if (pcr_pid != 0x1fff) {
+		/* Check not already in list */
+		if (!newPids.contains(pcr_pid))
+			newPids.insert(pcr_pid);
+	}
+
 	for (int i = 0; i < pids.size(); ++i) {
 		int pid = pids.at(i);
 
diff --git a/src/dvb/dvbsi.h b/src/dvb/dvbsi.h
index 4d27252..9b4bbe0 100644
--- a/src/dvb/dvbsi.h
+++ b/src/dvb/dvbsi.h
@@ -1098,6 +1098,11 @@  public:
 		return (at(3) << 8) | at(4);
 	}
 
+	int pcr_pid() const
+	{
+		return ((at(8) & 0x1f) << 8) | at(9);
+	}
+
 	DvbDescriptor descriptors() const
 	{
 		return DvbDescriptor(getData() + 12, descriptorsLength);