[MPlayer-dev-eng] [PATCH] Teletext support try3 (1/5, core)

Michael Niedermayer michaelni at gmx.at
Sun Jul 15 16:52:30 CEST 2007


Hi

On Sun, Jul 15, 2007 at 06:01:13PM +0700, Vladimir Voroshilov wrote:
> Hi, All
> 
> Here is current version of teletext patch (w/o zvbi).
> Attached patch contains driver-independent core functionality
> (analyzing raw vbi data, packets, decoding pages, etc)
> 
> 1. Good cache implementation is needed
> 


> 2. There are two routines for converting raw data into bytes.
> 
> decode_raw_line_sine - extracted from Michael Nidermauer's code. It
> can't detect any data. Perhaps error somewhere in code.
> 
> decode_raw_line_runin - got from MythTV project source, based on
> detecting min/max values on clock run-in sequence.

ok you can leave this, ill fix it after its in svn and working


[...]
> + * TODO:
[...]

> + *  is better quality on poor signal possible ?

yes


[...]
> + *  optional libzvbi support

no


[...]

> +/// page magazine entry structure
> +typedef struct _MAG{
> +  tt_page *pt;
> +  int order;
> +  int lang;
> +} MAG;
> +
> +
> +typedef struct {
> +    int            on;            ///< teletext on/off

indention of these 2 structs is inconsistent

> +    int            pgno;          ///< seek page number
> +    int            subno;         ///< seek subpage
> +    int            curr_pgno;     ///< current page number
> +    int            pagenumdec;    ///< set page num with dec

please use consistent abbriviations (no vs. num)


> +
> +    int            tformat;       ///< 0:text, 1:spu
> +    int            tmode;         ///< 0:, 1:bw, 2:gray, 3:color
> +    int            half;          ///< 0:half mode off, 1:top half page, 2:bottom half page

these could be enums



> +    MAG*           mag;           ///< pages magazine (has 8 entities)

> +    iconv_t        icd;           ///< opened iconv  descriptor
> +    char*          input_section; ///< saved section name (for bining keys)
> +    tt_page        display_page;  ///< currently displayed page (with additional info, e.g current time)
> +
> +    int bpb;   /// number of raw bytes between two subsequent encoded bits
> +    /// clock run-in sequence will be searched in buffer in [soc:eoc] bytes range
> +    int soc;
> +    int eoc;
> +    int bp8bl; /// minimum number of raw vbi bytes wich can be decoded into 8 data bits
> +    int bp8bh; /// maximum number of raw vbi bytes wich can be decoded into 8 data bits

it should by ///<
and it could be vertically aligned


[...]
> +static unsigned char corrHamm48[256]={

static const


> +  0x01, 0xff, 0x01, 0x01, 0xff, 0x00, 0x01, 0xff,  0xff, 0x02, 0x01, 0xff, 0x0a, 0xff, 0xff, 0x07, 
> +  0xff, 0x00, 0x01, 0xff, 0x00, 0x00, 0xff, 0x00,  0x06, 0xff, 0xff, 0x0b, 0xff, 0x00, 0x03, 0xff, 
> +  0xff, 0x0c, 0x01, 0xff, 0x04, 0xff, 0xff, 0x07,  0x06, 0xff, 0xff, 0x07, 0xff, 0x07, 0x07, 0x07, 
> +  0x06, 0xff, 0xff, 0x05, 0xff, 0x00, 0x0d, 0xff,  0x06, 0x06, 0x06, 0xff, 0x06, 0xff, 0xff, 0x07, 
> +  0xff, 0x02, 0x01, 0xff, 0x04, 0xff, 0xff, 0x09,  0x02, 0x02, 0xff, 0x02, 0xff, 0x02, 0x03, 0xff, 
> +  0x08, 0xff, 0xff, 0x05, 0xff, 0x00, 0x03, 0xff,  0xff, 0x02, 0x03, 0xff, 0x03, 0xff, 0x03, 0x03, 
> +  0x04, 0xff, 0xff, 0x05, 0x04, 0x04, 0x04, 0xff,  0xff, 0x02, 0x0f, 0xff, 0x04, 0xff, 0xff, 0x07, 
> +  0xff, 0x05, 0x05, 0x05, 0x04, 0xff, 0xff, 0x05,  0x06, 0xff, 0xff, 0x05, 0xff, 0x0e, 0x03, 0xff, 
> +  0xff, 0x0c, 0x01, 0xff, 0x0a, 0xff, 0xff, 0x09,  0x0a, 0xff, 0xff, 0x0b, 0x0a, 0x0a, 0x0a, 0xff, 
> +  0x08, 0xff, 0xff, 0x0b, 0xff, 0x00, 0x0d, 0xff,  0xff, 0x0b, 0x0b, 0x0b, 0x0a, 0xff, 0xff, 0x0b, 
> +  0x0c, 0x0c, 0xff, 0x0c, 0xff, 0x0c, 0x0d, 0xff,  0xff, 0x0c, 0x0f, 0xff, 0x0a, 0xff, 0xff, 0x07, 
> +  0xff, 0x0c, 0x0d, 0xff, 0x0d, 0xff, 0x0d, 0x0d,  0x06, 0xff, 0xff, 0x0b, 0xff, 0x0e, 0x0d, 0xff, 
> +  0x08, 0xff, 0xff, 0x09, 0xff, 0x09, 0x09, 0x09,  0xff, 0x02, 0x0f, 0xff, 0x0a, 0xff, 0xff, 0x09, 
> +  0x08, 0x08, 0x08, 0xff, 0x08, 0xff, 0xff, 0x09,  0x08, 0xff, 0xff, 0x0b, 0xff, 0x0e, 0x03, 0xff, 
> +  0xff, 0x0c, 0x0f, 0xff, 0x04, 0xff, 0xff, 0x09,  0x0f, 0xff, 0x0f, 0x0f, 0xff, 0x0e, 0x0f, 0xff, 
> +  0x08, 0xff, 0xff, 0x05, 0xff, 0x0e, 0x0d, 0xff,  0xff, 0x0e, 0x0f, 0xff, 0x0e, 0x0e, 0xff, 0x0e };

this has trailing whitespace though its not forbidden in mplayer
so this is just a note not a complaint


> +
> +
> +#define LAT_UNI 0
> +#define RUS_UNI 1
> +
> +#define LANGS 2
> +
> +// conversion table for chars 0x20-0x7F (UCS-2LE)
> +// TODO: add another languages
> +unsigned int lang_chars[LANGS][0x60]={
> + {
> +  //Latin
> +  0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,
> +  0x28,0x29,0x2a,0x2b,0x2c,0x2d,0x2e,0x2f,
> +  0x30,0x31,0x32,0x33,0x34,0x35,0x36,0x37,
> +  0x38,0x39,0x3a,0x3b,0x3c,0x3d,0x3e,0x3f,
> +  0x40,0x41,0x42,0x43,0x44,0x45,0x46,0x47,
> +  0x48,0x49,0x4a,0x4b,0x4c,0x4d,0x4e,0x4f,
> +  0x50,0x51,0x52,0x53,0x54,0x55,0x56,0x57,
> +  0x58,0x59,0x5a,0x5b,0x5c,0x5d,0x5e,0x5f,
> +  0x60,0x61,0x62,0x63,0x64,0x65,0x66,0x67,
> +  0x68,0x69,0x6a,0x6b,0x6c,0x6d,0x6e,0x6f,
> +  0x70,0x71,0x72,0x73,0x74,0x75,0x76,0x77,
> +  0x78,0x79,0x7a,0x7b,0x7c,0x7d,0x7e,0x7f
> + },
> + {
> +  //Russian
> +  0x20,0x21,0x22,0x23,0x24,0x25,0x044b,0x27,
> +  0x28,0x29,0x2a,0x2b,0x2c,0x2d,0x2e,0x2f,
> +  0x30,0x31,0x32,0x33,0x34,0x35,0x36,0x37,
> +  0x38,0x39,0x3a,0x3b,0x3c,0x3d,0x3e,0x3f,
> +  0x042e,0x0410,0x0411,0x0426,0x0414,0x0415,0x0424,0x0413,
> +  0x0425,0x0418,0x0419,0x041a,0x041b,0x041c,0x041d,0x041e,
> +  0x041f,0x042f,0x0420,0x0421,0x0422,0x0423,0x0416,0x0412,
> +  0x042c,0x042a,0x0417,0x0428,0x042d,0x0429,0x0427,0x042b,
> +  0x044e,0x0430,0x0431,0x0446,0x0434,0x0435,0x0444,0x0433,
> +  0x0445,0x0438,0x0439,0x043a,0x043b,0x043c,0x043d,0x043e,
> +  0x043f,0x044f,0x0440,0x0441,0x0442,0x0443,0x0436,0x0432,
> +  0x044c,0x044a,0x0437,0x0448,0x044d,0x0449,0x0447,0x044b
> + }
> +};
> +
> +int lang2id (int lang){
> +  switch(lang){
> +  case 3: //Stub. My teletext provider (1TV, Russia) sets this this language code for russian teletext pages
> +          //TODO: make this configurable
> +      return RUS_UNI;
> +  default:
> +      return LAT_UNI;
> +  }
> +}
> +
> +/**
> + * \brief convert chars from curent teletext codepage into MPlayer charset
> + * \param icd opened iconv descriptor
> + * \param p tt_char structure to recode
> + * \param lang teletext internal language code (see lang2id)
> + *
> + * \remarks
> + * routine will analyze raw member of given tt_char structure and
> + * fill unicode member of the same struct with apropriate utf8 code.
> + *
> + */
> +void conv2uni(iconv_t icd,tt_char*p,int lang)
> +{
> +    unsigned int u1,u2;
> +    char* ibuf,*obuf;
> +    int inum,onum;
> +
> +    if (lang>=LANGS) //make sure that we will not get out of bounds error
> +        lang=LAT_UNI;//fall back to latin
> +
> +    if(p[0].raw<0x80 && p[0].raw>=0x20){
> +        u1=lang_chars[lang][p[0].raw-0x20];
> +        u2=0;
> +        inum=2;
> +        onum=4;
> +        ibuf=(char*)&u1;
> +        obuf=(char*)&u2;
> +        if(iconv(icd,&ibuf,&inum,&obuf,&onum)<0)
> +            u2=0x20;
> +    }else
> +        u2=0x20;
> +    p[0].unicode=u2;
> +}

no, remove this or the patch is rejected, converting from teletexts
native encodings to some other encoding and then with iconv to unicode/utf8
is unaccpetable, iconv is not needed!

convert teletexts native encoding to unicode and store with PUT_UTF8() as utf8



> +
> +/**
> + * \brief convert 123 --> 0x123
> + * \param dec decimal value
> + * \return apropriate hexadecimal value
> + *
> + */
> +static inline int vbi_dec2bcd(int dec){
> +    unsigned char c1,c2,c3;
> +    int ret;
> +    c1=dec%10;
> +    c2=(dec/10)%10;
> +    c3=(dec/100)%10;
> +    ret=(c3<<8)|(c2<<4)|c1;
> +    mp_msg(MSGT_TV,MSGL_DBG3,"vbi_dec2bcd: %d -> 0x%03x\n",dec,ret);
> +    return ret;
> +}
> +
> +/**
> + * \brief convert 0x123 --> 123
> + * \param bcd hexadecimal value
> + * \return apropriate decimal value
> + *
> + */
> +static inline int vbi_bcd2dec(int bcd){
> +    unsigned char c1,c2,c3;
> +    int ret;
> + 
> +    c1=(bcd>>8)&0xf;
> +    c2=(bcd>>4)&0xf;
> +    c3=(bcd)&0xf;
> +    
> +    if(c1>9 || c2>9 || c3>9)
> +        return 0;
> +
> +    ret=(c1*100+c2*10+c3);
> +    mp_msg(MSGT_TV,MSGL_DBG3,"vbi_bcd2dec:0x%03x -> %d \n",bcd,ret);
> +    return ret;
> +}
> +
> +/**
> + * \brief calculate increased/decreased by given value page number
> + * \param curr  current page number in hexadecimal for
> + * \param direction decimal value (can be negative) to add to value or curr parameter
> + * \return new page number in hexadecimal form
> + *
> + * VBI page numbers are represented in special hexadecimal form, e.g.
> + * page with number 123 (as seen by user) internally has number 0x123.
> + * and equation 0x123+8 should be equal to 0x131 instead of regular 0x12b.
> + *
> + *
> + * Page numbers 0xYYY (where Y is not belongs to (0..9) and pages below 0x100 and
> + * higher 0x799 are reserved for internal use.
> + *
> + */
> +static int steppage(int curr, int direction)
> +{
> +    int newpage = vbi_dec2bcd(vbi_bcd2dec(curr) + direction);
> +    if (newpage < 0x0)
> +        newpage = 0x799;
> +    if (newpage > 0x799)
> +        newpage = 0x0;
> +    mp_msg(MSGT_TV,MSGL_DBG3,"steppage is called: curr:0x%03x, direction: %d, newpage: 0x%03x\n",curr,direction,newpage);
> +    return newpage;
> +}

ohh my god is this a mess, please use something like the following
(note, its untested)

static int todec(unsigned int p){
    int d= p&15;
    if     (d==15) p-=6;
    else if(d==10) p+=6;
    return p;
}
static int steppage(unsigned int p){
    p= todec(p & 0x7FF);
    p=     (p& 15) + (todec(p>>4)<<4);
    return ((p&255) + (todec(p>>8)<<8)) & 0x7FF;
}


[...]
> +/*
> +------------------------------------------------------------------
> +   Renderer stuff
> +------------------------------------------------------------------
> +*/
> +#ifdef DEBUG_DUMP
> +/**
> + * \brief renders teletext page into given file
> + * \param f opened file descriptor
> + * \param pgno which page to render
> + * \param colored use colors not implementede yet)
> + *
> + * Text will be UCS-2LE encoded

NOOOOOOOOOOOOOOOOOOOOOOOO!

use utf8 with PUT_UTF8()



> + *
> + */
> +void render2ascii(priv_vbi_t* priv,FILE* f,int pgno,int colored){

and dont call it ascii if its not ascii


[...]
> +/**
> + * \brief converts raw teletext page into usefull format (1st rendering stage)
> + * \param priv private data structure
> + * \param pgno which page to decode
> + *
> + * Routine fills tt_char structure of each teletext_page character with proper info about
> + * foreground and background colors, character type (graphics/control/text).
> + *
> + */
> +void decode_page(priv_vbi_t* priv,tt_page* pg,int pgno,int subno)
> +{
> +    int c,gfx=0,lat=0;
> +    int i=0;
> +    int color=0;
> +    int bkg_color=0;
> +    int row,col;
> +    int separated=0;
> +    int lang;
> +    tt_char *p;
> +
> +    if(!pg)
> +        return;

why should this be called with a pg==NULL ?


[...]
> +    for(row=0;row<VBI_ROWS;row++)   {
> +        lat=lang==0?1:0;

?1:0 is superflous


[...]
> +		p[i].ctl=1;
> +		if(p[i].gfx)
> +                    p[i].unicode=0;
> +		else

dont mix tabs and spaces


> +                    p[i].unicode=' ';
> +                continue;
> +            }
> +        
> +            if(gfx){

> +                p[i].unicode=p[i].raw;
> +                p[i].unicode-=0x20;

p[i].unicode= p[i].raw - 0x20;


[...]
> +        priv_vbi->display_page.text[0].unicode='N';
> +        priv_vbi->display_page.text[1].unicode='o';
> +        priv_vbi->display_page.text[2].unicode=' ';
> +        priv_vbi->display_page.text[3].unicode='t';
> +        priv_vbi->display_page.text[4].unicode='e';
> +        priv_vbi->display_page.text[5].unicode='l';
> +        priv_vbi->display_page.text[6].unicode='e';
> +        priv_vbi->display_page.text[7].unicode='t';
> +        priv_vbi->display_page.text[8].unicode='e';
> +        priv_vbi->display_page.text[9].unicode='x';
> +        priv_vbi->display_page.text[10].unicode='t';

strcpy(), memcpy() or something like that with "No teletext"


[...]
> +	if((priv_vbi->pgno>>8)&0xf)

priv_vbi->pgno & 0xf00


> +            priv_vbi->display_page.text[5].unicode='0'+((priv_vbi->pgno>>8)&0xf);
> +        else
> +            priv_vbi->display_page.text[5].unicode='8';

> +        priv_vbi->display_page.text[6].unicode='0'+((priv_vbi->pgno>>4)&0xf);
> +        priv_vbi->display_page.text[7].unicode='0'+((priv_vbi->pgno)&0xf);
> +        priv_vbi->display_page.text[8].unicode='.';
> +        priv_vbi->display_page.text[9].unicode='0'+((priv_vbi->subno>>4)&0xf);
> +        priv_vbi->display_page.text[10].unicode='0'+((priv_vbi->subno)&0xf);
> +        priv_vbi->display_page.text[11].unicode=' ';

s(n)printf with "%X.%X" or something like that


[...]
> +/**
> + * \brief decode packets 1..24 (teletext page header)
> + * \param priv private data structure 
> + * \param data raw teletext data
> + * \param magAddr teletext page's magazine address
> + * \param rowAddr teletext page's row number
> + *
> + * 
> + * \remarks 
> + * data buffer was shifted by 6 and now contains 40 bytes of display data:
> + * this type of packet is not proptected by Hamm 8/4 code
> + *
> + */
> +void decode_pkt_page(priv_vbi_t* priv,unsigned char*data,int magAddr,int rowAddr){
> +    int i,err;
> +    if (!priv->mag[magAddr].pt)
> +        return;
> +
> +    priv->mag[magAddr].order=rowAddr;
> +
> +    err=0;
> +    for(i=0; i<VBI_COLUMNS; i++){
> +        data[i]= fixParity[ data[i] ];
> +        if( data[i]&0x80){ //HammError
> +	    err++;
> +            priv->mag[magAddr].pt->text[i+rowAddr*VBI_COLUMNS].raw='?';
> +        }else
> +            priv->mag[magAddr].pt->text[i+rowAddr*VBI_COLUMNS].raw=data[i];
> +    }
> +    pll_add(priv,1,err);
> +
> +    store_in_cache(priv,magAddr);
> +}

you should copy only _UNDAMAGED_ chars, that is ones with
correct parity bit into the cache, the others should be left as they where
before in the cache or '?' if the page was not in the cache

also color/graphic char decoding MUST be done after the cache otherwise
you cannot recover from damage in chars which affect the meaning of
following chars


[...]
> + * flag:
> + * 0 - off
> + * 1 - on & opaque
> + * 2 - on & transparent
> + * 3 - on & transparent  with black foreground color (only in bw mode)

use enum or define dont just document the meaning in some comment


[...]
> +    if(priv_vbi->on==0 && priv_vbi->input_section!=NULL) {

the != NULL is superflous


[...]
> +    case TV_VBI_CONTROL_SET_PAGE:
> +    {
> +        int val=*(int *) arg;
> +        if(val<0 || val>0x799)
> +            return TVI_CONTROL_FALSE;
> +        pthread_mutex_lock(&(priv_vbi->buffer_mutex));
> +        priv_vbi->pgno = steppage(0, val);

this is wrong, if the user asks for page =0x7AB then you should return
that page and not another page

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20070715/d6644c72/attachment.pgp>


More information about the MPlayer-dev-eng mailing list