[MPlayer-dev-eng] [PATCH] Libass BOM fix

[Ivan Kalvachev]

2007/10/28, Ulion <ulion2002 at gmail.com>:
> 2007/10/28, Ivan Kalvachev <ikalvachev at gmail.com>:
> > 2007/10/28, Ulion <ulion2002 at gmail.com>:
> > > 2007/10/28, Ivan Kalvachev <ikalvachev at gmail.com>:
> > > > 2007/10/27, Ulion <ulion2002 at gmail.com>:
> > > > > 2007/10/27, Ivan Kalvachev <ikalvachev at gmail.com>:
> > > > > > 2007/10/27, Ulion <ulion2002 at gmail.com>:
> > > > > > > 2007/10/27, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > > > > > > Hello,
> > > > > > > > On Sat, Oct 27, 2007 at 10:01:29PM +0800, Ulion wrote:
> > > > > > > > > A memory leak bug found in sub_recode, my fix is here:
> > > > > > > >
> > > > > > > > Better don't send patches for new issues as replies.
> > > > > > > > Also if you send them inline take care your mailer doesn't wrap lines.
> > > > > > >
> > > > > > > Sorry for that. I sent it inline to be noticed directly, it's just a
> > > > > > > notifaction.
> > > > > > >
> > > > > > > > And attached patch seems simpler (and would allow to remove the later
> > > > > > > > #ifdef HAVE_ENCA).
> > > > > > > > Note that it is untested though.
> > > > > > >
> > > > > > > Your one is simper for resolve this little leak.
> > > > > > > But as ivan suggested, maybe we should get rid of all strdup stuff for
> > > > > > > enca, I think we should wait for his patch.
> > > > > >
> > > > > > Here it is.
> > > > > > Have in mind there is something fishy with it. It compiled and run
> > > > > > from the first time.
> > > > > > According to Murphy's law, it means that the concept is flowed. ;)
> > > > > >
> > > > >
> > > > > Variable tmp may removed since it's not used any more.
> > > >
> > > > It is used.
> > > > The ENCS_CS_UNKNOWN is valid to enca_charset_name() and it won't
> > > > return NULL. (at least this is how I understand
> > > > http://trific.ath.cx/software/enca/libenca/libenca-Charsets-and-Surfaces/#enca-charset-name
> > > >  )
> > > >
> > > > Of course I can move the check before the enca_charset_name(), but
> > > > then I'd have to check for NULL again later, probably at the end
> > > > (using the else).
> > > > This clean up is not really related to the strdup removal, so I prefer
> > > > to do it in 2 steps.
> > > > First the patch I posted earlier and then the clean-up variable patch
> > > > I attach now.
> > >
> > > About the step 2, you check ENCS_CS_UNKNOWN first, and after
> > > enca_charset_name() check NULL. It need not check for NULL 'again',
> > > only once.
> >
> > Be so good to read the patch. I believe I've done so.
> > However this prevents the detected enca message to appear for each
> > language that is detected (some kind of debugging I guess).
>
> Sorry for didn't read it. it looks good. Only one language is
> prefered, so the patch will change nothing about the message.
>
> >
> > > Another place I think should fix is the end of the loop, should put a
> > > break there.
> >
> > I'll let you do the commit for that. The break is missing in the old
> > code and I am not sure if this is bug or it is intensional.
>
> Missing the break will affect nothing, add it just to make it clear
> that only one language is matched. OK, I will make up the break one
> day after your patch applied.

First patch committed.

Yes you are right about the break, it won't change anything as now we
can check only for one language. I'll committing it together with my
changes.

I wonder how it would be best to support multiple languages...