[FFmpeg-devel] [PATCH] avcodec/ituh263enc: Add AV_CODEC_CAP_SLICE_THREADS to old H.263

[Andreas Rheinhardt]

Michael Niedermayer:
> On Tue, Apr 05, 2022 at 05:07:22PM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> It is supported by the H.263+ AVCodec already
>>>
>>> Is there any case where this does not work ?
>>>
>>> Fixes regression of some command lines
>>>
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavcodec/ituh263enc.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavcodec/ituh263enc.c b/libavcodec/ituh263enc.c
>>> index db7cdf1fcb..82dce05e36 100644
>>> --- a/libavcodec/ituh263enc.c
>>> +++ b/libavcodec/ituh263enc.c
>>> @@ -908,6 +908,7 @@ const FFCodec ff_h263_encoder = {
>>>      .p.id           = AV_CODEC_ID_H263,
>>>      .p.pix_fmts = (const enum AVPixelFormat[]){AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE},
>>>      .p.priv_class   = &h263_class,
>>> +    .p.capabilities = AV_CODEC_CAP_SLICE_THREADS,
>>>      .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
>>>      .priv_data_size = sizeof(MpegEncContext),
>>>      .init           = ff_mpv_encode_init,
>>
>> 1. If you claim that there is a regression, you should mention the
>> commit that introduced them in the commit message (it's obviously
>> 8ca4b515e73079cda068e253853654db394b8171 in this case).
>> 2. What command lines regressed exactly? The only command lines that
>> should be affected by said commit are command lines that set the slices
>> option to a value > 1.
>> 3. As the commit message of 8ca4b515e73079cda068e253853654db394b8171
>> explains, this was intentional, as the H.263 encoder produces broken
>> files with multiple slices (whether with slice-threading or not). One
>> gets all kinds of error messages when decoding such a file: "I cbpy
>> damaged at 1 7", "Error at MB: 316", "illegal ac vlc code at 0x29",
>> "slice end not reached but screenspace end (7 left 800000, score=
>> -125)", "run overflow at 0x7 i:1". Of course, there are visual
>> artifacts, too.
>> 4. With this patch, this encoder will by default (at least, by the
>> defaults of the ffmpeg command line tool) produce broken files.
>> 5. "Is there any case where this does not work ?": Is there any where it
>> works?
> 
> The testcases i had where these:
> ./ffmpeg -threads 1 -i fate-suite/svq3/Vertical400kbit.sorenson3.mov -t 1 -bitexact -qscale 2 -slices 1 -y -threads 1 -vcodec h263 -s 352x288 -an /tmp/file-h263-s1t1.h263
> ./ffmpeg -threads 1 -i fate-suite/svq3/Vertical400kbit.sorenson3.mov -t 1 -bitexact -qscale 2 -slices 2 -y -threads 1 -vcodec h263 -s 352x288 -an /tmp/file-h263-s2t1.h263
> ./ffmpeg -threads 1 -i fate-suite/svq3/Vertical400kbit.sorenson3.mov -t 1 -bitexact -qscale 2 -slices 1 -y -threads 2 -vcodec h263 -s 352x288 -an /tmp/file-h263-s1t2.h263
> ./ffmpeg -threads 1 -i fate-suite/svq3/Vertical400kbit.sorenson3.mov -t 1 -bitexact -qscale 2 -slices 2 -y -threads 2 -vcodec h263 -s 352x288 -an /tmp/file-h263-s2t2.h263
> 
> The files seem to play fine
> i did not try to find a case that fails
> 

./ffmpeg -threads 1 -i fate-suite/svq3/Vertical400kbit.sorenson3.mov -t
1 -bitexact -qscale 2 -slices 4 -y -threads 4 -vcodec h263 -s 1408x1152
-an /tmp/file-h263-s2t2.h263

produces garbage; also with -s 704x576. slices < 4 seem fine. If one
uses too many slices with smaller resolutions, the file is no longer
correctly probed, but can be correctly decoded with -f h263.
I don't know what is wrong with the bigger resolutions and too many
slices; I don't know H.263 at all. My first (and admittedly only) test
for whether using multiple slices with a single thread works produced
garbage, so I put this codec in the "multiple slices not supported" box.

- Andreas