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

[Diego Biurrun]

I repeat: Get rid of all the tabs.

On Mon, Oct 26, 2009 at 10:21:57PM -0400, Michael Wu wrote:
> >> --- libmpdemux/demux_ts.c	(revision 29794)
> >> +++ libmpdemux/demux_ts.c	(working copy)
> >> @@ -38,6 +38,11 @@
> >>
> >> +#ifdef CONFIG_ASS
> >> +#include "libass/ass.h"
> >> +#include "libass/ass_mp.h"
> >> +#endif
> >
> > Is this #ifdef necessar?
> >
> Yes. All uses of ass.h I've seen so far are inside #ifdef CONFIG_ASS.

They're likely all wrong.

> >> --- configure	(revision 29794)
> >> +++ configure	(working copy)
> >> @@ -6150,6 +6154,30 @@
> >>
> >> +if test "$_arib" = yes ; then
> >> +    def_arib='#define CONFIG_ARIB'
> >> +else
> >> +    def_arib='#undef CONFIG_ARIB'
> >> +fi
> >
> > This can be simplified if you first unset CONFIG_ARIB.
> >
> > Also, set CONFIG_ARIB to 1.
> >
> I just used the standard code used in other feature tests.

Improve upon it instead.

> >> --- sub_arib.c	(revision 0)
> >> +++ sub_arib.c	(revision 0)
> >> @@ -0,0 +1,796 @@
> >> +/* sub_arib.c
> >> + * Caption decoder for ARIB STD-B24
> >> + *
> >> + * Copyright 2009, Michael Wu <altape at ymail.com>
> >> + *
> >> + * sub_arib is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published by
> >> + * the Free Software Foundation; either version 2 of the License, or
> >> + * (at your option) any later version.
> >
> > Use our standard license boilerplate.
> >
> Which is? I just checked three files and none of them had a license.

You had bad luck while checking.  Try av_opts.c instead.

> >> --- mp_msg.h	(revision 29794)
> >> +++ mp_msg.h	(working copy)
> >> @@ -104,6 +104,8 @@
> >>
> >>  #define MSGT_STATUSLINE 45 // playback/encoding status line
> >>
> >> +#define MSGT_ARIB 46 // sub_arib.c
> >
> > Do we really need a new message type?  Is there no subtitle-related one
> > that you can use?
> >
> Please suggest one if you think there is a subtitle related one that I
> should use. I looked and I didn't see any that I thought were appropriate.

MSGT_SUBREADER or add MSGT_SUBTITLE.

The following warning from your code looks suspicious:

sub_arib.c: In function 'arib_encode_sjis':
sub_arib.c:420: warning: dereferencing type-punned pointer will break strict-aliasing rules

Diego