| Message ID | 20230221002516.25784-3-rcombs@rcombs.me |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure_x86 | warning | Failed to apply patch |
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
rcombs (12023-02-20): > Fixes ASS output when multiple rects are present. > --- > fftools/ffmpeg.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) Does this not belong to the framework? > > 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) > + 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; > > 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); Regards,
> On Feb 21, 2023, at 01:45, Nicolas George <george@nsup.org> wrote: > > rcombs (12023-02-20): >> Fixes ASS output when multiple rects are present. >> --- >> fftools/ffmpeg.c | 28 ++++++++++++++++++---------- >> 1 file changed, 18 insertions(+), 10 deletions(-) > > Does this not belong to the framework? I don't see a way it could be? avcodec_encode_subtitle takes an AVSubtitle and writes to a single caller-provided buffer; there's no way for it to generate multiple outputs for a single input. > >> >> 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) >> + 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; >> >> 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); > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
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);
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) + 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; 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);