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

[Michael Niedermayer]

On Wed, Apr 06, 2022 at 03:05:07AM +0200, Andreas Rheinhardt wrote:
> 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.

Its a while ago but H263+ has a nice slice structired mode H263 lacks that
and uses a more restricted mode see ff_h263_encode_gob_header()
I would wildly guess these restrictions are what causes some cases not
to work but i didnt check

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20220406/73223f44/attachment.sig>