[mythtv] EIT fix for Norwegian DVB-T -- feed back and testing please ?

Janne Grunau janne-mythtv at grunau.be
Fri May 21 16:29:34 UTC 2010


On Fri, May 21, 2010 at 02:23:44PM +0200, Håkon Alstadheim wrote:
> Hi all, I have a patch for doctoring eit info on norwegian dvb-t, see  
> attachment. Where is the correct place to send it? The patch is against 
> 0.23-fixes rev 24755. See attached file. I'm pretty happy with the 
> effect of this patch, but I'm sure somebody will have advice on 
> coding-style. I've mentioned this patch on the list (and the users list 
> I believe), but I've had no testers step forward.  If the code is 
> unacceptable as is, at least I'd like to reserve an enum number in 
> FixUpType in eitfixup.h.

we have already a kFixNO in trunk, see http://svn.mythtv.org/trac/changeset/24121
The fixups strip just rerun information. please extend your patch based on
that one.

> Coding style:
> I'm not a professional programmer (pretty sure it shows), but I've tried 
> to factor the code in a reasonable way. I've found no obvious 
> commonalities though, so maybe I'd be better off with just one long 
> function ?

Doesn't really matter. Short, descriptive named functions doing a single thing
are prefered over long functions.

> Iffiness:
> I've noticed some very few entries in the database for programid and 
> seriesid that are clearly garbage. These prevent the frontend from 
> displaying the recorded programs  and the schedule. Setting them to the 
> empty string in the database restored functionality. Don't know If I 
> should use the fix up function to just set to empty all 
> programid/seriesid that contain non-ASCII characters. I'm pretty sure 
> the garbage is not a result of my code.

Please don't reset programid/seriesid in you're code. If it's clearly garbage
it's a bug somewhere (either in common mythtv code, your modifications or in
the broadcasting system). The bug should be identified and fixed and not
worked around.

I'll comment on the patch inline below.


> Index: libs/libmythtv/eitfixup.cpp
> ===================================================================
> --- libs/libmythtv/eitfixup.cpp	(revision 24755)
> +++ libs/libmythtv/eitfixup.cpp	(working copy)
> @@ -1,3 +1,4 @@
> +// -*- eval:(c-set-style "BSD") tab-width: 4;c-basic-offset:4; indent-tabs-mode: nil-*-

please remove, one style comment for each editor in each file doesn't scale

>  // C++ headers
>  #include <algorithm>
>  
> @@ -9,6 +10,9 @@
>  
>  #include "programinfo.h" // for subtitle types and audio and video properties
>  
> +#define LOC QString("EITFixUp: ")
> +#define LOC_ERR QString("EITFixUp, Error: ")
> +
>  /*------------------------------------------------------------------------
>   * Event Fix Up Scripts - Turned on by entry in dtv_privatetype table
>   *------------------------------------------------------------------------*/
> @@ -103,7 +107,11 @@
>        m_nlYear2("([\\s]{1}[\\(]{1}[A-Z]{0,3}/?)([1-2]{2}[0-9]{2})([\\)]{1})"),
>        m_nlDirector("(?=\\svan\\s)(([A-Z]{1}[a-z]+\\s)|([A-Z]{1}\\.\\s))"),
>        m_nlCat("^(Amusement|Muziek|Informatief|Nieuws/actualiteiten|Jeugd|Animatie|Sport|Serie/soap|Kunst/Cultuur|Documentaire|Film|Natuur|Erotiek|Comedy|Misdaad|Religieus)\\.\\s"),
> -      m_nlOmroep ("\\s\\(([A-Z]+/?)+\\)$")
> +      m_nlOmroep ("\\s\\(([A-Z]+/?)+\\)$"),
> +      m_ColonSubtitle("^([^:]+): (.+)"),
> +      m_NRKCategories("^(Barne-tv|Fantorangen|Supermorgen|Julemorgen|Sommermorgen|Kuraffen-TV|Sport i dag|NRKs sportsl.rdag|NRKs sportss.ndag|Dagens dokumentar|NRK2s historiekveld|Detektimen|Nattkino|Filmklassiker|Film|Kortfilm|P.skemorgen|Radioteatret|Opera|P2-Akademiet|Nyhetsmorgen i P2 og Alltid Nyheter:): (.+)"),
> +      m_NOPremiere("\\s+-\\s+(Sesongpremiere|Premiere)!?$"),
> +      m_NRKPartOfSeries(" ([0-9]+):([0-9]+)$")
>  
>  {
>  }
> @@ -120,6 +128,8 @@
>              event.description = event.subtitle;
>              event.subtitle = QString("");
>          }
> +        VERBOSE(VB_EIT, LOC + QString("have event.fixup %1, on %2, desc.len %3, starttime %4")
> +                .arg(event.fixup) .arg(event.title.left(100)) .arg(event.description.length()) .arg(event.starttime.toString("hh:mm:ss")));

not sure if that's useful, remove or change the level to VB_EIT|VB_EXTRA
the same is probably true for most of the VERBOSE messages

>      }
>  
>      if (kFixHDTV & event.fixup)
> @@ -155,6 +165,18 @@
>      if (kFixNL & event.fixup)
>          FixNL(event);
>  
> +    if (kFixNO & event.fixup){
> +		VERBOSE(VB_EIT, LOC + QString("have event.fixup kFixNO"));

indent is 4 spaces, VERBOSE is not useful

> +        //event.fixup |=  EITFixUp::kFixGenericDVB;

please remove, should be already the case

> +		FixNO(event);
> +		FixNRKSeries(event);
> +		FixColonSubtitle(event);
> +		FixTitleQuote(event);
> +		VERBOSE(VB_EIT, LOC + QString("New Title:%1 Subtitle:%2")
> +				.arg(event.title.left(35)) .arg(event.subtitle.left(35)));
> +		
> +    }
> +
>      if (event.fixup)
>      {
>          if (!event.title.isEmpty())
> @@ -167,6 +189,7 @@
>  
>      if (kFixGenericDVB & event.fixup)
>      {
> +        VERBOSE(VB_EIT, LOC + QString("have event.fixup kFixGenericDVB"));

please remove

>          event.programId = AddDVBEITAuthority(event.chanid, event.programId);
>          event.seriesId  = AddDVBEITAuthority(event.chanid, event.seriesId);
>      }
> @@ -1081,6 +1104,131 @@
>  
>  }
>  
> +
> +
> +/** \fn EITFixUp::FixNRKSeries(DBEventEIT&) const
> + *  \brief Remove colon-separated episode number. Run before ColonSubtitle

despite my earlier comment. if there are dependencies between the functions it
makes probably sense to handle it in a single function

> + */
> +void EITFixUp::FixNRKSeries(DBEventEIT &event) const
> +{
> +    int position1;
> +    // Work out the episode numbers (if any)
> +    bool    series  = false;
> +    QRegExp tmpExp1 = m_NRKPartOfSeries;
> +    if ((position1 = tmpExp1.indexIn(event.title)) != -1)
> +    {
> +        if ((tmpExp1.cap(1).toUInt() <= tmpExp1.cap(2).toUInt())
> +            && tmpExp1.cap(2).toUInt()<=50)
> +        {
> +            event.partnumber = tmpExp1.cap(1).toUInt();
> +            event.parttotal  = tmpExp1.cap(2).toUInt();
> +            // Remove from the title
> +			VERBOSE(VB_EIT, LOC + QString("Dropping series episode # from title"));
> +            event.title = event.title.left(position1) +
> +				event.title.mid(position1 + tmpExp1.cap(0).length());
> +            series = true;
> +        }
> +    }
> +}
> +
> +/** \fn EITFixUp::FixColonSubtitle(DBEventEIT&) const
> + *  \brief Use this to standardise the Norwegian channels. Only NRK for now.
> + */
> +void EITFixUp::FixColonSubtitle(DBEventEIT &event) const
> +{
> +    int        position;
> +    QRegExp    tmpExp1;
> +    // Try to find colon-delimited subtitle in title, only validated for NRK channels
> +    tmpExp1 = m_ColonSubtitle;
> +    if (!event.title.startsWith("CSI:") && !event.title.startsWith("CD:") && !event.title.startsWith("Distriktsnyheter: fra"))
> +    {
> +		if ((position = tmpExp1.indexIn(event.title)) != -1)
> +		{
> +
> +			if (event.subtitle.length() <= 0)
> +			{
> +				event.title    = tmpExp1.cap(1);
> +				event.subtitle = tmpExp1.cap(2);
> +			} else if (event.subtitle == tmpExp1.cap(2)) {
> +				VERBOSE(VB_EIT, LOC + QString("Dropping existing subtitle from title"));
> +				event.title    = tmpExp1.cap(1);
> +			} else {
> +				VERBOSE(VB_EIT, LOC + QString("Found colon, but it seems there already is a subtitle"));
> +			}
> +		}
> +    }
> +}
> +
> +
> +/** \fn EITFixUp::FixTitleQuote(DBEventEIT&) const
> + *  \brief After cleanup, sometimes the title/subtitle will be in quotes
> + */
> +void EITFixUp::FixTitleQuote(DBEventEIT &event) const
> +{
> +	// Trimming may seem to be done twice,
> +	// But after removing quotes, it is needed again
> +	if (!event.title.isEmpty())
> +		event.title = event.title.trimmed();
> +	if (!event.subtitle.isEmpty())
> +		event.subtitle = event.subtitle.trimmed();
> +	if (!event.description.isEmpty())
> +		event.description = event.description.trimmed();
> +  
> +	if (event.title.startsWith("\""))
> +	{
> +		if(event.title.endsWith("\""))
> +		{
> +			VERBOSE(VB_EIT, LOC + QString("Removing quotes around title"));
> +			event.title = event.title.mid(1,event.title.length() -2);
> +			
> +		}
> +		else
> +		{
> +			int titleEndPos;
> +			titleEndPos = event.title.indexOf('"',1);
> +			if(titleEndPos != -1)
> +			{
> +				VERBOSE(VB_EIT, LOC + QString("Removing quotes around title, moving rest to desc."));
> +				event.description = event.description + QString(" ") + event.title.mid(titleEndPos + 1);
> +				event.title = event.title.mid(1,titleEndPos - 1);
> +			}
> +		}
> +	}
> +	if (event.subtitle.startsWith("\"") && event.subtitle.endsWith("\""))
> +	{
> +		VERBOSE(VB_EIT, LOC + QString("Removing quotes around subtitle"));
> +		event.subtitle = event.subtitle.mid(1,event.title.length() -2);
> +	}
> +}
> +
> +/** \fn EITFixUp::FixNO(DBEventEIT&) const
> + *  \brief Use this to standardise the Norwegian channels. Only NRK for now.
> + */
> +void EITFixUp::FixNO(DBEventEIT &event) const
> +{
> +    int        position;
> +    QRegExp    tmpExp1;
> +    // Remove category from program-titles
> +	// Have seen "NRK2s historiekveld: Film: bla-bla"
> +    tmpExp1 = m_NRKCategories;
> +    while ((position = tmpExp1.indexIn(event.title)) != -1){
> +		VERBOSE(VB_EIT, LOC + QString("Looping Category match"));
> +		if(tmpExp1.cap(2).length() > 1){
> +			event.title  = tmpExp1.cap(2);
> +			event.description = "(" + tmpExp1.cap(1) + ") " + event.description;
> +			VERBOSE(VB_EIT, LOC + QString("Category ") + tmpExp1.cap(1)  + QString(" removed") );
> +		}
> +    }
> +    // Remove season premiere markings
> +    tmpExp1 = m_NOPremiere;
> +    if ((position = tmpExp1.indexIn(event.title)) >=3){
> +		VERBOSE(VB_EIT, LOC + QString("Removing Premiere tag"));
> +		event.title.remove(m_NOPremiere);
> +    }
> +    // should doctor event.programId to only contain ASCII chars
> +    
> +}
> +
>  /** \fn EITFixUp::FixRTL(DBEventEIT&) const
>   *  \brief Use this to standardise the RTL group guide in Germany.
>   */
> Index: libs/libmythtv/eithelper.cpp
> ===================================================================
> --- libs/libmythtv/eithelper.cpp	(revision 24755)
> +++ libs/libmythtv/eithelper.cpp	(working copy)
> @@ -692,6 +692,10 @@
>      ///////////////////////////////////////////////////////////////////////////
>      // Fixups to make EIT provided listings more useful
>      // transport_id<<32 | netword_id<<16 | service_id
> +  // NRK hos hakon
> +  // mplexid 	sourceid 	transportid 	networkid
> +  // 27  	2  		910	  	8770
> +  // 

remove the comment

>  
>      // Bell Express VU Canada
>      fix[  256U << 16] = EITFixUp::kFixBell;
> @@ -736,6 +740,25 @@
>      fix[40999U << 16 | 1131] = EITFixUp::kFixSubtitle;
>      fix[40999U << 16 | 1068] = EITFixUp::kFixSubtitle;
>      fix[40999U << 16 | 1069] = EITFixUp::kFixSubtitle;
> +    // Norway/NTV
> +    fix[8770U  << 16 | 0x006f] = EITFixUp::kFixNO;  //NRK Folkemusikk

please use the full networkid, transportid, serviceid triple for identifying
single services. Are the services all on a single transport? are there other
services on that transport? Does it maybe make more sense to use kFixNO_NKR
since only NKR services are marked?

> +    fix[8770U  << 16 | 0x0070] = EITFixUp::kFixNO;  //NRK Stortinget
> +    fix[8770U  << 16 | 0x0071] = EITFixUp::kFixNO;  //NRK Super
> +    fix[8770U  << 16 | 0x0072] = EITFixUp::kFixNO;  //NRK Sport
> +    fix[8770U  << 16 | 0x0073] = EITFixUp::kFixNO;  //NRK Gull
> +    fix[8770U  << 16 | 0x0074] = EITFixUp::kFixNO;  //NRK Jazz
> +    fix[8770U  << 16 | 0x0067] = EITFixUp::kFixNO;  //NRK Super / NRK3
> +    fix[8770U  << 16 | 0x0068] = EITFixUp::kFixNO;  //NRK Tegnspr???
> +    fix[8770U  << 16 | 0x0069] = EITFixUp::kFixNO;  //NRK P2
> +    fix[8770U  << 16 | 0x006a] = EITFixUp::kFixNO;  //NRK P3
> +    fix[8770U  << 16 | 0x006b] = EITFixUp::kFixNO;  //NRK Alltid Nyheter
> +    fix[8770U  << 16 | 0x006c] = EITFixUp::kFixNO;  //NRK mP3
> +    fix[8770U  << 16 | 0x006d] = EITFixUp::kFixNO;  //NRK Klassisk
> +    fix[8770U  << 16 | 0x006e] = EITFixUp::kFixNO;  //NRK S???i Radio
> +    fix[8770U  << 16 | 0x0066] = EITFixUp::kFixNO;  //NRK2
> +    fix[8770U  << 16 | 0x03f0] = EITFixUp::kFixNO;  //NRK1 M???e og Romsdal
> +    fix[8770U  << 16 | 0x0455] = EITFixUp::kFixNO;  //NRK P1 Tr???delag
> +    fix[8770U  << 16 | 0x03f1] = EITFixUp::kFixNO;  //NRK1 Midtnytt
>  
>      // Australia
>      fix[ 4096U << 16] = EITFixUp::kFixAUStar;
> Index: libs/libmythtv/eitfixup.h
> ===================================================================
> --- libs/libmythtv/eitfixup.h	(revision 24755)
> +++ libs/libmythtv/eitfixup.h	(working copy)
> @@ -45,10 +45,11 @@
>          kFixPremiere   = 0x0400,
>          kFixHDTV       = 0x0800,
>          kFixNL         = 0x1000,
> +        kFixNO         = 0x2000,

no need order, just continue with the next free bit

>  
>          // Early fixups
> -        kEFixForceISO8859_1  = 0x2000,
> -        kEFixForceISO8859_15 = 0x4000,
> +        kEFixForceISO8859_1  = 0x4000,
> +        kEFixForceISO8859_15 = 0x8000,
>      };
>  
>      EITFixUp();
> @@ -77,6 +78,10 @@
>                     bool parse_subtitle) const;      // Sweden DVB-C
>      void FixAUStar(DBEventEIT &event) const;        // Australia DVB-S
>      void FixMCA(DBEventEIT &event) const;           // MultiChoice Africa DVB-S
> +    void FixNO(DBEventEIT &event) const;            // Norway, only NRK for now
> +    void FixTitleQuote(DBEventEIT &event) const;    // Remove quotes from title/subtitle 
> +    void FixColonSubtitle(DBEventEIT &event) const; // Norway, only NRK for now
> +    void FixNRKSeries(DBEventEIT &event) const;     // Norway, only NRK for now
>      void FixRTL(DBEventEIT &event) const;           // RTL group DVB
>      void FixFI(DBEventEIT &event) const;            // Finland DVB-T
>      void FixPremiere(DBEventEIT &event) const;      // german pay-tv Premiere
> @@ -170,6 +175,10 @@
>      const QRegExp m_nlDirector;
>      const QRegExp m_nlCat;
>      const QRegExp m_nlOmroep;
> +    const QRegExp m_ColonSubtitle;
> +    const QRegExp m_NRKCategories;
> +    const QRegExp m_NOPremiere;
> +    const QRegExp m_NRKPartOfSeries;
>  };
>  
>  #endif // EITFIXUP_H


Janne


More information about the mythtv-dev mailing list