Message ID | 20211217215155.19805-1-michael@niedermayer.cc |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/4] avcodec/ass: Faster ff_ass_add_rect() | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
Michael Niedermayer: > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/ass.c | 32 ++++++++++++++++++++++++++------ > libavcodec/ass.h | 7 +++++++ > 2 files changed, 33 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/ass.c b/libavcodec/ass.c > index 725e4d42ba1..d0a4d678bb4 100644 > --- a/libavcodec/ass.c > +++ b/libavcodec/ass.c > @@ -114,17 +114,30 @@ char *ff_ass_get_dialog(int readorder, int layer, const char *style, > speaker ? speaker : "", text); > } > > -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, > int readorder, int layer, const char *style, > - const char *speaker) > + const char *speaker, size_t *nb_rect_allocated) > { > - AVSubtitleRect **rects, *rect; > + AVSubtitleRect **rects = sub->rects, *rect; > char *ass_str; > + uint64_t new_nb = 0; > > - rects = av_realloc_array(sub->rects, sub->num_rects+1, sizeof(*sub->rects)); > - if (!rects) > + if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { > + new_nb = sub->num_rects + sub->num_rects/16LL + 1; > + } else if (!nb_rect_allocated) > + new_nb = sub->num_rects + 1LL; > + if (new_nb > SIZE_MAX) > return AVERROR(ENOMEM); AVSubtitle.num_rects is unsigned, so this number should always be bounded by UINT_MAX (and nb_rect_allocated can be a pointer to unsigned, too). (Feels like I should resurrect my old av_fast_realloc_array: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200101132758.4452-1-andreas.rheinhardt@gmail.com/) > - sub->rects = rects; > + > + if (new_nb) { > + rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects)); > + if (!rects) > + return AVERROR(ENOMEM); > + if (nb_rect_allocated) > + *nb_rect_allocated = new_nb; > + sub->rects = rects; > + } > + > rect = av_mallocz(sizeof(*rect)); > if (!rect) > return AVERROR(ENOMEM); > @@ -137,6 +150,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > return 0; > } > > +int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > + int readorder, int layer, const char *style, > + const char *speaker) > +{ > + return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, NULL); > +} > + > void ff_ass_decoder_flush(AVCodecContext *avctx) > { > FFASSDecoderContext *s = avctx->priv_data; > diff --git a/libavcodec/ass.h b/libavcodec/ass.h > index 2c260e4e785..784f46c42c5 100644 > --- a/libavcodec/ass.h > +++ b/libavcodec/ass.h > @@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > int readorder, int layer, const char *style, > const char *speaker); > > +/** > + * Add an ASS dialog to a subtitle. > + */ > +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, > + int readorder, int layer, const char *style, > + const char *speaker, size_t *nb_rect_allocated); > + > /** > * Helper to flush a text subtitles decoder making use of the > * FFASSDecoderContext. >
On Fri, Dec 17, 2021 at 11:15:06PM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/ass.c | 32 ++++++++++++++++++++++++++------ > > libavcodec/ass.h | 7 +++++++ > > 2 files changed, 33 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/ass.c b/libavcodec/ass.c > > index 725e4d42ba1..d0a4d678bb4 100644 > > --- a/libavcodec/ass.c > > +++ b/libavcodec/ass.c > > @@ -114,17 +114,30 @@ char *ff_ass_get_dialog(int readorder, int layer, const char *style, > > speaker ? speaker : "", text); > > } > > > > -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > > +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, > > int readorder, int layer, const char *style, > > - const char *speaker) > > + const char *speaker, size_t *nb_rect_allocated) > > { > > - AVSubtitleRect **rects, *rect; > > + AVSubtitleRect **rects = sub->rects, *rect; > > char *ass_str; > > + uint64_t new_nb = 0; > > > > - rects = av_realloc_array(sub->rects, sub->num_rects+1, sizeof(*sub->rects)); > > - if (!rects) > > + if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { > > + new_nb = sub->num_rects + sub->num_rects/16LL + 1; > > + } else if (!nb_rect_allocated) > > + new_nb = sub->num_rects + 1LL; > > + if (new_nb > SIZE_MAX) > > return AVERROR(ENOMEM); > > AVSubtitle.num_rects is unsigned, so this number should always be > bounded by UINT_MAX (and nb_rect_allocated can be a pointer to unsigned, > too). i had that initially but then wanted to move to better types but missed this, changed it back locally thx [...]
Michael Niedermayer: > On Fri, Dec 17, 2021 at 11:15:06PM +0100, Andreas Rheinhardt wrote: >> Michael Niedermayer: >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/ass.c | 32 ++++++++++++++++++++++++++------ >>> libavcodec/ass.h | 7 +++++++ >>> 2 files changed, 33 insertions(+), 6 deletions(-) >>> >>> diff --git a/libavcodec/ass.c b/libavcodec/ass.c >>> index 725e4d42ba1..d0a4d678bb4 100644 >>> --- a/libavcodec/ass.c >>> +++ b/libavcodec/ass.c >>> @@ -114,17 +114,30 @@ char *ff_ass_get_dialog(int readorder, int layer, const char *style, >>> speaker ? speaker : "", text); >>> } >>> >>> -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, >>> +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, >>> int readorder, int layer, const char *style, >>> - const char *speaker) >>> + const char *speaker, size_t *nb_rect_allocated) >>> { >>> - AVSubtitleRect **rects, *rect; >>> + AVSubtitleRect **rects = sub->rects, *rect; >>> char *ass_str; >>> + uint64_t new_nb = 0; >>> >>> - rects = av_realloc_array(sub->rects, sub->num_rects+1, sizeof(*sub->rects)); >>> - if (!rects) >>> + if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { >>> + new_nb = sub->num_rects + sub->num_rects/16LL + 1; >>> + } else if (!nb_rect_allocated) >>> + new_nb = sub->num_rects + 1LL; >>> + if (new_nb > SIZE_MAX) >>> return AVERROR(ENOMEM); >> >> AVSubtitle.num_rects is unsigned, so this number should always be >> bounded by UINT_MAX (and nb_rect_allocated can be a pointer to unsigned, >> too). > > i had that initially but then wanted to move to better types but missed > this, changed it back locally > Simply changing it back is wrong, because it might happen that the old num_rects is < UINT_MAX with new_nb > UINT_MAX. In this case one should clip new_nb to UINT_MAX. (This case can't happen currently: The loop in ccaption_dec can lead to at most INT_MAX/3 rects; the other two ff_ass_add_rect() can bring this to at most INT_MAX/3 + 2.) - Andreas
diff --git a/libavcodec/ass.c b/libavcodec/ass.c index 725e4d42ba1..d0a4d678bb4 100644 --- a/libavcodec/ass.c +++ b/libavcodec/ass.c @@ -114,17 +114,30 @@ char *ff_ass_get_dialog(int readorder, int layer, const char *style, speaker ? speaker : "", text); } -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, int readorder, int layer, const char *style, - const char *speaker) + const char *speaker, size_t *nb_rect_allocated) { - AVSubtitleRect **rects, *rect; + AVSubtitleRect **rects = sub->rects, *rect; char *ass_str; + uint64_t new_nb = 0; - rects = av_realloc_array(sub->rects, sub->num_rects+1, sizeof(*sub->rects)); - if (!rects) + if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { + new_nb = sub->num_rects + sub->num_rects/16LL + 1; + } else if (!nb_rect_allocated) + new_nb = sub->num_rects + 1LL; + if (new_nb > SIZE_MAX) return AVERROR(ENOMEM); - sub->rects = rects; + + if (new_nb) { + rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects)); + if (!rects) + return AVERROR(ENOMEM); + if (nb_rect_allocated) + *nb_rect_allocated = new_nb; + sub->rects = rects; + } + rect = av_mallocz(sizeof(*rect)); if (!rect) return AVERROR(ENOMEM); @@ -137,6 +150,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, return 0; } +int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, + int readorder, int layer, const char *style, + const char *speaker) +{ + return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, NULL); +} + void ff_ass_decoder_flush(AVCodecContext *avctx) { FFASSDecoderContext *s = avctx->priv_data; diff --git a/libavcodec/ass.h b/libavcodec/ass.h index 2c260e4e785..784f46c42c5 100644 --- a/libavcodec/ass.h +++ b/libavcodec/ass.h @@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, int readorder, int layer, const char *style, const char *speaker); +/** + * Add an ASS dialog to a subtitle. + */ +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, + int readorder, int layer, const char *style, + const char *speaker, size_t *nb_rect_allocated); + /** * Helper to flush a text subtitles decoder making use of the * FFASSDecoderContext.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/ass.c | 32 ++++++++++++++++++++++++++------ libavcodec/ass.h | 7 +++++++ 2 files changed, 33 insertions(+), 6 deletions(-)