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

[Alban Bedel]

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.

	Albeu