* [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT @ 2023-02-21 0:25 rcombs 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT rcombs ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: rcombs @ 2023-02-21 0:25 UTC (permalink / raw) To: ffmpeg-devel --- doc/APIchanges | 3 +++ libavcodec/codec.h | 5 +++++ libavcodec/version.h | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/doc/APIchanges b/doc/APIchanges index bc52a07964..56f33aa25b 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -14,6 +14,9 @@ libavutil: 2021-04-27 API changes, most recent first: +2023-02-20 - xxxxxxxxxx - lavc 59.60.100 - codec.h + Add AV_CODEC_CAP_SINGLE_SUB_RECT. + 2023-01-29 - xxxxxxxxxx - lavc 59.59.100 - avcodec.h Add AV_CODEC_FLAG_COPY_OPAQUE and AV_CODEC_FLAG_FRAME_DURATION. diff --git a/libavcodec/codec.h b/libavcodec/codec.h index 77a1a3f5a2..c0df33ef3c 100644 --- a/libavcodec/codec.h +++ b/libavcodec/codec.h @@ -190,6 +190,11 @@ */ #define AV_CODEC_CAP_ENCODER_RECON_FRAME (1 << 22) +/** + * This encoder requires a single rectangle per AVSubtitle. + */ +#define AV_CODEC_CAP_SINGLE_SUB_RECT (1 << 23) + /** * AVProfile. */ diff --git a/libavcodec/version.h b/libavcodec/version.h index 752adc81f8..2ed4ef5547 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #include "version_major.h" -#define LIBAVCODEC_VERSION_MINOR 59 +#define LIBAVCODEC_VERSION_MINOR 60 #define LIBAVCODEC_VERSION_MICRO 100 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ -- 2.39.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 0:25 [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT rcombs @ 2023-02-21 0:25 ` rcombs 2023-02-21 7:48 ` Nicolas George 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 7:42 ` [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT Nicolas George 2 siblings, 1 reply; 11+ messages in thread From: rcombs @ 2023-02-21 0:25 UTC (permalink / raw) To: ffmpeg-devel This already gave garbled output when multiple rects were present, so this is simply documenting an existing requirement. --- libavcodec/assenc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavcodec/assenc.c b/libavcodec/assenc.c index db6fd25dd7..1c49a6685b 100644 --- a/libavcodec/assenc.c +++ b/libavcodec/assenc.c @@ -74,6 +74,7 @@ const FFCodec ff_ssa_encoder = { CODEC_LONG_NAME("ASS (Advanced SubStation Alpha) subtitle"), .p.type = AVMEDIA_TYPE_SUBTITLE, .p.id = AV_CODEC_ID_ASS, + .p.capabilities = AV_CODEC_CAP_SINGLE_SUB_RECT, .init = ass_encode_init, FF_CODEC_ENCODE_SUB_CB(ass_encode_frame), }; @@ -85,6 +86,7 @@ const FFCodec ff_ass_encoder = { CODEC_LONG_NAME("ASS (Advanced SubStation Alpha) subtitle"), .p.type = AVMEDIA_TYPE_SUBTITLE, .p.id = AV_CODEC_ID_ASS, + .p.capabilities = AV_CODEC_CAP_SINGLE_SUB_RECT, .init = ass_encode_init, FF_CODEC_ENCODE_SUB_CB(ass_encode_frame), }; -- 2.39.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT rcombs @ 2023-02-21 7:48 ` Nicolas George 2023-02-21 8:12 ` Ridley Combs 0 siblings, 1 reply; 11+ messages in thread From: Nicolas George @ 2023-02-21 7:48 UTC (permalink / raw) To: FFmpeg development discussions and patches rcombs (12023-02-20): > This already gave garbled output when multiple rects were present, > so this is simply documenting an existing requirement. > --- > libavcodec/assenc.c | 2 ++ > 1 file changed, 2 insertions(+) NAK: the code has provisions for multiple rectangles, if you enforce a single rectangle you need to remove the code that is now useless. But I do not think pushing the issue onto the applications is a good way to fix the problem. Or even pushing the issue onto the framework, since the framework does not know the specifics. Better fix the code in ASS that handles multiple rectangles than inventing yet another annoying flag. A single frame can be encoded into multiple packets, so that should not be hard. Regards, -- Nicolas George _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 7:48 ` Nicolas George @ 2023-02-21 8:12 ` Ridley Combs 2023-02-22 13:24 ` Nicolas George 0 siblings, 1 reply; 11+ messages in thread From: Ridley Combs @ 2023-02-21 8:12 UTC (permalink / raw) To: ffmpeg-devel > On Feb 21, 2023, at 01:48, Nicolas George <george@nsup.org> wrote: > > rcombs (12023-02-20): >> This already gave garbled output when multiple rects were present, >> so this is simply documenting an existing requirement. >> --- >> libavcodec/assenc.c | 2 ++ >> 1 file changed, 2 insertions(+) > > NAK: the code has provisions for multiple rectangles, if you enforce a > single rectangle you need to remove the code that is now useless. Fair enough, if we're fine with breaking the existing case further. Should I simply drop rectangles after a first, or return an error? > > But I do not think pushing the issue onto the applications is a good way > to fix the problem. Or even pushing the issue onto the framework, since > the framework does not know the specifics. Better fix the code in ASS > that handles multiple rectangles than inventing yet another annoying > flag. A single frame can be encoded into multiple packets, so that > should not be hard. This is only true for audio/video encoders; subtitle encoders still use a different API, which does not have M:N support. There's some long-ongoing work to change that, but for now, this seems like the only way to deal with this case before that API overhaul. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 8:12 ` Ridley Combs @ 2023-02-22 13:24 ` Nicolas George 0 siblings, 0 replies; 11+ messages in thread From: Nicolas George @ 2023-02-22 13:24 UTC (permalink / raw) To: FFmpeg development discussions and patches Ridley Combs (12023-02-21): > Fair enough, if we're fine with breaking the existing case further. > Should I simply drop rectangles after a first, or return an error? You have to ask? Between reporting an error and silently corrupting data, the answer is never in doubt. > This is only true for audio/video encoders; subtitle encoders still > use a different API, which does not have M:N support. There's some > long-ongoing work to change that, but for now, this seems like the > only way to deal with this case before that API overhaul. Oh, I had forgotten this. In that cas, I would suggest to keep API changes to a minimum: - Have assenc print and return an error if there are several rectangles. - Add a special case in the command-line tool ffmpeg based on the encoding codec. Or probably even better, for all text subtitles. This will not interfere with overhauling the subtitles encoding API. Of course, other developers might disagree, give it a few days. Regards, -- Nicolas George _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 0:25 [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT rcombs @ 2023-02-21 0:25 ` rcombs 2023-02-21 7:45 ` Nicolas George 2023-03-01 15:19 ` TADANO Tokumei 2023-02-21 7:42 ` [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT Nicolas George 2 siblings, 2 replies; 11+ messages in thread From: rcombs @ 2023-02-21 0:25 UTC (permalink / raw) To: ffmpeg-devel 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) + 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); -- 2.39.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT rcombs @ 2023-02-21 7:45 ` Nicolas George 2023-02-21 8:09 ` Ridley Combs 2023-03-01 15:19 ` TADANO Tokumei 1 sibling, 1 reply; 11+ messages in thread From: Nicolas George @ 2023-02-21 7:45 UTC (permalink / raw) To: FFmpeg development discussions and patches 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, -- Nicolas George _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 7:45 ` Nicolas George @ 2023-02-21 8:09 ` Ridley Combs 0 siblings, 0 replies; 11+ messages in thread From: Ridley Combs @ 2023-02-21 8:09 UTC (permalink / raw) To: ffmpeg-devel > 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". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 7:45 ` Nicolas George @ 2023-03-01 15:19 ` TADANO Tokumei 1 sibling, 0 replies; 11+ messages in thread From: TADANO Tokumei @ 2023-03-01 15:19 UTC (permalink / raw) To: ffmpeg-devel 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); _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 0:25 [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT rcombs @ 2023-02-21 7:42 ` Nicolas George 2023-02-21 8:07 ` Ridley Combs 2 siblings, 1 reply; 11+ messages in thread From: Nicolas George @ 2023-02-21 7:42 UTC (permalink / raw) To: FFmpeg development discussions and patches rcombs (12023-02-20): > --- > doc/APIchanges | 3 +++ > libavcodec/codec.h | 5 +++++ > libavcodec/version.h | 2 +- > 3 files changed, 9 insertions(+), 1 deletion(-) And... No change to the framework to make use of this flag? Like, return AVERROR(EINVAL) if the flag is present but several rectangles are given? Regards, -- Nicolas George _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT 2023-02-21 7:42 ` [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT Nicolas George @ 2023-02-21 8:07 ` Ridley Combs 0 siblings, 0 replies; 11+ messages in thread From: Ridley Combs @ 2023-02-21 8:07 UTC (permalink / raw) To: ffmpeg-devel > On Feb 21, 2023, at 01:42, Nicolas George <george@nsup.org> wrote: > > rcombs (12023-02-20): >> --- >> doc/APIchanges | 3 +++ >> libavcodec/codec.h | 5 +++++ >> libavcodec/version.h | 2 +- >> 3 files changed, 9 insertions(+), 1 deletion(-) > > And... No change to the framework to make use of this flag? Like, return > AVERROR(EINVAL) if the flag is present but several rectangles are given? I didn't want to break existing applications further, though I don't feel strongly about it either way (since the existing behavior is already very poor). > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-03-01 15:19 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-21 0:25 [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 7:48 ` Nicolas George 2023-02-21 8:12 ` Ridley Combs 2023-02-22 13:24 ` Nicolas George 2023-02-21 0:25 ` [FFmpeg-devel] [PATCH 3/3] ffmpeg: respect AV_CODEC_CAP_SINGLE_SUB_RECT rcombs 2023-02-21 7:45 ` Nicolas George 2023-02-21 8:09 ` Ridley Combs 2023-03-01 15:19 ` TADANO Tokumei 2023-02-21 7:42 ` [FFmpeg-devel] [PATCH 1/3] lavc/codec.h: add AV_CODEC_CAP_SINGLE_SUB_RECT Nicolas George 2023-02-21 8:07 ` Ridley Combs
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git