[FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT

2023-02-20 Thread rcombs
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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT

2023-02-20 Thread Nicolas George
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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT

2023-02-21 Thread Ridley Combs



> On Feb 21, 2023, at 01:48, Nicolas George  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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 2/3] lavc/assenc: set AV_CODEC_CAP_SINGLE_SUB_RECT

2023-02-22 Thread Nicolas George
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-requ...@ffmpeg.org with subject "unsubscribe".