Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Pushed, thanks. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
On Tue, Nov 27, 2018 at 11:03:50PM +0100, Martin Vignali wrote: > utils.c |4 > 1 file changed, 4 insertions(+) > 16e7e76e9bde70f7604c3df1a5ec42828e38555b > 0001-avcodec-utils-add-YUVA444P12-and-YUVA422P12-to-pixfm.patch > From db7fb1bd052ff3dba0fe3510912214aa6e66d56b Mon Sep 17 00:00:00 2001 > From: Martin Vignali > Date: Tue, 27 Nov 2018 22:21:47 +0100 > Subject: [PATCH 01/10] avcodec/utils : add YUVA444P12 and YUVA422P12 to pixfmt > who need height padding in avcodec_align_dimensions2 > > --- > libavcodec/utils.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 1661d48b90..c4c64a6ca4 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -214,6 +214,8 @@ void avcodec_align_dimensions2(AVCodecContext *s, int > *width, int *height, > case AV_PIX_FMT_YUVA422P9BE: > case AV_PIX_FMT_YUVA422P10LE: > case AV_PIX_FMT_YUVA422P10BE: > +case AV_PIX_FMT_YUVA422P12LE: > +case AV_PIX_FMT_YUVA422P12BE: > case AV_PIX_FMT_YUVA422P16LE: > case AV_PIX_FMT_YUVA422P16BE: > case AV_PIX_FMT_YUV440P10LE: > @@ -234,6 +236,8 @@ void avcodec_align_dimensions2(AVCodecContext *s, int > *width, int *height, > case AV_PIX_FMT_YUVA444P9BE: > case AV_PIX_FMT_YUVA444P10LE: > case AV_PIX_FMT_YUVA444P10BE: > +case AV_PIX_FMT_YUVA444P12LE: > +case AV_PIX_FMT_YUVA444P12BE: > case AV_PIX_FMT_YUVA444P16LE: > case AV_PIX_FMT_YUVA444P16BE: > case AV_PIX_FMT_GBRP9LE: LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 1 "Used only once"- "Some unspecified defect prevented a second use" "In good condition" - "Can be repaird by experienced expert" "As is" - "You wouldnt want it even if you were payed for it, if you knew ..." signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Hello, New patchs in attach 001 : add YUVA444P12 and YUVA422P12 to avcodec_dimensions2 (in order to have height padding) 002 to 009 : unchanged 010 : mention ticket 7163 in commit msg (unchanged otherwise) Martin 0002-avcodec-proresdsp-remove-unused-value.patch Description: Binary data 0001-avcodec-utils-add-YUVA444P12-and-YUVA422P12-to-pixfm.patch Description: Binary data 0004-avcodec-proresdec-rename-dsp-part-for-10b-and-check-.patch Description: Binary data 0005-avcodec-proresdsp-indent-after-prev-commit.patch Description: Binary data 0003-avcodec-proresdec-move-dsp-init-after-codec-tag-chec.patch Description: Binary data 0010-avcodec-proresdec-add-12b-decoding.patch Description: Binary data 0006-avcodec-proresdec-put-unpack-alpha-func-in-prores-ct.patch Description: Binary data 0008-avcodec-proresdec-add-unpack-alpha-12-func.patch Description: Binary data 0009-avcodec-proresdec-add-12b-prores-idct.patch Description: Binary data 0007-avcodec-proresdec-make-inline-func-for-unpack-alpha.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
> > +/* align height to 16, to avoid segfault */ > > +tframe.f->height = FFALIGN(avctx->height, 16); > > +tframe.f->width = FFALIGN(avctx->width, 16); > > +tframe.f->crop_bottom = tframe.f->height - avctx->height; > > + > > if ((ret = ff_thread_get_buffer(avctx, , 0)) < 0) > > return ret; > > Why is this now needed but was not before ? > Is avcodec_align_dimensions2() correct ? > > Hello, This is the part, where i'm not really sure about. Like it's only segfault when there is alpha, but segfault on luma decoding part and alpha decoding part. Seems like, i need to add YUVA4xxP12 in avcodec_align_dimensions2, to have the correct "height padding". Will test this, and send new patch. Thanks Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
On Sat, Nov 24, 2018 at 08:59:50PM +0100, Martin Vignali wrote: > proresdec2.c |5 + > 1 file changed, 5 insertions(+) > 99ab52ec787a2de79da37daa0e17c7885fcb558f > 0010-avcodec-proresdec-align-height-buffer-to-16-avoid-se.patch > From 3c319ed4ef51e25bccfa0b4fc50edf0bcebf2f0a Mon Sep 17 00:00:00 2001 > From: Martin Vignali > Date: Sat, 17 Nov 2018 23:47:59 +0100 > Subject: [PATCH 10/11] avcodec/proresdec : align height buffer to 16 (avoid > segfault) > > --- > libavcodec/proresdec2.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c > index 8a537eed1a..f819f8db21 100644 > --- a/libavcodec/proresdec2.c > +++ b/libavcodec/proresdec2.c > @@ -750,6 +750,11 @@ static int decode_frame(AVCodecContext *avctx, void > *data, int *got_frame, > buf += frame_hdr_size; > buf_size -= frame_hdr_size; > > +/* align height to 16, to avoid segfault */ > +tframe.f->height = FFALIGN(avctx->height, 16); > +tframe.f->width = FFALIGN(avctx->width, 16); > +tframe.f->crop_bottom = tframe.f->height - avctx->height; > + > if ((ret = ff_thread_get_buffer(avctx, , 0)) < 0) > return ret; Why is this now needed but was not before ? Is avcodec_align_dimensions2() correct ? thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Martin Vignali wrote: >Only enable 12b decoding if the codec tag is Prores or XQ >let 10b decoding for 422 codecs tag. Indeed! Best regards, Reto ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Hello, New patchs in attach (002 to 010 unchanged) 011 : Remove user option for setting decoding precision Only enable 12b decoding if the codec tag is Prores or XQ let 10b decoding for 422 codecs tag. Martin 0006-avcodec-proresdec-put-unpack-alpha-func-in-prores-ct.patch Description: Binary data 0002-avcodec-proresdsp-remove-unused-value.patch Description: Binary data 0004-avcodec-proresdec-rename-dsp-part-for-10b-and-check-.patch Description: Binary data 0003-avcodec-proresdec-move-dsp-init-after-codec-tag-chec.patch Description: Binary data 0005-avcodec-proresdsp-indent-after-prev-commit.patch Description: Binary data 0009-avcodec-proresdec-add-12b-prores-idct.patch Description: Binary data 0007-avcodec-proresdec-make-inline-func-for-unpack-alpha.patch Description: Binary data 0008-avcodec-proresdec-add-unpack-alpha-12-func.patch Description: Binary data 0010-avcodec-proresdec-align-height-buffer-to-16-avoid-se.patch Description: Binary data 0011-avcodec-proresdec-add-12b-decoding.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
2018-11-18 23:50 GMT+01:00, Kieran O Leary : > On Sun, 18 Nov 2018, 21:58 Martin Vignali >> I don't know, how to get the "wanted" pix_fmt output using ffmpeg cli, >> and I don't know, what the general "rules" of this project for multiple >> pix_fmt output for the same input file. >> >> If user option for decoding precision need to be remove, i think >> 422 need to stay in 10b decoding, and 444 need to be decode in >> 12b. > > What happens if a 10-bit 444 file is decoded with 12-bit precision? > Is this just a resampling up to 12-bit for no real benefit? Iiuc, it is not possible to detect if the original input was 10 or 12 bit. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
2018-11-18 22:58 GMT+01:00, Martin Vignali : > Le dim. 18 nov. 2018 à 01:57, Carl Eugen Hoyos a > écrit : > >> 2018-11-18 0:28 GMT+01:00, Martin Vignali : >> >> > 012 : Add 12b support for 444 by default, >> >> Is it slower? >> I believe that once 12 bit decoding is not slower, it should >> be the default for 422. >> > > Yes 12b is much slower > > On a 422 HQ file : > 10b decoding precision : > frame= 3316 fps=280 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A > speed=11.2x > bench: utime=88.741s stime=1.257s rtime=11.833s > > 12b decoding precision : > frame= 3316 fps=176 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A > speed=7.02x > bench: utime=145.096s stime=0.769s rtime=18.894s Thank you for explaining and after rereading the ticket, I agree that the defaults you suggested make sense. >> > and add user option for setting decoding precision >> >> I wonder if this is necessary and correct: Calling applications >> should choose the bit depth depending on their use case, if >> ffmpeg (the cli) really doesn't support that, it is a missing >> feature, different pix_fmts for one decoder (and input file) is >> not new in FFmpeg, MPlayer had used this feature since >> forever. > > I don't know, how to get the "wanted" pix_fmt output using > ffmpeg cli, and I don't know, what the general "rules" of this > project for multiple pix_fmt output for the same input file. That's exactly what I am trying to say: We both assume that there is a bug (or missing feature) in the ffmpeg cli, but this should not be a reason to add a codec-specific option, the cli should be fixed instead. > If user option for decoding precision need to be remove, > i think 422 need to stay in 10b decoding, and 444 need to be > decode in 12b. Then the simple option may be to hardcode this pix_fmts for the moment, but maybe sorting them is already enough. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
On Sun, 18 Nov 2018, 21:58 Martin Vignali > I don't know, how to get the "wanted" pix_fmt output using ffmpeg cli, and > I don't know, what the general "rules" of this project for multiple pix_fmt > output for the same input file. > > If user option for decoding precision need to be remove, > i think 422 need to stay in 10b decoding, and 444 need to be decode in 12b. > What happens if a 10-bit 444 file is decoded with 12-bit precision? Is this just a resampling up to 12-bit for no real benefit? -Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Le dim. 18 nov. 2018 à 01:57, Carl Eugen Hoyos a écrit : > 2018-11-18 0:28 GMT+01:00, Martin Vignali : > > > 012 : Add 12b support for 444 by default, > > Is it slower? > I believe that once 12 bit decoding is not slower, it should > be the default for 422. > Yes 12b is much slower On a 422 HQ file : 10b decoding precision : frame= 3316 fps=280 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A speed=11.2x bench: utime=88.741s stime=1.257s rtime=11.833s 12b decoding precision : frame= 3316 fps=176 q=-0.0 Lsize=N/A time=00:02:12.64 bitrate=N/A speed=7.02x bench: utime=145.096s stime=0.769s rtime=18.894s > > > and add user option for setting decoding precision > > I wonder if this is necessary and correct: Calling applications > should choose the bit depth depending on their use case, if > ffmpeg (the cli) really doesn't support that, it is a missing > feature, different pix_fmts for one decoder (and input file) is > not new in FFmpeg, MPlayer had used this feature since > forever. > > I don't know, how to get the "wanted" pix_fmt output using ffmpeg cli, and I don't know, what the general "rules" of this project for multiple pix_fmt output for the same input file. If user option for decoding precision need to be remove, i think 422 need to stay in 10b decoding, and 444 need to be decode in 12b. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
> > Are all 444(4?) Encodings 12-bit? Also is there a way to verify if your > file should be decoded as 10 or 12-bit? Or does this code automate this > detection? > > The auto mode of the patch, only use codec_tag to switch between 10b and 12b, no other value. I don't think, prores have metadata for encoding precision. Apple doc say : 444 are 12b and 422 are 10b, so these value by default seems reasonable to me, even if the file is not encode with that precision (like using ffmpeg prores_aw/ks encoder) Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
2018-11-18 0:28 GMT+01:00, Martin Vignali : > 012 : Add 12b support for 444 by default, Is it slower? I believe that once 12 bit decoding is not slower, it should be the default for 422. > and add user option for setting decoding precision I wonder if this is necessary and correct: Calling applications should choose the bit depth depending on their use case, if ffmpeg (the cli) really doesn't support that, it is a missing feature, different pix_fmts for one decoder (and input file) is not new in FFmpeg, MPlayer had used this feature since forever. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Hi On Sat, 17 Nov 2018, 23:28 Martin Vignali Hello, > > Patchs in attach add 12b decoding for prores > Theses patch are based on patch by Kieran Kunhya > https://pastebin.com/mYNJkdMH > > After theses patch by default Prores 422 are decode in 10b and 444 in 12b > I add a user option, to force 10b or 12b decoding > Are all 444(4?) Encodings 12-bit? Also is there a way to verify if your file should be decoded as 10 or 12-bit? Or does this code automate this detection? This all looks very cool. Thanks, Kieran O Leary Irish Film Institute ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] avcodec/proresdec : add 12b decoding support
Hello, Patchs in attach add 12b decoding for prores Theses patch are based on patch by Kieran Kunhya https://pastebin.com/mYNJkdMH After theses patch by default Prores 422 are decode in 10b and 444 in 12b I add a user option, to force 10b or 12b decoding Need to be apply after patch "avutil - swscale : add YUVA444P12 and YUVA422P12" 003 : cosmetic, remove unused value 004 : move dsp init after codec tag check (in order to set later decoding precision based on codec_tag by default) 005/006 : rename dsp part for 10b 007/008/009 : reorganize unpack alpha and add 12b alpha decoding 010 : Add 12b idct for prores (keep the 10b version) 011 : In 12b, i have a segfault, for file with non multiple of 16 height. Don't understand why doesn't appear in 10b mode. But like several func, seems to write after the last visible line. I increase buffer height, and set crop bottom. 012 : Add 12b support for 444 by default, and add user option for setting decoding precision 013 : Add fate test for prores with 12b decoding precision Comments welcome Martin 0004-avcodec-proresdec-move-dsp-init-after-codec-tag-chec.patch Description: Binary data 0007-avcodec-proresdec-put-unpack-alpha-func-in-prores-ct.patch Description: Binary data 0003-avcodec-proresdsp-remove-unused-value.patch Description: Binary data 0006-avcodec-proresdsp-indent-after-prev-commit.patch Description: Binary data 0005-avcodec-proresdec-rename-dsp-part-for-10b-and-check.patch Description: Binary data 0008-avcodec-proresdec-make-inline-func-for-unpack-alpha.patch Description: Binary data 0012-avcodec-proresdec-add-12b-decoding.patch Description: Binary data 0011-avcodec-proresdec-align-height-buffer-to-16-avoid.patch Description: Binary data 0009-avcodec-proresdec-add-unpack-alpha-12-func.patch Description: Binary data 0010-avcodec-proresdec-add-12b-prores-idct.patch Description: Binary data 0013-fate-proresdec-add-test-for-12b-decoding.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
