[MPlayer-dev-eng] [PATCH] Add DVB teletext support

[Francesco Lavra]

On Sat, 2009-11-07 at 17:07 +0100, Reimar Döffinger wrote:
> The C99 type is called uint8_t, not u_int8_t.
> Also just just ff_reverse from libavcodec or of that's too hackish
> ask FFmpeg to move it to libavutil and export it.

Waiting for the relevant FFmpeg patch to reviewed/applied...
In the meantime, hope you don't mind if I post a "temporary" patch which
doesn't use ff_reverse, so you can play with it.

On Sat, 2009-11-07 at 12:33 +0000, Carl Eugen Hoyos wrote: 
> Francesco Lavra <francescolavra <at> interfree.it> writes:
> 
> > In attachment the updated patches, in the first one a hunk has been
> > added to make the above change.
> 
> I applied the separation of teletext from tv.
> The remaining patch makes mplayer crash when exiting from tv://, are you able to
> test this?

In my region there is no analog TV broadcast, so I'm not able to
reproduce that crash, but I'm pretty sure I've found and corrected the
bug which caused it: it was teletext_control() being called with a
pointer to a structure already freed.

On Sat, 2009-11-07 at 13:50 +0100, Reimar Döffinger wrote: 
> > +    /* Reverse bit order, skipping the first two bytes (field parity, line
> > +       offset and framing code). */
> > +    for (i = 0; i < sizeof(data); i++)
> > +        data[i] = byterev8[buf[2 + i]];
> > +
> > +    vbi_decode_line(priv, data);
> 
> Can't this be done in-place, i.e.
> 
> for (i = 2; i < 44; i++)
>     buf[i] = byterev8[buf[i]];
> vbi_decode_line(priv, buf + 2);
> 
> Maybe it is better not to, but then buf should be declared const.

buf declared const

> > +    if (priv->cache_reset){
> > +        pthread_mutex_lock(&(priv->buffer_mutex));
> > +        priv->cache_reset--;
> > +        pthread_mutex_unlock(&(priv->buffer_mutex));
> 
> useless ()

removed ()

> > Index: mpcommon.c
> > ===================================================================
> > --- mpcommon.c	(revision 29843)
> > +++ mpcommon.c	(working copy)
> > @@ -141,9 +141,17 @@
> >  
> >          if (spudec_changed(vo_spudec))
> >              vo_osd_changed(OSDTYPE_SPU);
> > -    } else if (dvdsub_id >= 0 && (type == 't' || type == 'm' || type == 'a')) {
> > +    } else if (dvdsub_id >= 0 && (type == 't' || type == 'm' || type == 'a' || type == 'd')) {
> >          double curpts = refpts + sub_delay;
> >          double endpts;
> > +        if (type == 'd' && !d_dvdsub->demuxer->teletext) {
> > +            tt_stream_props tsp;
> > +            memset(&tsp, 0, sizeof(tsp));
> > +            d_dvdsub->demuxer->teletext = &tsp;
> 
> WTF? You can't assign the address of a stack variable to a globally
> available pointer.

Actually that code is perfectly safe, because the address of
d_dvdsub->demuxer->teletext is passed to teletext_control(), which first
reads the contents for the structure pointed to by it, then modifies the
argument to point to globally available data; I just didn't want to
declare an extra pointer variable. But I admit this code is not nice to
see, so I changed it.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dvb_teletext.patch
Type: text/x-patch
Size: 7204 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20091108/ec7a253a/attachment.bin>