Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
> Not sure if this justifies not adding the new code to PutBitContext: it > doesn't > have to work for all situations, only for the encoder that initially uses > it. > > Like the current patch, works on big and little endian, i will try to rewrite this patch using a start of a new PutbitContext64. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
2019-02-27 17:50 GMT+01:00, Martin Vignali : >> >> Shouldn’t there be a 64Bit PutBitContext instead so other encoders can >> also profit? > > I only use here a small part of putbitcontext, and in an "unsafe" mode (the > buffer of the prores aw encoder is big enough to not check it, and write 64 > bits during the flush even if less can be use (to keep code more simple)) Not sure if this justifies not adding the new code to PutBitContext: it doesn't have to work for all situations, only for the encoder that initially uses it. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
On Tue, Mar 05, 2019 at 11:28:56AM +0100, Martin Vignali wrote: > Hello, > > do i understand correctly that there is no check that prevents out of array > > writing ? > > not even an assert > > If thats the case, then i think this is unwise. > > > > > Thanks for testing. > > For the buffer check, i can add a test (assert or error), before slice > plane encoding, in order to check if there is enough space in the dst > buffer for the "worst" slice plane target size. Whatever check is added it should be robust and ensure that no out of array writes can happen. For example someone changing what is written should not be able to miss any checks that he would need to update. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
Hello, do i understand correctly that there is no check that prevents out of array > writing ? > not even an assert > If thats the case, then i think this is unwise. > > Thanks for testing. For the buffer check, i can add a test (assert or error), before slice plane encoding, in order to check if there is enough space in the dst buffer for the "worst" slice plane target size. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
On Mon, Mar 04, 2019 at 01:24:27PM +0100, Martin Vignali wrote: > Hello, > > Test on big endian is welcome. > If it's works, it will avoid to add #if HAVE_BIG_ENDIAN in lot of place, to > separate the old code from the new. it passes fate on qemu mips [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Let us carefully observe those good qualities wherein our enemies excel us and endeavor to excel them, by avoiding what is faulty, and imitating what is excellent in them. -- Plutarch signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
On Tue, Feb 26, 2019 at 04:54:38PM +0100, Martin Vignali wrote: > Hello, > > Patch in attach, change codeword bits writing > by replacing PutBitContext, and use instead uint64_t bit_buf > Also remove byte buffer length check > > encode_codeword func have two version now > - one for dc coeff and run coeff (same as previous encode_codeword func) > - one for level coeff who also integrate the "IS_NEGATIVE(val)" bit writing > > encode_codeword func code is reorganize > in the first part, calculate codeword value and number of bits for this > value > at the end, write the codeword in bit_buf, and if bit_buf is full, write it > in dst buf. > > Pass fate test for me (X86_64 os X) > also tested, on real samples. Md5 hash of the target file doesn't change > for me > > Speed improvment on X86_64 : > test file 1 : 140 -> 154 fps > test file 2 : 55 -> 62 fps > > Not tested on big endian and X86_32. > > Fate test cmd : > make fate-vsynth3-prores;make fate-vsynth2-prores;make > fate-vsynth1-prores;make fate-vsynth_lena-prores SAMPLES=fate-suite/ > > Comments welcome > > Martin > proresenc_anatoliy.c | 133 > +-- > 1 file changed, 108 insertions(+), 25 deletions(-) > a5390e02202e780e4ae88d931af3959f338d447d > 0001-avcodec-proresenc_aw-replace-putbitsContext-by-uint6.patch > From ce7709c6145920a9eeea8618810894d3a7fe99ba Mon Sep 17 00:00:00 2001 > From: Martin Vignali > Date: Tue, 26 Feb 2019 16:33:39 +0100 > Subject: [PATCH] avcodec/proresenc_aw : replace putbitsContext by uint64 > bit_buf > > also remove the target buffer length check do i understand correctly that there is no check that prevents out of array writing ? not even an assert If thats the case, then i think this is unwise. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Breaking DRM is a little like attempting to break through a door even though the window is wide open and the only thing in the house is a bunch of things you dont want and which you would get tomorrow for free anyway signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
Hello, Test on big endian is welcome. If it's works, it will avoid to add #if HAVE_BIG_ENDIAN in lot of place, to separate the old code from the new. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
> > Shouldn’t there be a 64Bit PutBitContext instead so other encoders can > also profit? > > Carl Eugen > I only use here a small part of putbitcontext, and in an "unsafe" mode (the buffer of the prores aw encoder is big enough to not check it, and write 64 bits during the flush even if less can be use (to keep code more simple)) Maybe can be used later as a start to write a more general PutBitContext with 64 bits bit_buf, for use in various encoders. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
> Am 26.02.2019 um 16:54 schrieb Martin Vignali : > > Patch in attach, change codeword bits writing > by replacing PutBitContext, and use instead uint64_t bit_buf Shouldn’t there be a 64Bit PutBitContext instead so other encoders can also profit? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] avcodec/proresenc_aw : improve speed by replacing PutBitContext for codeword encoding
Hello, Patch in attach, change codeword bits writing by replacing PutBitContext, and use instead uint64_t bit_buf Also remove byte buffer length check encode_codeword func have two version now - one for dc coeff and run coeff (same as previous encode_codeword func) - one for level coeff who also integrate the "IS_NEGATIVE(val)" bit writing encode_codeword func code is reorganize in the first part, calculate codeword value and number of bits for this value at the end, write the codeword in bit_buf, and if bit_buf is full, write it in dst buf. Pass fate test for me (X86_64 os X) also tested, on real samples. Md5 hash of the target file doesn't change for me Speed improvment on X86_64 : test file 1 : 140 -> 154 fps test file 2 : 55 -> 62 fps Not tested on big endian and X86_32. Fate test cmd : make fate-vsynth3-prores;make fate-vsynth2-prores;make fate-vsynth1-prores;make fate-vsynth_lena-prores SAMPLES=fate-suite/ Comments welcome Martin 0001-avcodec-proresenc_aw-replace-putbitsContext-by-uint6.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
