[MPlayer-dev-eng] [PATCH] Add support for ARIB STD-B24 captions v2

[Uoti Urpala]

On Mon, 2009-10-26 at 10:32 -0700, Michael Wu wrote:
> --- On Mon, 10/26/09, Uoti Urpala <uoti.urpala at pp1.inet.fi> wrote:
> > +#define arib_stub(...) mp_msg(MSGT_ARIB, MSGL_WARN, __VA_ARGS__)
> > 
> > +         arib_stub("cc: APB\n");
> > 
> > Messages like these will confuse most users...
> > 
> > 
> I'll just convert everything to dbg then. The spec is quite big and I'm not sure which parts are actually used in real streams, so these messages are set up in the hopes that users will report them if they find streams that uses those features. However, I think I've implemented enough of the spec that most users won't end up noticing or caring about these messages. I should be playing enough arib ts's to find the remaining issues anyway..

I think it'd be OK to have them at warning level, but the message should
have enough detail to tell it is about missing features in the arib
decoder. Just "cc: APB" is meaningless if you don't already know what
those messages are about.

> > +#ifdef CONFIG_ASS
> > +               
> >                
> >            
> >    sub->ass_track =
> > ass_default_track(ass_library);
> > +#endif
> > 
> > This doesn't belong in an individual demuxer, especially
> > when using
> > libass at all is in implementation detail of the decoder.
> > Current
> > MPlayer code is somewhat ugly in this area too, but
> > allocating the track
> > in demuxer.c where the other ass_track variables are
> > allocated would at
> > least be better than this.
> > 
> I couldn't use the current ass_track allocation code in demuxer.c because it assumes detection at open, whereas demux_ts.c is much more likely to detect a subtitle stream at a later point. I don't feel too strongly about this since having any specific subtitle decoder handling in libmpdemux is pretty bad, but I'll try to move the libass stuff to demuxer.c.

Yes there should be a separate "open subtitle codec" step. However
letting the code spread into individual demuxers now will make it harder
to clean things up later, and it also means that possible arib support
for the lavf mpeg demuxer will require separate changes. I'm not sure
whether there's any particularly nice solution for this case though (at
least without cleaning up some of the existing MPlayer code).

BTW your mailer linewraps quotes in a manner that totally mangles them,
while at the same time your own comments aren't wrapped at all and are
on a single long line.