Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On Tue, 9 Oct 2018, Timo Rothenpieler wrote: On 09/10/2018 02:32, Carl Eugen Hoyos wrote: 2018-10-08 21:59 GMT+02:00, Timo Rothenpieler : So, to be able to use these hardware compatible formats, we need definitions for them in ffmpeg. Right now, I need this for nvdec, but Vulkan also uses the same format definitions. Sorry if this was already done and I forgot but please explain why you cannot use YUV444P16 for this use-case. If the only thing missing is libavfilter understanding bits_per_raw_sample we should add it there: It is needed in any case and would save us a few pix_fmts and could speed up many use cases compared to your solution. It's pretty much all of ffmpeg and all client applications understanding it. First of all, bits_per_raw_sample, or an equivalent bit depth field, is missing from AVFrame. "In" avfilter it's primarily swscale I guess, but it also affects each and every filter supporting YUV444P16. Also, for encoders, it would mean they need a way to indicate what bit depths they can take, and that also has to be per pix_fmt they support. Then all of avcodec, ffmpeg.c, ... needs support for that. It's quite a deep rabit hole of changes with quite a good chance of something going horribly wrong in the process. Probably even in more places than just the ones mentioned here. This may or may not be true, unfortunately my question was not how difficult it would be to fix this mess (I don't think it is), but why you want to add more to the mess. What would happen if you just use YUV444P16? For example nvenc only supports 10 bit encoding, but takes it in the YUV444P16 format. The rest of ffmpeg has no idea it only supports 10 bit, and at times even up-converts 10 bit content to 16 bit via swscale due to that. Isn't the same thing happening for audio with s24le codec? What is the downside of extending the precision of the filtering pipeline to full 16 bit in cases like this? I don't think it affects most filters speedwise. Also, it loses quality without the user noticing, as there is no outside indication it does in fact not support 16 bit. It's up to the tool to be user friendly, not the framework. I still think proper implementation is hard (even the YUVJ removal patch got abandoned), so I don't know how to move forward. Maybe we should come up with some issue and requirement list first, and decide after that. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On 09/10/2018 02:32, Carl Eugen Hoyos wrote: 2018-10-08 21:59 GMT+02:00, Timo Rothenpieler : So, to be able to use these hardware compatible formats, we need definitions for them in ffmpeg. Right now, I need this for nvdec, but Vulkan also uses the same format definitions. Sorry if this was already done and I forgot but please explain why you cannot use YUV444P16 for this use-case. If the only thing missing is libavfilter understanding bits_per_raw_sample we should add it there: It is needed in any case and would save us a few pix_fmts and could speed up many use cases compared to your solution. It's pretty much all of ffmpeg and all client applications understanding it. First of all, bits_per_raw_sample, or an equivalent bit depth field, is missing from AVFrame. "In" avfilter it's primarily swscale I guess, but it also affects each and every filter supporting YUV444P16. Also, for encoders, it would mean they need a way to indicate what bit depths they can take, and that also has to be per pix_fmt they support. Then all of avcodec, ffmpeg.c, ... needs support for that. It's quite a deep rabit hole of changes with quite a good chance of something going horribly wrong in the process. Probably even in more places than just the ones mentioned here. This may or may not be true, unfortunately my question was not how difficult it would be to fix this mess (I don't think it is), but why you want to add more to the mess. What would happen if you just use YUV444P16? For example nvenc only supports 10 bit encoding, but takes it in the YUV444P16 format. The rest of ffmpeg has no idea it only supports 10 bit, and at times even up-converts 10 bit content to 16 bit via swscale due to that. Also, it loses quality without the user noticing, as there is no outside indication it does in fact not support 16 bit. smime.p7s Description: S/MIME Cryptographic Signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
2018-10-08 21:59 GMT+02:00, Timo Rothenpieler : >>> So, to be able to use these hardware compatible formats, we need >>> definitions for them in ffmpeg. Right now, I need this for nvdec, >>> but Vulkan also uses the same format definitions. >> >> Sorry if this was already done and I forgot but please explain why >> you cannot use YUV444P16 for this use-case. >> >> If the only thing missing is libavfilter understanding >> bits_per_raw_sample >> we should add it there: It is needed in any case and would save us a few >> pix_fmts and could speed up many use cases compared to your solution. > > It's pretty much all of ffmpeg and all client applications understanding > it. > First of all, bits_per_raw_sample, or an equivalent bit depth field, is > missing from AVFrame. > "In" avfilter it's primarily swscale I guess, but it also affects each > and every filter supporting YUV444P16. > Also, for encoders, it would mean they need a way to indicate what bit > depths they can take, and that also has to be per pix_fmt they support. > Then all of avcodec, ffmpeg.c, ... needs support for that. > It's quite a deep rabit hole of changes with quite a good chance of > something going horribly wrong in the process. Probably even in more > places than just the ones mentioned here. This may or may not be true, unfortunately my question was not how difficult it would be to fix this mess (I don't think it is), but why you want to add more to the mess. What would happen if you just use YUV444P16? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
So, to be able to use these hardware compatible formats, we need definitions for them in ffmpeg. Right now, I need this for nvdec, but Vulkan also uses the same format definitions. Sorry if this was already done and I forgot but please explain why you cannot use YUV444P16 for this use-case. If the only thing missing is libavfilter understanding bits_per_raw_sample we should add it there: It is needed in any case and would save us a few pix_fmts and could speed up many use cases compared to your solution. It's pretty much all of ffmpeg and all client applications understanding it. First of all, bits_per_raw_sample, or an equivalent bit depth field, is missing from AVFrame. "In" avfilter it's primarily swscale I guess, but it also affects each and every filter supporting YUV444P16. Also, for encoders, it would mean they need a way to indicate what bit depths they can take, and that also has to be per pix_fmt they support. Then all of avcodec, ffmpeg.c, ... needs support for that. It's quite a deep rabit hole of changes with quite a good chance of something going horribly wrong in the process. Probably even in more places than just the ones mentioned here. smime.p7s Description: S/MIME Cryptographic Signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
2018-10-07 19:50 GMT+02:00, Philip Langdale : > Currently, ffmpeg defines a set of YUV444P formats for use where > the bits-per-pixel are between 8 and 16 bits. In these formats, > the bits are packed in the MSBs of the 16 bits of available storage. > > On the other hand, all the hardware vendors have defined their > equivalent formats with the bits packed in the LSBs, which has the > virtue of making the memory layouts compatible with being treated > as full 16 bit values (which is also why P010 is defined this way). (This does not sound like a good reason but this is not related to this patch.) > So, to be able to use these hardware compatible formats, we need > definitions for them in ffmpeg. Right now, I need this for nvdec, > but Vulkan also uses the same format definitions. Sorry if this was already done and I forgot but please explain why you cannot use YUV444P16 for this use-case. If the only thing missing is libavfilter understanding bits_per_raw_sample we should add it there: It is needed in any case and would save us a few pix_fmts and could speed up many use cases compared to your solution. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On Mon, 8 Oct 2018 11:55:09 +0200 Hendrik Leppkes wrote: > On Mon, Oct 8, 2018 at 10:40 AM Timo Rothenpieler > wrote: > > > > > > I don't think it's YUVJ all over again, as it was the exact same bit > > layout than normal YUV, while this one is actually different. > > > > Its also the same bit layout as YUV444P16, with one piece of > additional metadata tacked on top - seems quite similar to YUVJ to me. So, this argument applies equally to P010 vs P016 doesn't it? Yet we all think P010 is a completely reasonable format to have, and in fact when I added it to cuvid/nvdec originally, I only did it as P016, and we all agreed that it should explicitly handle P010. I don't have a strong opinion on this one - I first wrote this patch as YUV444P16 as well and Timo asked for the explicit formats - but I think the consistency argument makes sense. And yes, I messed up the names. I renamed all to _MSB and have updated locally. Will repost after we're agreed on the principle. Thanks, --phil ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On Mon, Oct 8, 2018 at 10:40 AM Timo Rothenpieler wrote: > > > I don't think it's YUVJ all over again, as it was the exact same bit > layout than normal YUV, while this one is actually different. > Its also the same bit layout as YUV444P16, with one piece of additional metadata tacked on top - seems quite similar to YUVJ to me. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On 08/10/2018 09:24, Marton Balint wrote: On Sun, 7 Oct 2018, Philip Langdale wrote: Currently, ffmpeg defines a set of YUV444P formats for use where the bits-per-pixel are between 8 and 16 bits. In these formats, the bits are packed in the MSBs of the 16 bits of available storage. On the other hand, all the hardware vendors have defined their equivalent formats with the bits packed in the LSBs, which has the virtue of making the memory layouts compatible with being treated as full 16 bit values (which is also why P010 is defined this way). So, to be able to use these hardware compatible formats, we need definitions for them in ffmpeg. Right now, I need this for nvdec, but Vulkan also uses the same format definitions. MSB and LSB is mixed up as far as I understand. In classic formats ffmpeg stores data in low bits, the least significant bits of 16 bit values. In P010 and so on the docs says "zeros are in the low bits", therefore data is in the high bits, which are the most significant ones. Am I missing something? Otherwise, I dont't disagree that this is the fastest/easiest/least-error-prone way to add support for this, but it sure feels like JUVJ formats all over again. Yes, the padding is in the LSB, data in MSB. _MSB is probably the better name then. I don't think it's YUVJ all over again, as it was the exact same bit layout than normal YUV, while this one is actually different. smime.p7s Description: S/MIME Cryptographic Signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On Sun, 7 Oct 2018, Philip Langdale wrote: Currently, ffmpeg defines a set of YUV444P formats for use where the bits-per-pixel are between 8 and 16 bits. In these formats, the bits are packed in the MSBs of the 16 bits of available storage. On the other hand, all the hardware vendors have defined their equivalent formats with the bits packed in the LSBs, which has the virtue of making the memory layouts compatible with being treated as full 16 bit values (which is also why P010 is defined this way). So, to be able to use these hardware compatible formats, we need definitions for them in ffmpeg. Right now, I need this for nvdec, but Vulkan also uses the same format definitions. MSB and LSB is mixed up as far as I understand. In classic formats ffmpeg stores data in low bits, the least significant bits of 16 bit values. In P010 and so on the docs says "zeros are in the low bits", therefore data is in the high bits, which are the most significant ones. Am I missing something? Otherwise, I dont't disagree that this is the fastest/easiest/least-error-prone way to add support for this, but it sure feels like JUVJ formats all over again. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On Sun, Oct 7, 2018 at 7:51 PM Philip Langdale wrote: > > Currently, ffmpeg defines a set of YUV444P formats for use where > the bits-per-pixel are between 8 and 16 bits. In these formats, > the bits are packed in the MSBs of the 16 bits of available storage. > > On the other hand, all the hardware vendors have defined their > equivalent formats with the bits packed in the LSBs, which has the > virtue of making the memory layouts compatible with being treated > as full 16 bit values (which is also why P010 is defined this way). > Don't you have that flipped around? In our default formats, the data is in LSB, padding in MSB. In these hardware formats, data is in MSB, padding in LSB. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
On 07.10.2018 19:50, Philip Langdale wrote: Currently, ffmpeg defines a set of YUV444P formats for use where the bits-per-pixel are between 8 and 16 bits. In these formats, the bits are packed in the MSBs of the 16 bits of available storage. On the other hand, all the hardware vendors have defined their equivalent formats with the bits packed in the LSBs, which has the virtue of making the memory layouts compatible with being treated as full 16 bit values (which is also why P010 is defined this way). So, to be able to use these hardware compatible formats, we need definitions for them in ffmpeg. Right now, I need this for nvdec, but Vulkan also uses the same format definitions. I'm very much in favor of adding those, and potentially similar other ones in the future. The other option is to add a bit depth field to AVFrame, and changes in various places to support it, including logic for encoder to signal support for pix_fmt/bit_depth support. The later approach, while it does seem cleaner, also seems very error prone and invasive, so I'd prefer adding the few cases we have as pixel formats. smime.p7s Description: S/MIME Cryptographic Signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/5] avutil: Add YUV444P10_LSB and YUV444P12_LSB pixel formats
Currently, ffmpeg defines a set of YUV444P formats for use where
the bits-per-pixel are between 8 and 16 bits. In these formats,
the bits are packed in the MSBs of the 16 bits of available storage.
On the other hand, all the hardware vendors have defined their
equivalent formats with the bits packed in the LSBs, which has the
virtue of making the memory layouts compatible with being treated
as full 16 bit values (which is also why P010 is defined this way).
So, to be able to use these hardware compatible formats, we need
definitions for them in ffmpeg. Right now, I need this for nvdec,
but Vulkan also uses the same format definitions.
Signed-off-by: Philip Langdale
---
libavutil/pixdesc.c | 48 +
libavutil/pixfmt.h | 8
libavutil/version.h | 4 ++--
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 970a83214c..c48a100907 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -1629,6 +1629,54 @@ static const AVPixFmtDescriptor
av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
},
.flags = AV_PIX_FMT_FLAG_BE | AV_PIX_FMT_FLAG_PLANAR,
},
+[AV_PIX_FMT_YUV444P10LE_LSB] = {
+.name = "yuv444p10lelsb",
+.nb_components = 3,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 0, 2, 0, 6, 10, 1, 9, 1 },/* Y */
+{ 1, 2, 0, 6, 10, 1, 9, 1 },/* U */
+{ 2, 2, 0, 6, 10, 1, 9, 1 },/* V */
+},
+.flags = AV_PIX_FMT_FLAG_PLANAR,
+},
+[AV_PIX_FMT_YUV444P10BE_LSB] = {
+.name = "yuv444p10belsb",
+.nb_components = 3,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 0, 2, 0, 6, 10, 1, 9, 1 },/* Y */
+{ 1, 2, 0, 6, 10, 1, 9, 1 },/* U */
+{ 2, 2, 0, 6, 10, 1, 9, 1 },/* V */
+},
+.flags = AV_PIX_FMT_FLAG_BE | AV_PIX_FMT_FLAG_PLANAR,
+},
+[AV_PIX_FMT_YUV444P12LE_LSB] = {
+.name = "yuv444p12lelsb",
+.nb_components = 3,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 0, 2, 0, 4, 12, 1, 11, 1 },/* Y */
+{ 1, 2, 0, 4, 12, 1, 11, 1 },/* U */
+{ 2, 2, 0, 4, 12, 1, 11, 1 },/* V */
+},
+.flags = AV_PIX_FMT_FLAG_PLANAR,
+},
+[AV_PIX_FMT_YUV444P12BE_LSB] = {
+.name = "yuv444p12belsb",
+.nb_components = 3,
+.log2_chroma_w = 0,
+.log2_chroma_h = 0,
+.comp = {
+{ 0, 2, 0, 4, 12, 1, 11, 1 },/* Y */
+{ 1, 2, 0, 4, 12, 1, 11, 1 },/* U */
+{ 2, 2, 0, 4, 12, 1, 11, 1 },/* V */
+},
+.flags = AV_PIX_FMT_FLAG_BE | AV_PIX_FMT_FLAG_PLANAR,
+},
[AV_PIX_FMT_D3D11VA_VLD] = {
.name = "d3d11va_vld",
.log2_chroma_w = 1,
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index 6815f8dc7b..910cb0a995 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -340,6 +340,11 @@ enum AVPixelFormat {
AV_PIX_FMT_GRAYF32BE, ///< IEEE-754 single precision Y, 32bpp, big-endian
AV_PIX_FMT_GRAYF32LE, ///< IEEE-754 single precision Y, 32bpp,
little-endian
+AV_PIX_FMT_YUV444P10LE_LSB, ///< planar YUV 4:4:4, 30bpp, (1 Cr & Cb
sample per 1x1 Y samples), LSB packed, little-endian
+AV_PIX_FMT_YUV444P10BE_LSB, ///< planar YUV 4:4:4, 30bpp, (1 Cr & Cb
sample per 1x1 Y samples), LSB packed, big-endian
+AV_PIX_FMT_YUV444P12LE_LSB, ///< planar YUV 4:4:4, 36bpp, (1 Cr & Cb
sample per 1x1 Y samples), LSB packed, little-endian
+AV_PIX_FMT_YUV444P12BE_LSB, ///< planar YUV 4:4:4, 36bpp, (1 Cr & Cb
sample per 1x1 Y samples), LSB packed, big-endian
+
AV_PIX_FMT_NB ///< number of pixel formats, DO NOT USE THIS if you
want to link with shared libav* because the number of formats might differ
between versions
};
@@ -391,6 +396,9 @@ enum AVPixelFormat {
#define AV_PIX_FMT_YUV422P16 AV_PIX_FMT_NE(YUV422P16BE, YUV422P16LE)
#define AV_PIX_FMT_YUV444P16 AV_PIX_FMT_NE(YUV444P16BE, YUV444P16LE)
+#define AV_PIX_FMT_YUV444P10_LSB AV_PIX_FMT_NE(YUV444P10BE_LSB,
YUV444P10LE_LSB)
+#define AV_PIX_FMT_YUV444P12_LSB AV_PIX_FMT_NE(YUV444P12BE_LSB,
YUV444P12LE_LSB)
+
#define AV_PIX_FMT_GBRP9 AV_PIX_FMT_NE(GBRP9BE ,GBRP9LE)
#define AV_PIX_FMT_GBRP10AV_PIX_FMT_NE(GBRP10BE,GBRP10LE)
#define AV_PIX_FMT_GBRP12AV_PIX_FMT_NE(GBRP12BE,GBRP12LE)
diff --git a/libavutil/version.h b/libavutil/version.h
index f84ec89154..377714a91e 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,8 +79,8 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 56
-#define LIBAVUTIL_VERSION_MINOR 19
-#define LIBAVUTIL_VERSION_MICRO 101
+#define LIBAVUTIL_VERSION_MINOR 20
+#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_M
