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

[Michael Niedermayer]

Hi

On Sat, Jul 21, 2007 at 07:51:47AM +0700, Vladimir Voroshilov wrote:
> 2007/7/19, Vladimir Voroshilov <voroshil at gmail.com>:
> >Hi, Michael
> >
> >2007/7/18, Michael Niedermayer <michaelni at gmx.at>:
> >> Hi
> >>
> >> On Tue, Jul 17, 2007 at 10:19:42PM +0700, Vladimir Voroshilov wrote:
> >> > +
> >> > +    if(p[0].raw<0x80 && p[0].raw>=0x20){
> >> > +        p[0].unicode=lang_chars[lang][p[0].raw-0x20];
> >> > +    }else
> >> > +        p[0].unicode=0x20;
> >> > +}
> >>
> >> conv2uni(unsigned int p, int lang)
> >> {
> >>     if(p-0x20 < 0x80) return lang_chars[lang][p - 0x20];
> >>     else              return 0x20;
> >> }
> >I'm afraid you have an error in "if(p-0x20<0x80).
> >Values over 0x80 are damaged, lower than 0x20 are control chars.
> >Rewrited in similar way.
> >
> >All other  issues (i hope miss nothing) have been fixed as suggested by 
> >you.
> >See attached file.
> 
> Fixed switching teletext on/off
> Debug dump disabled.
> 
[...]

> +static tt_page** ptt_cache;
> +static unsigned char* ptt_cache_first_subpage;

shouldnt these and other be part of the priv_vbi_t context?


[...]
> +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 p raw teletext char to decode
> + * \param lang teletext internal language code (see lang2id)
> + * \return UTF8 char
> + *
> + * \remarks
> + * routine will analyze raw member of given tt_char structure and
> + * fill unicode member of the same struct with apropriate utf8 code.
> + */
> +unsigned int conv2uni(unsigned int p,int lang)
> +{
> +//    assert(lang>=0 && lang<LANGS ); //make sure that we will not get out of bounds error
> +
> +    if(p<0x80 && p>=0x20){
> +        return lang_chars[lang][p-0x20];
> +    }else
> +        return 0x20;
> +}

these functions and others should either get a sensible name
(inlcuding teletext or such) or be static


[...]
> +static int steppage(int p, int direction, int skip_hidden)
> +{
> +    if(skip_hidden)
> +        p=(p&15)+((p>>4)&15)*10+(p>>8)*100;
> +    p+=direction;
> +    if(skip_hidden){
> +        p=(p+800)%800;
> +        p=(p%10)+((p/10)%10)*16+(p/100)*256;
> +    }else
> +        p&=0x7ff;
> +
> +    if(p&0x700)
> +        return p;
> +    else
> +        return p|0x800;
> +}
> +/*
> +------------------------------------------------------------------
> +   Cache stuff
> +------------------------------------------------------------------
> +*/
> +static void put_to_cache(priv_vbi_t* priv,tt_page* pg){
> +    int cache_idx;
> +    tt_page* pgc; //page in cache
> +    int i;
> +
> +    cache_idx=(pg->pagenum&0x7ff)+VBI_MAX_PAGES*pg->subpagenum;
> +    pthread_mutex_lock(&(priv->buffer_mutex));
> +    if(ptt_cache_first_subpage[pg->pagenum]>pg->subpagenum){
> +        ptt_cache_first_subpage[pg->pagenum]=pg->subpagenum;
> +    }

indexing ptt_cache_first_subpage by 0x100-0x8FF wastes space
also why is the internal page number using such a odd range, wouldnt it
make more sense to just do the convertion for display
instead of filling the code with &0x7ff and if/else checks

also
((p+0x700)&0x7FF)+0x100
can be used for the convertion no need to have these if/else all over the code


[...]
> +        for(col=0;col<VBI_COLUMNS;col++){
> +            i=row*VBI_COLUMNS+col;
> +            c=raw[i];
> +            if(c&0x80){ //damaged char
> +              p[i]=tt_error;
> +              p[i].raw=c;
> +              continue;
> +            }

indention is inconsistant


[...]
> +
> +
> +/**
> + * \brief checks whether page is ready and copies it into cache array if so
> + * \param priv private data structure
> + * \param magAddr page's magazine address (0-7)
> + *
> + * Routine also calls decode_page to perform 1st stage of rendering
> + */
> +void store_in_cache(priv_vbi_t* priv, int magAddr){
> +    if (priv->mag[magAddr].order!=23){ //is last row ?
> +        mp_msg(MSGT_TV,MSGL_DBG2,"store_in_cache(%d): pagenum:%x\n",
> +            priv->mag[magAddr].order,
> +            priv->mag[magAddr].pt->pagenum);
> +        return;
> +    }
> +
> +    put_to_cache(priv,priv->mag[magAddr].pt);
> +    priv->curr_pagenum=priv->mag[magAddr].pt->pagenum;

wouldnt it be better to store each row in the cache as it is received
instead of doing it only when the last row is encountere?
think of cases where a row is so damaged that its not recognized properly


[...]
> +    flag%=4;
> +    if(flag<0)
> +        flag+=4;

flag &=3;


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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20070721/4c405852/attachment.pgp>