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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3329 bytes --]

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@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

thx

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

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".