[FFmpeg-devel] [PATCH] avformat/assenc: Don't truncate lines to 4095 characters

[Paul B Mahol]

Please send such patch.

On Sun, Aug 4, 2019 at 5:41 PM Marton Balint <cus at passwd.hu> wrote:

>
>
> On Sun, 4 Aug 2019, Paul B Mahol wrote:
>
> > On Sun, Aug 4, 2019 at 2:58 PM Marton Balint <cus at passwd.hu> wrote:
> >
> >>
> >>
> >> On Sun, 4 Aug 2019, Paul B Mahol wrote:
> >>
> >> > On Sun, Aug 4, 2019 at 12:54 PM Marton Balint <cus at passwd.hu> wrote:
> >> >
> >> >>
> >> >>
> >> >> On Sat, 3 Aug 2019, Tellow Krinkle wrote:
> >> >>
> >> >> > Fixes #6390
> >> >> >
> >> >> > Sample file for new test:
> >> >>
> >>
> https://gist.githubusercontent.com/tellowkrinkle/d6a6e328f892dbbacc000ad9c3890644/raw/4f68e56b1f0fab594aae040723722af4f5161a02/longline.ass
> >> >> >
> >> >> > Signed-off-by: Tellow Krinkle <tellowkrinkle at gmail.com>
> >> >> > ---
> >> >> > libavformat/assenc.c     | 4 +++-
> >> >> > tests/fate/subtitles.mak | 4 ++++
> >> >> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >> >> >
> >> >> > diff --git a/libavformat/assenc.c b/libavformat/assenc.c
> >> >> > index d50f18feb1..9b44b16597 100644
> >> >> > --- a/libavformat/assenc.c
> >> >> > +++ b/libavformat/assenc.c
> >> >> > @@ -95,7 +95,9 @@ static void purge_dialogues(AVFormatContext *s,
> int
> >> >> force)
> >> >> >                    ass->expected_readorder, dialogue->readorder);
> >> >> >             ass->expected_readorder = dialogue->readorder;
> >> >> >         }
> >> >> > -        avio_printf(s->pb, "Dialogue: %s\r\n", dialogue->line);
> >> >> > +        avio_write(s->pb, "Dialogue: ", 10);
> >> >> > +        avio_write(s->pb, dialogue->line, strlen(dialogue->line));
> >> >> > +        avio_write(s->pb, "\r\n", 2);
> >> >>
> >> >> IMHO avio_printf should be fixed instead to use an AVBPrint buffer,
> this
> >> >> should remove the 4k limit from it.
> >> >>
> >> >
> >> > That is unrelated issue. Using avio_printf in this specific case is
> >> > overkill.
> >>
> >> Using 3 avio_writes instead is a useless micro optimalization which also
> >> reduces readblity, but feel free to apply if you feel stong about it.
> >>
> >
> > I doubt so, make actual benchmark to prove your claims.
> > Allocation + memcpy is slower than direct writing.
>
> And considering the size of ASS files (a few hundred KB) it will not be
> measureable overall, only if you benchmark the actual function.
>
> If you are really concerened, we could add a function like this:
>
> int avio_print_n_strings(AVIOContext *s, int nb_strings, ...)
> {
>      va_list ap;
>      int ret = 0;
>
>      va_start(ap, nb_strings);
>      for (int i = 0; i < nb_strings; i++) {
>          const char *str = va_arg(ap, const char *);
>          int len = strlen(str);
>          avio_write(s, (const unsigned char *)str, len);
>          ret += len;
>      }
>      va_end(ap);
>
>      return ret;
> }
>
> /**
>   * Write nb_strings number of strings (const char *) to the context.
>   */
> #define avio_print(s, ...) avio_print_n_strings(s, sizeof((const
> char*[]){__VA_ARGS__}) / sizeof(const char*), __VA_ARGS__);
>
> So from ASSenc you could use:
>
> avio_print(s->pb, "Dialogue: ", dialogue->line, "\r\n");
>
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".