On Tue, Jun 13, 2017 at 01:19:56PM +0200, wm4 wrote:
> On Tue, 13 Jun 2017 00:01:04 +0200
> Michael Niedermayer <mich...@niedermayer.cc> wrote:
>
> > Suggested-by: wm4
> > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> > ---
> > libavcodec/htmlsubtitles.c | 44
> > ++++++++++++++++++++++++++------------------
> > 1 file changed, 26 insertions(+), 18 deletions(-)
> >
> > diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
> > index 70311c66d5..be5c9316ca 100644
> > --- a/libavcodec/htmlsubtitles.c
> > +++ b/libavcodec/htmlsubtitles.c
> > @@ -51,6 +51,30 @@ static void rstrip_spaces_buf(AVBPrint *buf)
> > buf->str[--buf->len] = 0;
> > }
> >
> > +/* skip all {\xxx} substrings except for {\an%d}
> > + and all microdvd like styles such as {Y:xxx} */
> > +static void handle_open_brace(AVBPrint *dst, const char **inp, int *an,
> > int *closing_brace_missing)
> > +{
> > + int len = 0;
> > + const char *in = *inp;
> > +
> > + *an += sscanf(in, "{\\an%*1u}%n", &len) >= 0 && len > 0;
> > +
> > + if (!*closing_brace_missing) {
> > + if ( (*an != 1 && in[1] == '\\')
> > + || (in[1] && strchr("CcFfoPSsYy", in[1]) && in[2] == ':')) {
> > + char *bracep = strchr(in+2, '}');
> > + if (bracep) {
> > + *inp = bracep;
> > + return;
> > + } else
> > + *closing_brace_missing = 1;
> > + }
> > + }
> > +
> > + av_bprint_chars(dst, *in, 1);
> > +}
> > +
> > int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in)
> > {
> > char *param, buffer[128], tmp[128];
> > @@ -80,24 +104,8 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst,
> > const char *in)
> > if (!line_start)
> > av_bprint_chars(dst, *in, 1);
> > break;
> > - case '{': /* skip all {\xxx} substrings except for {\an%d}
> > - and all microdvd like styles such as {Y:xxx} */
> > - len = 0;
> > - an += sscanf(in, "{\\an%*1u}%n", &len) >= 0 && len > 0;
> > -
> > - if (!closing_brace_missing) {
> > - if ( (an != 1 && in[1] == '\\')
> > - || (in[1] && strchr("CcFfoPSsYy", in[1]) && in[2] ==
> > ':')) {
> > - char *bracep = strchr(in+2, '}');
> > - if (bracep) {
> > - in = bracep;
> > - break;
> > - } else
> > - closing_brace_missing = 1;
> > - }
> > - }
> > -
> > - av_bprint_chars(dst, *in, 1);
> > + case '{':
> > + handle_open_brace(dst, &in, &an, &closing_brace_missing);
> > break;
> > case '<':
> > tag_close = in[1] == '/';
>
> Looked at the an thing again (at the old code in git master). It makes
> no sense to me - skipping should always behave the same.
>
> Maybe it's actually a bug, or it attempted to emit subsequent an tags
> by trying to be overly clever.
>
> In theory, only 1 an tag is ok per line, so "{\an1}{\an2}" the second
> an tag would either do nothing, or override the first tag. Maybe
> someone thought it'd be reasonable to skip the an second tag, but I
> don't see how it matters.
>
> It could also be an attempt to get the same behavior between libass and
> VSFitler between redundant an tags (maybe VSFilter uses the value of
> the first tag, while libass uses the second tag). Then I'd say this
> should be fixed in libass instead.
>
> Anyway, I'd remove that extra an handling. It's a good example of
> overly complicated code that makes everything less readable for
> absolutely no reason. (Unless I'm overlooking something obvious.)ok, ive sent a patch that does that > > Rest LGTM. will apply the 3 patches in a day or 2 unless there are more comments thx [...9 -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
