[FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT

[TADANO Tokumei]

On 2023/02/21 9:25, rcombs wrote:
> Fixes ASS output when multiple rects are present.
> ---
>   fftools/ffmpeg.c | 28 ++++++++++++++++++----------
>   1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 9884e0c6c6..23eac52438 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -1021,6 +1021,7 @@ static void do_subtitle_out(OutputFile *of,
>       AVCodecContext *enc;
>       AVPacket *pkt = ost->pkt;
>       int64_t pts;
> +    int single_rect;
>   
>       if (sub->pts == AV_NOPTS_VALUE) {
>           av_log(ost, AV_LOG_ERROR, "Subtitle packets must have a pts\n");
> @@ -1031,11 +1032,15 @@ static void do_subtitle_out(OutputFile *of,
>   
>       enc = ost->enc_ctx;
>   
> +    single_rect = !!(enc->codec->capabilities & AV_CODEC_CAP_SINGLE_SUB_RECT);
> +
>       /* Note: DVB subtitle need one packet to draw them and one other
>          packet to clear them */
>       /* XXX: signal it in the codec context ? */
>       if (enc->codec_id == AV_CODEC_ID_DVB_SUBTITLE)
>           nb = 2;
> +    else if (single_rect)

If the problem exists on assenc only, how about using codec id to be hard-coded?

     else if (enc->codec_id == AV_CODEC_ID_ASS)

It meets your goal without changing framework (but may be dirty solution).
The current code already hard-codes AV_CODEC_ID_DVB_SUBTITLE as a special case.

> +        nb = FFMAX(sub->num_rects, 1);
>       else
>           nb = 1;
>   
> @@ -1044,7 +1049,7 @@ static void do_subtitle_out(OutputFile *of,
>       if (output_files[ost->file_index]->start_time != AV_NOPTS_VALUE)
>           pts -= output_files[ost->file_index]->start_time;
>       for (i = 0; i < nb; i++) {
> -        unsigned save_num_rects = sub->num_rects;
> +        AVSubtitle local_sub = *sub;

It is better to put it out from "for" loop.
It takes costs to copy entire structure on each loop.

>   
>           if (!check_recording_time(ost, pts, AV_TIME_BASE_Q))
>               return;
> @@ -1053,19 +1058,22 @@ static void do_subtitle_out(OutputFile *of,
>           if (ret < 0)
>               report_and_exit(AVERROR(ENOMEM));
>   
> -        sub->pts = pts;
> +        local_sub.pts = pts;
>           // start_display_time is required to be 0
> -        sub->pts               += av_rescale_q(sub->start_display_time, (AVRational){ 1, 1000 }, AV_TIME_BASE_Q);
> -        sub->end_display_time  -= sub->start_display_time;
> -        sub->start_display_time = 0;
> -        if (i == 1)
> -            sub->num_rects = 0;
> +        local_sub.pts               += av_rescale_q(sub->start_display_time, (AVRational){ 1, 1000 }, AV_TIME_BASE_Q);
> +        local_sub.end_display_time  -= sub->start_display_time;
> +        local_sub.start_display_time = 0;
> +
> +        if (enc->codec_id == AV_CODEC_ID_DVB_SUBTITLE && i == 1)
> +            local_sub.num_rects = 0;
> +        else if (single_rect && sub->num_rects > 0) {
> +            local_sub.num_rects = 1;
> +            local_sub.rects += i;
> +        }
>   
>           ost->frames_encoded++;
>   
> -        subtitle_out_size = avcodec_encode_subtitle(enc, pkt->data, pkt->size, sub);
> -        if (i == 1)
> -            sub->num_rects = save_num_rects;
> +        subtitle_out_size = avcodec_encode_subtitle(enc, pkt->data, pkt->size, &local_sub);
>           if (subtitle_out_size < 0) {
>               av_log(ost, AV_LOG_FATAL, "Subtitle encoding failed\n");
>               exit_program(1);