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

Marton Balint cus at passwd.hu
Sun Aug 4 18:41:06 EEST 2019



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


More information about the ffmpeg-devel mailing list