| Message ID | 20170703114343.4141-1-nfxjfg@googlemail.com |
|---|---|
| State | Accepted |
| Commit | f605b56ad9c5965792359e5474238e318aed1399 |
| Headers | show |
On Mon, Jul 03, 2017 at 01:43:43PM +0200, wm4 wrote: > Some .srt files use this tag. > > (An alternative implementation would be correctly ignoring unknown tags, > and treating them as whitespace. libass can do automatic line wrapping.) > --- > libavcodec/htmlsubtitles.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c > index be5c9316ca..fe991678d5 100644 > --- a/libavcodec/htmlsubtitles.c > +++ b/libavcodec/htmlsubtitles.c > @@ -167,6 +167,8 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in) > } > } else if (tagname[0] && !tagname[1] && strspn(tagname, "bisu") == 1) { > av_bprintf(dst, "{\\%c%d}", tagname[0], !tag_close); > + } else if (!strcmp(tagname, "br")) { > + av_bprintf(dst, "\\N"); > } else { > unknown = 1; > snprintf(tmp, sizeof(tmp), "</%s>", tagname); So this supports <br>, < br > and <br/>? Which makes me think we should use (av_)strcasecmp instead. I think there is a patch for this on the ml (which you may want to apply before this one). I assume you tested fate-subtitles.
On Mon, 3 Jul 2017 13:54:50 +0200 Clément Bœsch <u@pkh.me> wrote: > On Mon, Jul 03, 2017 at 01:43:43PM +0200, wm4 wrote: > > Some .srt files use this tag. > > > > (An alternative implementation would be correctly ignoring unknown tags, > > and treating them as whitespace. libass can do automatic line wrapping.) > > --- > > libavcodec/htmlsubtitles.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c > > index be5c9316ca..fe991678d5 100644 > > --- a/libavcodec/htmlsubtitles.c > > +++ b/libavcodec/htmlsubtitles.c > > @@ -167,6 +167,8 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in) > > } > > } else if (tagname[0] && !tagname[1] && strspn(tagname, "bisu") == 1) { > > av_bprintf(dst, "{\\%c%d}", tagname[0], !tag_close); > > + } else if (!strcmp(tagname, "br")) { > > + av_bprintf(dst, "\\N"); > > } else { > > unknown = 1; > > snprintf(tmp, sizeof(tmp), "</%s>", tagname); > > So this supports <br>, < br > and <br/>? No idea. > Which makes me think we should use (av_)strcasecmp instead. I think there > is a patch for this on the ml (which you may want to apply before this > one). The font tag also uses strcmp(), so that's orthogonal. > I assume you tested fate-subtitles. > Yes.
On Mon, Jul 03, 2017 at 01:54:50PM +0200, Clément Bœsch wrote: > On Mon, Jul 03, 2017 at 01:43:43PM +0200, wm4 wrote: > > Some .srt files use this tag. > > > > (An alternative implementation would be correctly ignoring unknown tags, > > and treating them as whitespace. libass can do automatic line wrapping.) > > --- > > libavcodec/htmlsubtitles.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c > > index be5c9316ca..fe991678d5 100644 > > --- a/libavcodec/htmlsubtitles.c > > +++ b/libavcodec/htmlsubtitles.c > > @@ -167,6 +167,8 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in) > > } > > } else if (tagname[0] && !tagname[1] && strspn(tagname, "bisu") == 1) { > > av_bprintf(dst, "{\\%c%d}", tagname[0], !tag_close); > > + } else if (!strcmp(tagname, "br")) { > > + av_bprintf(dst, "\\N"); > > } else { > > unknown = 1; > > snprintf(tmp, sizeof(tmp), "</%s>", tagname); > > So this supports <br>, < br > and <br/>? i dont think this supports <br/> from a quick look [...]
On Mon, 3 Jul 2017 17:57:06 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Mon, Jul 03, 2017 at 01:54:50PM +0200, Clément Bœsch wrote: > > On Mon, Jul 03, 2017 at 01:43:43PM +0200, wm4 wrote: > > > Some .srt files use this tag. > > > > > > (An alternative implementation would be correctly ignoring unknown tags, > > > and treating them as whitespace. libass can do automatic line wrapping.) > > > --- > > > libavcodec/htmlsubtitles.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c > > > index be5c9316ca..fe991678d5 100644 > > > --- a/libavcodec/htmlsubtitles.c > > > +++ b/libavcodec/htmlsubtitles.c > > > @@ -167,6 +167,8 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in) > > > } > > > } else if (tagname[0] && !tagname[1] && strspn(tagname, "bisu") == 1) { > > > av_bprintf(dst, "{\\%c%d}", tagname[0], !tag_close); > > > + } else if (!strcmp(tagname, "br")) { > > > + av_bprintf(dst, "\\N"); > > > } else { > > > unknown = 1; > > > snprintf(tmp, sizeof(tmp), "</%s>", tagname); > > > > So this supports <br>, < br > and <br/>? > > i dont think this supports <br/> from a quick look Tried < br /> and < br / >, both turn out correctly. Pushed. Unfortunately I just noticed <br/> does not work. Well, whatever.
diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c index be5c9316ca..fe991678d5 100644 --- a/libavcodec/htmlsubtitles.c +++ b/libavcodec/htmlsubtitles.c @@ -167,6 +167,8 @@ int ff_htmlmarkup_to_ass(void *log_ctx, AVBPrint *dst, const char *in) } } else if (tagname[0] && !tagname[1] && strspn(tagname, "bisu") == 1) { av_bprintf(dst, "{\\%c%d}", tagname[0], !tag_close); + } else if (!strcmp(tagname, "br")) { + av_bprintf(dst, "\\N"); } else { unknown = 1; snprintf(tmp, sizeof(tmp), "</%s>", tagname);