On Sun, Dec 23, 2018 at 04:05:12PM +0100, Michael Niedermayer wrote: > 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
Will apply this patchset soon unless there are objections thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel