Re: [FFmpeg-devel] avcodec/proresenc_aw improvements
> > >if avtc->profile < 0 or > 4, return an error. > > Should 5 not become ProRes XQ (ap4x) as in prores_ks? > > Yes, plan to add it later. Need some test before. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw improvements
Martin Vignali wrote: >if avtc->profile < 0 or > 4, return an error. Should 5 not become ProRes XQ (ap4x) as in prores_ks? Best regards, Reto ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw improvements
> > See coverity bug report, avctx->profile is not checked for valid values i > think. > > Hello, Doesn't find where avctx->profile can have an invalid value seems to be checked in prores_encode_init if FF_PROFILE_UNKNOWN use pix fmt YUV422P10 or YUV444P10 to select the profile (can pix_fmt be another value here ?), if yes, will add an else part here if avtc->profile < 0 or > 4, return an error. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw improvements
> > > > > 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. > Seems like not all prores decoder use these values. Will check, with offical decoder. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresenc_aw improvements
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
Re: [FFmpeg-devel] avcodec/proresenc_aw improvements
> > > 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] avcodec/proresenc_aw improvements
On Sat, Jul 21, 2018 at 02:33:24PM +0200, Martin Vignali wrote: [...] > 004 : calculate dct part before the quantif search. Avoid to recalculate it > each time > On a basic test Prores decoding -> prores encoding, the global speed > improvment is around 2% nice speedup [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "I am not trying to be anyone's saviour, I'm trying to think about the future and not be sad" - Elon Musk 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
On Sat, Jul 21, 2018 at 02:33:24PM +0200, Martin Vignali wrote:
> Hello,
>
> Patch in attach improve some part of the prores_aw encoder.
>
> 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 ?
> 002 : use for the frame header, primaries, transfert, colorspace like in
> proresenc_ks
> avoid color shift on some software (update fate test)
> 003 : change qp start value for each profile based on official encoder
> output
> and update proxy qp end to a bigger value (also based on official encoder
> output)
> (update fate test)
> 004 : calculate dct part before the quantif search. Avoid to recalculate it
> each time
> On a basic test Prores decoding -> prores encoding, the global speed
> improvment is around 2%
>
> This encoder is cover by 4 fate test, can be run with :
> make fate-vsynth3-prores;make fate-vsynth2-prores;make
> fate-vsynth1-prores;make fate-vsynth_lena-prores SAMPLES=fate-suite/
>
> Plan to add later more part from the ks encoder to this one
>
> Martin
[...]
> libavcodec/proresenc_anatoliy.c |6 +++---
> tests/ref/vsynth/vsynth1-prores |2 +-
> tests/ref/vsynth/vsynth2-prores |2 +-
> tests/ref/vsynth/vsynth3-prores |2 +-
> tests/ref/vsynth/vsynth_lena-prores |2 +-
> 5 files changed, 7 insertions(+), 7 deletions(-)
> f469d157875b5b92fd7053dde08c226296c0cf6b
> 0002-avcodec-proresenc_aw-use-AVframe-primaries-transfert.patch
> From 2891017d83b9a0e10c912d68bc64f67e960bff38 Mon Sep 17 00:00:00 2001
> From: Martin Vignali
> Date: Sat, 21 Jul 2018 14:10:07 +0200
> Subject: [PATCH 2/4] avcodec/proresenc_aw : use AVframe primaries, transfert,
> colorspace for frame header instead of default (unknown, unknown, Rec601)
>
> avoid color shift, on some decoding software
> ---
> libavcodec/proresenc_anatoliy.c | 6 +++---
> tests/ref/vsynth/vsynth1-prores | 2 +-
> tests/ref/vsynth/vsynth2-prores | 2 +-
> tests/ref/vsynth/vsynth3-prores | 2 +-
> tests/ref/vsynth/vsynth_lena-prores | 2 +-
> 5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
> index dd6b1dcfb1..91b9a17947 100644
> --- a/libavcodec/proresenc_anatoliy.c
> +++ b/libavcodec/proresenc_anatoliy.c
> @@ -501,9 +501,9 @@ static int prores_encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
> bytestream_put_be16(, avctx->height);
> *buf++ = 0x83; // {10}(422){00}{00}(frame){11}
> *buf++ = 0;
> -*buf++ = 2;
> -*buf++ = 2;
> -*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 ?
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
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
> > Am I correct that the output files also change? > Patch 002 : change the header of the output file only, not the compress data. Patch 003 : change the output file mainly for near uniform bloc, where the low scale quantif value where used. But IMHO is better by default to be close to official encoding. I can maybe add later an option to let user use another min_quantif_scale in order to use 1 for example if need. Patch 004 : Doesn't change the output file 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-21 14:33 GMT+02:00, Martin Vignali : > Patch in attach improve some part of the prores_aw encoder. > > 001 : use scantable in prores_data instead of a duplicate one. > 002 : use for the frame header, primaries, transfert, colorspace like in > proresenc_ks avoid color shift on some software (update fate test) > 003 : change qp start value for each profile based on official encoder > output and update proxy qp end to a bigger value (also based on > official encoder output) > (update fate test) > 004 : calculate dct part before the quantif search. Avoid to recalculate > it each time > On a basic test Prores decoding -> prores encoding, the global speed > improvment is around 2% Am I correct that the output files also change? What about PSNR or SSIM or visual quality? The reason I ask is that iirc, PSNR was a little worse with ks. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] avcodec/proresenc_aw improvements
Hello, Patch in attach improve some part of the prores_aw encoder. 001 : use scantable in prores_data instead of a duplicate one. 002 : use for the frame header, primaries, transfert, colorspace like in proresenc_ks avoid color shift on some software (update fate test) 003 : change qp start value for each profile based on official encoder output and update proxy qp end to a bigger value (also based on official encoder output) (update fate test) 004 : calculate dct part before the quantif search. Avoid to recalculate it each time On a basic test Prores decoding -> prores encoding, the global speed improvment is around 2% This encoder is cover by 4 fate test, can be run with : make fate-vsynth3-prores;make fate-vsynth2-prores;make fate-vsynth1-prores;make fate-vsynth_lena-prores SAMPLES=fate-suite/ Plan to add later more part from the ks encoder to this one Martin 0001-avcodec-proresenc_aw-use-scan-table-from-prores_data.patch Description: Binary data 0002-avcodec-proresenc_aw-use-AVframe-primaries-transfert.patch Description: Binary data 0003-avcodec-proresenc_aw-use-qp-close-to-the-official-en.patch Description: Binary data 0004-avcodec-prores_enc-not-calculate-dct-a-each-quantif.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
