On Wed, Dec 19, 2018 at 10:35:25AM +0100, Jerome Martinez wrote:
> On 19/12/2018 02:40, Michael Niedermayer wrote:
> >Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> >---
> > libavcodec/ffv1enc.c | 10 +++-------
> > libavcodec/rangecoder.c | 4 +++-
> > libavcodec/rangecoder.h | 2 +-
> > libavcodec/snowenc.c | 2 +-
> > libavcodec/sonic.c | 2 +-
> > libavcodec/tests/rangecoder.c | 2 +-
> > 6 files changed, 10 insertions(+), 12 deletions(-)
> >
> >diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
> >index f5eb0feb4e..796d81f7c6 100644
> >--- a/libavcodec/ffv1enc.c
> >+++ b/libavcodec/ffv1enc.c
> >@@ -449,7 +449,7 @@ static int write_extradata(FFV1Context *f)
> > put_symbol(c, state, f->intra = (f->avctx->gop_size < 2), 0);
> > }
> >- f->avctx->extradata_size = ff_rac_terminate(c);
> >+ f->avctx->extradata_size = ff_rac_terminate(c, 0);
> > v = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0, f->avctx->extradata,
> > f->avctx->extradata_size);
> > AV_WL32(f->avctx->extradata + f->avctx->extradata_size, v);
> > f->avctx->extradata_size += 4;
> >@@ -1065,9 +1065,7 @@ retry:
> > encode_slice_header(f, fs);
> > }
> > if (fs->ac == AC_GOLOMB_RICE) {
> >- if (f->version > 2)
> >- put_rac(&fs->c, (uint8_t[]) { 129 }, 0);
> >- fs->ac_byte_count = f->version > 2 || (!x && !y) ?
> >ff_rac_terminate(&fs->c) : 0;
> >+ fs->ac_byte_count = f->version > 2 || (!x && !y) ?
> >ff_rac_terminate(&fs->c, f->version > 2) : 0;
>
> Moving the "129" stuff from FFV1 encoder code to rangecoder encoding part is
> good for factorization, but there is no more mirroring of FFV1 encoding and
> FFV1 decoding in that case ("129" stuff is still present in FFV1 decoder
> code, not rangecoder decoding part), IMO this makes the FFV1 code more
> difficult to understand.
>
> Isn't it possible to have the same kind of modification for the decoding
> part?The encoder side factors 2 different termination procedures. If you need version 1 there you need version 1 you cannot use version 0 in its place. The decoder has to deal with buggy input and the kind of termination needed depends on where the termination is done it is not 1:1 bound to the bitstream version. The encoder OTOH does not produce the buggy variants so it does not have anything symmetric for that. There are also 3 places where termination happens, each is different even if one disregards the old bug. One place also needs further investigation to understand the implications for the bitstream compatibility in case its changed. So while i can factor the decoder side in a way similar to the encoder this will still not make the code look more similar. So i suggest to apply this patchset as it is. I do have some patches locally that mess with the decoder side of the related code but the code does not become simpler even thhough it does get checked a bit more fully. So this is not really what you asked for IIUC thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB What does censorship reveal? It reveals fear. -- Julian Assange
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
