Message ID | 1568131558-19018-1-git-send-email-linjie.fu@intel.com |
---|---|
State | Superseded |
Headers | show |
Am Di., 10. Sept. 2019 um 18:08 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > Add some packed pixel formats for hardware decode support > in VAAPI and QSV: > > 4:2:2 10 bit: Y210 > 4:4:4 8 bit: AYUV > 4:4:4 10 bit: Y410 Please add a short explanation (either in the commit message or only in this thread) for which kind of samples these pixel formats are needed. Or to say it differently: Why is the hardware outputting planar formats for some samples and packed for others? I see you added LE and BE versions: Why are both needed? And if they are needed, why is there only AYUV and not AYUV and VUYA? > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavutil/pixdesc.c | 62 +++++++++++++++++++++++++++++++++++++++++++ > libavutil/pixfmt.h | 9 +++++++ > libavutil/tests/pixfmt_best.c | 1 + > libavutil/version.h | 2 +- > 4 files changed, 73 insertions(+), 1 deletion(-) > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > index d78e863..0176a2a 100644 > --- a/libavutil/pixfmt.h > +++ b/libavutil/pixfmt.h > @@ -348,6 +348,12 @@ enum AVPixelFormat { > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V) > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > + AV_PIX_FMT_Y210BE, ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr, big-endian > + AV_PIX_FMT_Y210LE, ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr, little-endian I believe they are 16bpp. Carl Eugen
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Wednesday, September 11, 2019 03:25 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 1/6] lavu/pixfmt: add new pixel format > ayuv/y210/y410 > > Am Di., 10. Sept. 2019 um 18:08 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > > > Add some packed pixel formats for hardware decode support > > in VAAPI and QSV: > > > > 4:2:2 10 bit: Y210 > > 4:4:4 8 bit: AYUV > > 4:4:4 10 bit: Y410 > > Please add a short explanation (either in the commit message or > only in this thread) for which kind of samples these pixel formats > are needed. > > Or to say it differently: Why is the hardware outputting planar > formats for some samples and packed for others? I kind of remember that it was discussed in previous patch thread: Previously, media driver provided planar format(like 420 8 bit), but for HEVC Range Extension (422/444 8/10 bit), the decoded image is produced in packed format. And the reason is " because Windows expects it" as you have pointed. It could be updated in the commit message if this is good enough. > I see you added LE and BE versions: Why are both needed? > And if they are needed, why is there only AYUV and not AYUV > and VUYA? I noticed LE and BE versions are added for some of the added pixel format, out of the compatibility for big-endian and little-endian(IMHO). And that's the reason I add it for Y210 and Y410. I'm not sure I understood it correctly, LE/BE version is necessary for newly added pixel format right? > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavutil/pixdesc.c | 62 > +++++++++++++++++++++++++++++++++++++++++++ > > libavutil/pixfmt.h | 9 +++++++ > > libavutil/tests/pixfmt_best.c | 1 + > > libavutil/version.h | 2 +- > > 4 files changed, 73 insertions(+), 1 deletion(-) > > > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > > index d78e863..0176a2a 100644 > > --- a/libavutil/pixfmt.h > > +++ b/libavutil/pixfmt.h > > @@ -348,6 +348,12 @@ enum AVPixelFormat { > > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 > plane for the UV components, which are interleaved (first byte U and the > following byte V) > > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > > > + AV_PIX_FMT_Y210BE, ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr, big- > endian > > + AV_PIX_FMT_Y210LE, ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr, little- > endian > > I believe they are 16bpp. Thanks for pointing out this, and since Y210 is a 10 bit 4:2:2 format, IMHO 20bpp is expected information. (see AV_PIX_FMT_YUV422P10LE,///< planar YUV 4:2:2, 20bpp) Regards, linjie
On Wed, Sep 11, 2019 at 5:20 AM Fu, Linjie <linjie.fu@intel.com> wrote: > > > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > > Of Carl Eugen Hoyos > > Sent: Wednesday, September 11, 2019 03:25 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH 1/6] lavu/pixfmt: add new pixel format > > ayuv/y210/y410 > > > > Am Di., 10. Sept. 2019 um 18:08 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > > > > > Add some packed pixel formats for hardware decode support > > > in VAAPI and QSV: > > > > > > 4:2:2 10 bit: Y210 > > > 4:4:4 8 bit: AYUV > > > 4:4:4 10 bit: Y410 > > > > Please add a short explanation (either in the commit message or > > only in this thread) for which kind of samples these pixel formats > > are needed. > > > > Or to say it differently: Why is the hardware outputting planar > > formats for some samples and packed for others? > > I kind of remember that it was discussed in previous patch thread: > Previously, media driver provided planar format(like 420 8 bit), > but for HEVC Range Extension (422/444 8/10 bit), the decoded image > is produced in packed format. And the reason is " because Windows > expects it" as you have pointed. > > It could be updated in the commit message if this is good enough. > > > I see you added LE and BE versions: Why are both needed? > > And if they are needed, why is there only AYUV and not AYUV > > and VUYA? > > I noticed LE and BE versions are added for some of the added pixel > format, out of the compatibility for big-endian and little-endian(IMHO). > And that's the reason I add it for Y210 and Y410. > > I'm not sure I understood it correctly, LE/BE version is necessary for > newly added pixel format right? > It depends on the definition of the pixel format. AYUV is defined as 4 consecutive BYTEs, which means its identical on LE and BE. Y210 is defined as being stored as an array of 4 WORDs (Y0, U, Y1, V). For a WORD, the difference of LE/BE matters. Y410 is defined as being stored as a single DWORD, again LE/BE matters here. Obviously the differences in LE/BE need to be implemented properly as well, for Y210 for each WORD, and for Y410 for the entire DWORD. - Hendrik
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Hendrik Leppkes > Sent: Wednesday, September 11, 2019 15:28 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 1/6] lavu/pixfmt: add new pixel format > ayuv/y210/y410 > > On Wed, Sep 11, 2019 at 5:20 AM Fu, Linjie <linjie.fu@intel.com> wrote: > > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > > Of Carl Eugen Hoyos > > > Sent: Wednesday, September 11, 2019 03:25 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH 1/6] lavu/pixfmt: add new pixel > format > > > ayuv/y210/y410 > > > > > > Am Di., 10. Sept. 2019 um 18:08 Uhr schrieb Linjie Fu > <linjie.fu@intel.com>: > > > > > > > > Add some packed pixel formats for hardware decode support > > > > in VAAPI and QSV: > > > > > > > > 4:2:2 10 bit: Y210 > > > > 4:4:4 8 bit: AYUV > > > > 4:4:4 10 bit: Y410 > > > > > > Please add a short explanation (either in the commit message or > > > only in this thread) for which kind of samples these pixel formats > > > are needed. > > > > > > Or to say it differently: Why is the hardware outputting planar > > > formats for some samples and packed for others? > > > > I kind of remember that it was discussed in previous patch thread: > > Previously, media driver provided planar format(like 420 8 bit), > > but for HEVC Range Extension (422/444 8/10 bit), the decoded image > > is produced in packed format. And the reason is " because Windows > > expects it" as you have pointed. > > > > It could be updated in the commit message if this is good enough. > > > > > I see you added LE and BE versions: Why are both needed? > > > And if they are needed, why is there only AYUV and not AYUV > > > and VUYA? > > > > I noticed LE and BE versions are added for some of the added pixel > > format, out of the compatibility for big-endian and little-endian(IMHO). > > And that's the reason I add it for Y210 and Y410. > > > > I'm not sure I understood it correctly, LE/BE version is necessary for > > newly added pixel format right? > > > > It depends on the definition of the pixel format. AYUV is defined as > 4 consecutive BYTEs, which means its identical on LE and BE. > Y210 is defined as being stored as an array of 4 WORDs (Y0, U, Y1, V). > For a WORD, the difference of LE/BE matters. > Y410 is defined as being stored as a single DWORD, again LE/BE matters here. Got it. > Obviously the differences in LE/BE need to be implemented properly as > well, for Y210 for each WORD, and for Y410 for the entire DWORD. Thanks Hendrik, for the detailed explanation. It seems AV_RL16 should be used for Y210LE, but for Y410LE, AV_RL32 is needed instead. linjie
Content-Type: text/plain; charset=y error: cannot convert from y to UTF-8 fatal: could not parse patch On Wed, Sep 11, 2019 at 12:05:58AM +0800, Linjie Fu wrote: > Add some packed pixel formats for hardware decode support in VAAPI > and QSV: > > 4:2:2 10 bit: Y210 > 4:4:4 8 bit: AYUV > 4:4:4 10 bit: Y410 > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavutil/pixdesc.c | 62 +++++++++++++++++++++++++++++++++++++++++++ > libavutil/pixfmt.h | 9 +++++++ > libavutil/tests/pixfmt_best.c | 1 + > libavutil/version.h | 2 +- > 4 files changed, 73 insertions(+), 1 deletion(-) [...]
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Michael Niedermayer > Sent: Wednesday, September 11, 2019 23:06 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 1/6] lavu/pixfmt: add new pixel format > ayuv/y210/y410 > > > Content-Type: text/plain; charset=y > > error: cannot convert from y to UTF-8 > fatal: could not parse patch Occasionally meet this prompt while sending out the patch: "The following files are 8bit, but do not declare a Content-Transfer-Encoding. 0001-lavu-pixfmt-add-new-pixel-format-ayuv-y210-y410.patch Which 8bit encoding should I declare [UTF-8]?" Seems I should press enter to choose the default [UTF-8] charset instead of pressing "y". This patch has been resent, thanks. - linjie
Am Mi., 11. Sept. 2019 um 09:35 Uhr schrieb Hendrik Leppkes <h.leppkes@gmail.com>: > > On Wed, Sep 11, 2019 at 5:20 AM Fu, Linjie <linjie.fu@intel.com> wrote: > > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > > > Of Carl Eugen Hoyos > > > Sent: Wednesday, September 11, 2019 03:25 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH 1/6] lavu/pixfmt: add new pixel format > > > ayuv/y210/y410 > > > > > > Am Di., 10. Sept. 2019 um 18:08 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > > > > > > > Add some packed pixel formats for hardware decode support > > > > in VAAPI and QSV: > > > > > > > > 4:2:2 10 bit: Y210 > > > > 4:4:4 8 bit: AYUV > > > > 4:4:4 10 bit: Y410 > > > > > > Please add a short explanation (either in the commit message or > > > only in this thread) for which kind of samples these pixel formats > > > are needed. > > > > > > Or to say it differently: Why is the hardware outputting planar > > > formats for some samples and packed for others? > > > > I kind of remember that it was discussed in previous patch thread: > > Previously, media driver provided planar format(like 420 8 bit), > > but for HEVC Range Extension (422/444 8/10 bit), the decoded image > > is produced in packed format. And the reason is " because Windows > > expects it" as you have pointed. > > > > It could be updated in the commit message if this is good enough. > > > > > I see you added LE and BE versions: Why are both needed? > > > And if they are needed, why is there only AYUV and not AYUV > > > and VUYA? > > > > I noticed LE and BE versions are added for some of the added pixel > > format, out of the compatibility for big-endian and little-endian(IMHO). > > And that's the reason I add it for Y210 and Y410. > > > > I'm not sure I understood it correctly, LE/BE version is necessary for > > newly added pixel format right? > > > > It depends on the definition of the pixel format. AYUV is defined as > 4 consecutive BYTEs, which means its identical on LE and BE. > Y210 is defined as being stored as an array of 4 WORDs (Y0, U, Y1, V). > For a WORD, the difference of LE/BE matters. > Y410 is defined as being stored as a single DWORD, again LE/BE matters here. > > Obviously the differences in LE/BE need to be implemented properly as > well, for Y210 for each WORD, and for Y410 for the entire DWORD. I wonder if only the LE variants would be sufficient for the time being. Or is it (theoretically) possible to use this code on BE? Carl Eugen
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c index 05dd4a1..c2de0d8 100644 --- a/libavutil/pixdesc.c +++ b/libavutil/pixdesc.c @@ -205,6 +205,68 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 0, 4, 1, 0, 8, 3, 7, 2 }, /* V */ }, }, + [AV_PIX_FMT_Y210LE] = { + .name = "y210le", + .nb_components = 3, + .log2_chroma_w = 1, + .log2_chroma_h = 0, + .comp = { + { 0, 4, 0, 6, 10, 3, 9, 1 }, /* Y */ + { 0, 8, 2, 6, 10, 7, 9, 3 }, /* U */ + { 0, 8, 6, 6, 10, 7, 9, 7 }, /* V */ + }, + }, + [AV_PIX_FMT_Y210BE] = { + .name = "y210be", + .nb_components = 3, + .log2_chroma_w = 1, + .log2_chroma_h = 0, + .comp = { + { 0, 4, 0, 6, 10, 3, 9, 1 }, /* Y */ + { 0, 8, 2, 6, 10, 7, 9, 3 }, /* U */ + { 0, 8, 6, 6, 10, 7, 9, 7 }, /* V */ + }, + .flags = AV_PIX_FMT_FLAG_BE, + }, + [AV_PIX_FMT_AYUV] = { + .name = "ayuv", + .nb_components = 4, + .log2_chroma_w = 0, + .log2_chroma_h = 0, + .comp = { + { 0, 4, 1, 0, 8, 3, 7, 2 }, /* Y */ + { 0, 4, 2, 0, 8, 3, 7, 3 }, /* U */ + { 0, 4, 3, 0, 8, 3, 7, 4 }, /* V */ + { 0, 4, 0, 0, 8, 3, 7, 1 }, /* A */ + }, + .flags = AV_PIX_FMT_FLAG_ALPHA, + }, + [AV_PIX_FMT_Y410LE] = { + .name = "y410le", + .nb_components = 4, + .log2_chroma_w = 0, + .log2_chroma_h = 0, + .comp = { + { 0, 32, 10, 0, 10, 31, 9, 11 }, /* Y */ + { 0, 32, 0, 0, 10, 31, 9, 1 }, /* U */ + { 0, 32, 20, 0, 10, 31, 9, 21 }, /* V */ + { 0, 32, 30, 0, 2, 31, 1, 31 }, /* A */ + }, + .flags = AV_PIX_FMT_FLAG_ALPHA | AV_PIX_FMT_FLAG_BITSTREAM, + }, + [AV_PIX_FMT_Y410BE] = { + .name = "y410be", + .nb_components = 4, + .log2_chroma_w = 0, + .log2_chroma_h = 0, + .comp = { + { 0, 32, 10, 0, 10, 31, 9, 11 }, /* Y */ + { 0, 32, 0, 0, 10, 31, 9, 1 }, /* U */ + { 0, 32, 20, 0, 10, 31, 9, 21 }, /* V */ + { 0, 32, 30, 0, 2, 31, 1, 31 }, /* A */ + }, + .flags = AV_PIX_FMT_FLAG_ALPHA | AV_PIX_FMT_FLAG_BITSTREAM | AV_PIX_FMT_FLAG_BE, + }, [AV_PIX_FMT_RGB24] = { .name = "rgb24", .nb_components = 3, diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h index d78e863..0176a2a 100644 --- a/libavutil/pixfmt.h +++ b/libavutil/pixfmt.h @@ -348,6 +348,12 @@ enum AVPixelFormat { AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V) AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped + AV_PIX_FMT_Y210BE, ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr, big-endian + AV_PIX_FMT_Y210LE, ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr, little-endian + AV_PIX_FMT_AYUV, ///< packed YUV 4:4:4, 32bpp, A Y Cb Cr, + AV_PIX_FMT_Y410LE, ///< packed YUV 4:4:4, 32bpp, Cr Y Cb A, little-endian + AV_PIX_FMT_Y410BE, ///< packed YUV 4:4:4, 32bpp, Cr Y Cb A, 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 }; @@ -436,6 +442,9 @@ enum AVPixelFormat { #define AV_PIX_FMT_P010 AV_PIX_FMT_NE(P010BE, P010LE) #define AV_PIX_FMT_P016 AV_PIX_FMT_NE(P016BE, P016LE) +#define AV_PIX_FMT_Y210 AV_PIX_FMT_NE(Y210BE, Y210LE) +#define AV_PIX_FMT_Y410 AV_PIX_FMT_NE(Y410BE, Y410LE) + /** * Chromaticity coordinates of the source primaries. * These values match the ones defined by ISO/IEC 23001-8_2013 ยง 7.1. diff --git a/libavutil/tests/pixfmt_best.c b/libavutil/tests/pixfmt_best.c index 53f7264..2939e48 100644 --- a/libavutil/tests/pixfmt_best.c +++ b/libavutil/tests/pixfmt_best.c @@ -91,6 +91,7 @@ int main(void) TEST(AV_PIX_FMT_YUVA420P, AV_PIX_FMT_YUV420P); TEST(AV_PIX_FMT_YUVA422P, AV_PIX_FMT_YUV422P); TEST(AV_PIX_FMT_YUVA444P, AV_PIX_FMT_YUV444P); + TEST(AV_PIX_FMT_AYUV, AV_PIX_FMT_YUV444P); TEST(AV_PIX_FMT_AYUV64, AV_PIX_FMT_YUV444P16); TEST(AV_PIX_FMT_RGBA, AV_PIX_FMT_RGB24); TEST(AV_PIX_FMT_ABGR, AV_PIX_FMT_RGB24); diff --git a/libavutil/version.h b/libavutil/version.h index 3395769..af3abf7 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 56 -#define LIBAVUTIL_VERSION_MINOR 35 +#define LIBAVUTIL_VERSION_MINOR 36 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
Add some packed pixel formats for hardware decode support in VAAPI and QSV: 4:2:2 10 bit: Y210 4:4:4 8 bit: AYUV 4:4:4 10 bit: Y410 Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavutil/pixdesc.c | 62 +++++++++++++++++++++++++++++++++++++++++++ libavutil/pixfmt.h | 9 +++++++ libavutil/tests/pixfmt_best.c | 1 + libavutil/version.h | 2 +- 4 files changed, 73 insertions(+), 1 deletion(-)