[MPlayer-dev-eng] [PATCH] Teletext support try3 (4/5, configure, slaves, etc)

[Vladimir Voroshilov]

2007/7/15, Alban Bedel <albeu at free.fr>:
> On Sun, 15 Jul 2007 21:13:21 +0700
> "Vladimir Voroshilov" <voroshil at gmail.com> wrote:
>
> > +/*
> > +  TELETEXT controls (through tv_teletext_control() )
> > +   NOTE:
> > +    _SET_ should be _GET_ +1
> > +   _STEP_ should be _GET_ +2
> > +*/
>
> Going that route you should perhaps use defines to construct the values
> instead of using explicit +1 and +2.
>
> > +#ifdef HAVE_TV_TELETEXT
> > +        {"tdevice", &tv_param_tdevice, CONF_TYPE_STRING, 0, 0, 0, NULL},
> > +        {"tformat", &tv_param_tformat, CONF_TYPE_STRING, 0, 0, 0, NULL},
> > +        {"tpage", &tv_param_tpage, CONF_TYPE_INT, CONF_RANGE, 100, 899, NULL},
> > +#endif
>
> Still wrong, the surrounding code use tabs.
>
> > +    default:
> > +        return M_PROPERTY_NOT_IMPLEMENTED;
> > +    }
> > +mp_msg(MSGT_TV,MSGL_ERR,"0x%x\n",base_ioctl);
>
> Look like some left over.
>
> > +    // this line could be reached only for SET and STEP properties with DEMUXER_TYPE_TV
> > +    if(base_ioctl==TV_VBI_CONTROL_GET_MODE){
> > +        if(tvh->functions->control(tvh->priv, base_ioctl, &val)==TVI_CONTROL_TRUE && val)
> > +            mp_input_set_section("teletext");
> > +        else
> > +            mp_input_set_section("tv");
> > +    }
>
> This is a bit ugly and with such approch PRINT will also need to
> switch on base_ioctl. In fact I was more thinking of simply having a
> function that take the needed VBI_CONTROL as extra argument. Then each
> property use its own function where special stuff like this can be
> implemented.
>
> BTW imho it would be better if turning the teletext on and off, and
> setting the rendering mode where two different control. Dunno what
> others think about it.

I didn't undestant your idea clearly.
Could you provide small example?

-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719