Re: [FFmpeg-devel] avcodec/proresenc_aw : add support for prores 444

2018-07-22 Thread Martin Vignali
> >
> > Speed (from prores 422 HQ file -> Prores 444)
> >
> > ./ffmpeg -i prores422hqfile.mov -pix_fmt yuv444p10 -c:v prores -an
> > -profile:v 4 res_aw.mov -y
> > ==> 25 fps
> >
> > ./ffmpeg -i prores422hqfile.mov -pix_fmt yuv444p10 -c:v prores_ks -an
> > -profile:v 4 res_ks.mov -y
> > ==> 10 fps
>
> This is unfair comparison.
>

What command line seems fair to you ?
Even if, i set the same min/max quant (1, 6) and reducing the target
bitrate (to have similar output size), the speed difference is very
important.
The resulting file is very close in quality (most of the picture is the
same), on few part there is a very little difference in favour of one or
another encoder depending of the content.



> >
> > 001 : Add support for Prores 444 (without alpha)
> > 002 : change the frame flag in header part, to be the same than the
> > official encoder in 422 mode
> > (and update fate test)
>
> Whatever you are doing regarding improving prores_aw is bad idea.
>
>
>
I disagree (of course !).
And i suppose you say that, because you have in mind to remove this encoder.

After taking a look on both encoder, prores_aw have a faster quantif
search, than prores_ks for me.
Ks encoder sometime reduce too much the quality on some slice, where aw, is
much simpler, and make less quality variation between slice.

IMHO :
Considering, the main use of prores codec (intermediate format in
post-production)
A simple/fast encoder (Prores Aw) is more appropriate in most of the case
(reason why i mainly use this one, when i can).
The main disadvantage (for now), for aw encoder, is lack of some user
option (to adapt speed/size/bitrate constraint (in other word, min_quant,
max_quant, target_mb_bitrate),
and the support of other kind of prores (444 with alpha, and interlace
content)
But probably not so much work to do to add it (considering, the work
already done in ks encoder).

Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] avcodec/proresenc_aw improvements

2018-07-22 Thread Martin Vignali
>
> > 001 : use scantable in prores_data instead of a duplicate one.
>
> This could negativly affect the performance of the changed inner loop
> have you checked that the changed function does not become slower ?
>
>
>
No, i will make some tests.

> > -*buf++ = 6;
> > +*buf++ = pict->color_primaries;
> > +*buf++ = pict->color_trc;
> > +*buf++ = pict->colorspace;
>
> Has someone confirmed that all values our enum contains or will
> contain can just be written with no check ?
>
>
Mostly see prores with Rec709, Rec601. Need to take a look for Rec2020.
Don't know if all value are authorized, doesn't use other colorspace, with
prores.

Do you think it's better to only authorize few colorspace ?

Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/3] lavfi/motion_estimation: use pixelutils API for sad.

2018-07-22 Thread Marton Balint



On Tue, 17 Jul 2018, myp...@gmail.com wrote:


On Sun, Jul 15, 2018 at 1:03 AM Michael Niedermayer
 wrote:


On Sat, Jul
14, 2018 at 12:04:46PM +0200, Marton Balint wrote:
>
>
> On Sat, 14 Jul 2018, Michael Niedermayer wrote:
>
> >On Fri, Jul 13, 2018 at 10:51:00AM +0200, Marton Balint wrote:
> >>
> >>
> >>On Thu, 12 Jul 2018, myp...@gmail.com wrote:
> >>
> >>>On Thu, Jul 12, 2018 at 12:43 AM Marton Balint  wrote:
> 
> 
> 
> On Wed, 11 Jul 2018, Jun Zhao wrote:
> 
> >use pixelutils API for sad in motion estimation.
> 
> Does it make sense to improve this code? I thought a superior and faster
> approach was a result of 2017 GSOC task:
> 
> 
https://docs.google.com/document/d/1Hyh_rxP1KGsVkg7i7yU8Bcv92z0LIL4r-axpoKfvMFk/edit
> 
> Maybe that code should be merged back, and any further optimalization
> should be done based on that code, no?
> 
> Thanks,
> Marton
> 
> >>>Hi, Marton:
> >>>
> >>>Yes, now I try to improve the
minterpolate, and after use perf
> >>>profiing the commands:
> >>>

> >>>./ffmpeg -i a.ts -filter_complex
> >>>"minterpolate=mi_mode=mci:mc_mode=aobmc:vsbmc=1" -f null /dev/null
> >>>I found the hotspot is:
> >>>- get_sbad_ob
> >>>- get_sbad
> >>>- get_sad_ob
> >>>- bilateral_obmc
> >>>- set_frame_data
> >>>
> >>>So, as my plan, I will try to use sse2/avx2
> >>>Scatter/Gather, optimized
> >>>sad function (use pixelutils API)
> >>>in  get_sbad_ob /  get_sbad /  get_sad_ob first, for  set_frame_data
> >>>case, maybe need to use Scatter/Gather SIMD instruction.
> >>
> >>That is great, all I am saying we should avoid diverging the two brances
> >>(FFmpeg branch, and GSOC 2017 branch), and try to merge back GSOC2017 if it
> >>can be done with reasonable amount of work before optimizing code, otherwise
> >>the GSOC2017 branch will rot and we will lose the result of the GSOC task.
> >>
> >>>
> >>>But if some guys have done some improve task in this case, I think
> >>>based on the pre-existing work is the better way.
> >>
> >>Michael was the mentor, maybe he can chip in on what should be done here.
> >
> >talk with the author/student who wrote the code, not me :)
>
> Well, his not active here,

yes but last i heared from him, he was interrested in continuing this project
i think ive not heared much from him after that but i now see that there is a
small commit in his repo from 2018 so he is not completely inactive.
I think you should talk with him


> and the question is if his work is ready for
> mainline inclusion or not, and if he has done enough valuable work during
> GSOC that its worth working on mainlining it.

He certainly did valuable work. Looking now at the ML, it seems the more or
less last thing on the ML was the RFC/Discussion thread about libmotion.
In that everyone wanted to dictate the design, and all that was contradicting
each other.
If you want to work on unifying this entangled bikeshed ball of conflicting
oppinions, that surely is very welcome. Important is that it ends in something
that is practical and high quality.
Personally i think the author should be given more authority in the design.
But again, please talk with the author of this code
I dont remember everything in as much detail about this ...

also ive added him to the CC

Thanks



Now the minterpolate/libmotion auther didn't give a feedback or
sugesstion, so I will update patch 1/2  (just add SSE2/AVX2 sad_32x32)
with some perf data and hold on the patch 3 about minterpolate, any
other comments?


I checked the "libmotion" series, and it seems they are in 
debug/development state and the commits are not clean, so some heavy 
refactoring is needed before applying them anyway.


Do what you prefer, snow codec based motion compenstaion is an additional 
algorithm to the existing code anyway as far as I see.


Thanks,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/5] avcodec/dirac_dwt_template: Fix several integer overflows in horizontal_compose_daub97i()

2018-07-22 Thread Michael Niedermayer
Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type 
'int'
Fixes: 
8926/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-6047609228623872

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/dirac_dwt_template.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/dirac_dwt_template.c b/libavcodec/dirac_dwt_template.c
index 2369c8d15b..5d55d932a1 100644
--- a/libavcodec/dirac_dwt_template.c
+++ b/libavcodec/dirac_dwt_template.c
@@ -190,15 +190,15 @@ static void RENAME(horizontal_compose_daub97i)(uint8_t 
*_b, uint8_t *_temp, int
 
 // second stage combined with interleave and shift
 b0 = b2 = COMPOSE_DAUB97iL0(temp[w2], temp[0], temp[w2]);
-b[0] = (b0 + 1) >> 1;
+b[0] = ~((~b0) >> 1);
 for (x = 1; x < w2; x++) {
 b2 = COMPOSE_DAUB97iL0(temp[x+w2-1], temp[x ], temp[x+w2]);
 b1 = COMPOSE_DAUB97iH0(  b0, temp[x+w2-1], b2);
-b[2*x-1] = (b1 + 1) >> 1;
-b[2*x  ] = (b2 + 1) >> 1;
+b[2*x-1] = ~((~b1) >> 1);
+b[2*x  ] = ~((~b2) >> 1);
 b0 = b2;
 }
-b[w-1] = (COMPOSE_DAUB97iH0(b2, temp[w-1], b2) + 1) >> 1;
+b[w-1] = ~((~COMPOSE_DAUB97iH0(b2, temp[w-1], b2)) >> 1);
 }
 
 static void RENAME(vertical_compose_dirac53iH0)(uint8_t *_b0, uint8_t *_b1, 
uint8_t *_b2,
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 5/5] avcodec/diracdec: Check bytes count in else branch in decode_lowdelay() too

2018-07-22 Thread Michael Niedermayer
Fixes: signed integer overflow: 8 * 340018243 cannot be represented in type 
'int'
Fixes: 
9441/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5194665207791616

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/diracdec.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavcodec/diracdec.c b/libavcodec/diracdec.c
index b27c743c58..9a417caec5 100644
--- a/libavcodec/diracdec.c
+++ b/libavcodec/diracdec.c
@@ -986,6 +986,10 @@ static int decode_lowdelay(DiracContext *s)
 for (slice_x = 0; bufsize > 0 && slice_x < s->num_x; slice_x++) {
 bytes = (slice_num+1) * (int64_t)s->lowdelay.bytes.num / 
s->lowdelay.bytes.den
- slice_num* (int64_t)s->lowdelay.bytes.num / 
s->lowdelay.bytes.den;
+if (bytes >= INT_MAX || bytes*8 > bufsize) {
+av_log(s->avctx, AV_LOG_ERROR, "too many bytes\n");
+return AVERROR_INVALIDDATA;
+}
 slices[slice_num].bytes   = bytes;
 slices[slice_num].slice_x = slice_x;
 slices[slice_num].slice_y = slice_y;
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/5] avcodec/diracdec: Prevent integer overflow in intermediate in global_mv()

2018-07-22 Thread Michael Niedermayer
Fixes: signed integer overflow: -393471 * 5460 cannot be represented in type 
'int'
Fixes: 
8890/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-6299775379963904

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/diracdec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/diracdec.c b/libavcodec/diracdec.c
index 753adeff61..e4cdd7ee2c 100644
--- a/libavcodec/diracdec.c
+++ b/libavcodec/diracdec.c
@@ -1399,8 +1399,8 @@ static void global_mv(DiracContext *s, DiracBlock *block, 
int x, int y, int ref)
 int *c  = s->globalmc[ref].perspective;
 
 int m   = (1> (ez+ep);
 block->u.mv[ref][1] = (my + (1<<(ez+ep))) >> (ez+ep);
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 4/5] avcodec/diracdec: Check slice numbers for overflows in relation to picture dimensions

2018-07-22 Thread Michael Niedermayer
Fixes: signed integer overflow: 88 * 33685506 cannot be represented in type 
'int'
Fixes: 
9433/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-5725943535501312

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/diracdec.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavcodec/diracdec.c b/libavcodec/diracdec.c
index 4ef1b3ea9b..b27c743c58 100644
--- a/libavcodec/diracdec.c
+++ b/libavcodec/diracdec.c
@@ -1243,7 +1243,10 @@ static int dirac_unpack_idwt_params(DiracContext *s)
 else {
 s->num_x= get_interleaved_ue_golomb(gb);
 s->num_y= get_interleaved_ue_golomb(gb);
-if (s->num_x * s->num_y == 0 || s->num_x * (uint64_t)s->num_y > 
INT_MAX) {
+if (s->num_x * s->num_y == 0 || s->num_x * (uint64_t)s->num_y > 
INT_MAX ||
+s->num_x * (uint64_t)s->avctx->width  > INT_MAX ||
+s->num_y * (uint64_t)s->avctx->height > INT_MAX
+) {
 av_log(s->avctx,AV_LOG_ERROR,"Invalid numx/y\n");
 s->num_x = s->num_y = 0;
 return AVERROR_INVALIDDATA;
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 3/5] avcodec/diracdec: Change frame_number to 64bit as its a 32bit from the bitstream and we also have a -1 special case

2018-07-22 Thread Michael Niedermayer
Fixes: signed integer overflow: 2147483647 + 1 cannot be represented in type 
'int'
Fixes: 
9291/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DIRAC_fuzzer-6324345860259840

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/diracdec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/diracdec.c b/libavcodec/diracdec.c
index e4cdd7ee2c..4ef1b3ea9b 100644
--- a/libavcodec/diracdec.c
+++ b/libavcodec/diracdec.c
@@ -141,7 +141,7 @@ typedef struct DiracContext {
 GetBitContext gb;
 AVDiracSeqHeader seq;
 int seen_sequence_header;
-int frame_number;   /* number of the next frame to display   */
+int64_t frame_number;   /* number of the next frame to display   */
 Plane plane[3];
 int chroma_x_shift;
 int chroma_y_shift;
@@ -2310,7 +2310,7 @@ static int dirac_decode_frame(AVCodecContext *avctx, void 
*data, int *got_frame,
 }
 
 if (*got_frame)
-s->frame_number = picture->display_picture_number + 1;
+s->frame_number = picture->display_picture_number + 1LL;
 
 return buf_idx;
 }
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 3/5] avcodec/tscc: Move reading the side data palette before other checks

2018-07-22 Thread Michael Niedermayer
On Sun, Jul 15, 2018 at 03:13:39PM +0200, Michael Niedermayer wrote:
> We do not want to loose the side data in case of errors
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/tscc.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)

will apply

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

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/5] avcodec/tscc: Do not duplicate images

2018-07-22 Thread Michael Niedermayer
On Sun, Jul 15, 2018 at 03:13:40PM +0200, Michael Niedermayer wrote:
> This improves speed
> 
> Fixes: Timeout
> Fixes: 
> 9010/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TSCC_fuzzer-6042614817095680
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/tscc.c |   6 +-
>  tests/ref/fate/tscc-15bit | 138 --
>  tests/ref/fate/tscc-32bit | 121 -
>  3 files changed, 4 insertions(+), 261 deletions(-)

will apply with a bugfix

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 5/5] avcodec/cdgraphics: Clear first frame only once

2018-07-22 Thread Michael Niedermayer
On Sun, Jul 15, 2018 at 03:13:41PM +0200, Michael Niedermayer wrote:
> frame_number will not increase if nothing is output
> 
> Fixes: Timeout
> Fixes: 
> 9057/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CDGRAPHICS_fuzzer-4844661498707968
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/cdgraphics.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

will apply

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

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] avcodec/proresenc_aw improvements

2018-07-22 Thread Michael Niedermayer
On Sun, Jul 22, 2018 at 01:04:29PM +0200, Martin Vignali wrote:
> >
> > > 001 : use scantable in prores_data instead of a duplicate one.
> >
> > This could negativly affect the performance of the changed inner loop
> > have you checked that the changed function does not become slower ?
> >
> >
> >
> No, i will make some tests.
> 
> > > -*buf++ = 6;
> > > +*buf++ = pict->color_primaries;
> > > +*buf++ = pict->color_trc;
> > > +*buf++ = pict->colorspace;
> >
> > Has someone confirmed that all values our enum contains or will
> > contain can just be written with no check ?
> >
> >
> Mostly see prores with Rec709, Rec601. Need to take a look for Rec2020.
> Don't know if all value are authorized, doesn't use other colorspace, with
> prores.
> 

> Do you think it's better to only authorize few colorspace ?

depends on what happens if "others" are stored.
if the official decoders fail with a blank screen then its probably
not a good idea to use such a value. If OTOH they ignore values they
do not support then it may be ok. 

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel