[MPlayer-dev-eng] [PATCH] Add support for ARIB STD-B24 captions v2
diego at biurrun.de
Tue Oct 27 11:04:01 CET 2009
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
More information about the MPlayer-dev-eng