diff mbox

[FFmpeg-devel,1/6] lavu/pixfmt: add new pixel format ayuv/y210/y410

Message ID 1568131558-19018-1-git-send-email-linjie.fu@intel.com
State Superseded
Headers show

Commit Message

Fu, Linjie Sept. 10, 2019, 4:05 p.m. UTC
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(-)

Comments

Carl Eugen Hoyos Sept. 10, 2019, 7:24 p.m. UTC | #1
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
Fu, Linjie Sept. 11, 2019, 3:19 a.m. UTC | #2
> -----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
Hendrik Leppkes Sept. 11, 2019, 7:27 a.m. UTC | #3
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
Fu, Linjie Sept. 11, 2019, 8:40 a.m. UTC | #4
> -----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
Michael Niedermayer Sept. 11, 2019, 3:06 p.m. UTC | #5
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(-)

[...]
Fu, Linjie Sept. 11, 2019, 4:40 p.m. UTC | #6
> -----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
Carl Eugen Hoyos Sept. 11, 2019, 8:31 p.m. UTC | #7
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 mbox

Patch

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, \