diff mbox series

[FFmpeg-devel,v1,1/9] lavu/pix_fmt: add P012 pixel format

Message ID 20200619015248.21873-1-fei.w.wang@intel.com
State New
Headers show
Series [FFmpeg-devel,v1,1/9] lavu/pix_fmt: add P012 pixel format
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Fei Wang June 19, 2020, 1:52 a.m. UTC 
P012 is 12bit planner format which is similar to NV12. It using two
bytes to store 12bit valid data and 4bit zero in LSB. This format
will be used for hardware decode/encode in VAAPI and QSV.

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
 libavutil/pixdesc.c              | 24 ++++++++++++++++++++++++
 libavutil/pixfmt.h               |  5 +++++
 libavutil/version.h              |  2 +-
 tests/ref/fate/sws-pixdesc-query | 11 +++++++++++
 4 files changed, 41 insertions(+), 1 deletion(-)

Comments

Hendrik Leppkes June 19, 2020, 7:21 a.m. UTC | #1 
On Fri, Jun 19, 2020 at 3:53 AM Fei Wang <fei.w.wang@intel.com> wrote:
>
> P012 is 12bit planner format which is similar to NV12. It using two
> bytes to store 12bit valid data and 4bit zero in LSB. This format
> will be used for hardware decode/encode in VAAPI and QSV.
>

P012 is not required, you can just use P016. All these P*  formats
have the advantage of being aligned to MSB, which means the bitdepth
does not matter for their layout.
Instead you can use P016 and if you must know, look at a bitdepth
value separately.

- Hendrik
Fei Wang June 19, 2020, 8:26 a.m. UTC | #2 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Hendrik Leppkes
> Sent: Friday, June 19, 2020 3:21 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v1 1/9] lavu/pix_fmt: add P012 pixel
> format
> 
> On Fri, Jun 19, 2020 at 3:53 AM Fei Wang <fei.w.wang@intel.com> wrote:
> >
> > P012 is 12bit planner format which is similar to NV12. It using two
> > bytes to store 12bit valid data and 4bit zero in LSB. This format will
> > be used for hardware decode/encode in VAAPI and QSV.
> >
> 
> P012 is not required, you can just use P016. All these P*  formats have the
> advantage of being aligned to MSB, which means the bitdepth does not matter
> for their layout.
> Instead you can use P016 and if you must know, look at a bitdepth value
> separately.
If using P016 instead of P012 for 12bit clips, that means for 16bit clips we also
need to use P016, which will bring the conflict with current VAAPI logic. It need
to resolve same av format map to different RT format and VA format.
@Mark Thompson, how about your opinion of this?

The other reason that I used P012 is that P010 also existed in ffempg, but not
replaced by P016.

Fei

> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
Carl Eugen Hoyos June 19, 2020, 4:58 p.m. UTC | #3 
Am Fr., 19. Juni 2020 um 10:27 Uhr schrieb Wang, Fei W <fei.w.wang@intel.com>:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Hendrik Leppkes
> > Sent: Friday, June 19, 2020 3:21 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v1 1/9] lavu/pix_fmt: add P012 pixel
> > format
> >
> > On Fri, Jun 19, 2020 at 3:53 AM Fei Wang <fei.w.wang@intel.com> wrote:
> > >
> > > P012 is 12bit planner format which is similar to NV12. It using two
> > > bytes to store 12bit valid data and 4bit zero in LSB. This format will
> > > be used for hardware decode/encode in VAAPI and QSV.
> > >
> >
> > P012 is not required, you can just use P016. All these P*  formats have the
> > advantage of being aligned to MSB, which means the bitdepth does not matter
> > for their layout.
> > Instead you can use P016 and if you must know, look at a bitdepth value
> > separately.
> If using P016 instead of P012 for 12bit clips, that means for 16bit clips we also
> need to use P016, which will bring the conflict with current VAAPI logic.

Then fix the logic, it is easy to detect how many significant bits the
data contains.

Carl Eugen
Fei Wang June 22, 2020, 1:38 a.m. UTC | #4 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Carl
> Eugen Hoyos
> Sent: Saturday, June 20, 2020 12:59 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v1 1/9] lavu/pix_fmt: add P012 pixel
> format
> 
> Am Fr., 19. Juni 2020 um 10:27 Uhr schrieb Wang, Fei W
> <fei.w.wang@intel.com>:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Hendrik Leppkes
> > > Sent: Friday, June 19, 2020 3:21 PM
> > > To: FFmpeg development discussions and patches
> > > <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH v1 1/9] lavu/pix_fmt: add P012
> > > pixel format
> > >
> > > On Fri, Jun 19, 2020 at 3:53 AM Fei Wang <fei.w.wang@intel.com> wrote:
> > > >
> > > > P012 is 12bit planner format which is similar to NV12. It using
> > > > two bytes to store 12bit valid data and 4bit zero in LSB. This
> > > > format will be used for hardware decode/encode in VAAPI and QSV.
> > > >
> > >
> > > P012 is not required, you can just use P016. All these P*  formats
> > > have the advantage of being aligned to MSB, which means the bitdepth
> > > does not matter for their layout.
> > > Instead you can use P016 and if you must know, look at a bitdepth
> > > value separately.
> > If using P016 instead of P012 for 12bit clips, that means for 16bit
> > clips we also need to use P016, which will bring the conflict with current
> VAAPI logic.
> 
> Then fix the logic, it is easy to detect how many significant bits the data
> contains.
Yes, it is not hard to refine the logic, and before the action I'd like to listen
From Mark Thompson, the maintainer of VAAPI to reach an agreement. 

Kindly Ping @Mark Thompson 

> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
Mark Thompson June 28, 2020, 9:01 p.m. UTC | #5 
On 19/06/2020 08:21, Hendrik Leppkes wrote:
> On Fri, Jun 19, 2020 at 3:53 AM Fei Wang <fei.w.wang@intel.com> wrote:
>>
>> P012 is 12bit planner format which is similar to NV12. It using two
>> bytes to store 12bit valid data and 4bit zero in LSB. This format
>> will be used for hardware decode/encode in VAAPI and QSV.
>>
> 
> P012 is not required, you can just use P016. All these P*  formats
> have the advantage of being aligned to MSB, which means the bitdepth
> does not matter for their layout.
> Instead you can use P016 and if you must know, look at a bitdepth
> value separately.

Tracking it separately does not seem fun - it looks to me like it would require adding a new bit depth field to AVFrame.

FFmpeg has always used pixfmt as defining both the memory layout and which bits are used in that (so, for example, ARGB and 0RGB are not the same thing), unlike most of the graphics APIs which tend to 
define those two separately.

- Mark
Carl Eugen Hoyos June 28, 2020, 9:06 p.m. UTC | #6 
Am So., 28. Juni 2020 um 23:01 Uhr schrieb Mark Thompson <sw@jkqxz.net>:

> FFmpeg has always used pixfmt as defining both the memory layout
> and which bits are used in that (so, for example, ARGB and 0RGB
> are not the same thing)

But they have the same bitdepth per component...

Carl Eugen
Xiang, Haihao June 30, 2020, 7:33 a.m. UTC | #7 
Agree with Mark. P012 and P016 have different significant bits, we should use
different pixfmts, otherwise an extra field in AVFrame is needed for bit depth. 

BTW there are the YUV420P variants for 10 / 12 / 14 / 16 bit in FFmpeg, it would
be better to follow FFmpeg's style to introduce P012 format instead of reusing
P016.

Thanks
Haihao


> Am So., 28. Juni 2020 um 23:01 Uhr schrieb Mark Thompson <sw@jkqxz.net>:
> 
> > FFmpeg has always used pixfmt as defining both the memory layout
> > and which bits are used in that (so, for example, ARGB and 0RGB
> > are not the same thing)
> 
> But they have the same bitdepth per component...
> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Xiang, Haihao Jan. 21, 2022, 5:56 a.m. UTC | #8 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Xiang,
> Haihao
> Sent: Tuesday, June 30, 2020 15:34
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v1 1/9] lavu/pix_fmt: add P012 pixel
> format
> 
> > Am So., 28. Juni 2020 um 23:01 Uhr schrieb Mark Thompson <sw@jkqxz.net>:
> >
> > > FFmpeg has always used pixfmt as defining both the memory layout
> > > and which bits are used in that (so, for example, ARGB and 0RGB
> > > are not the same thing)
> >
> > But they have the same bitdepth per component...
> >

> Agree with Mark. P012 and P016 have different significant bits, we should use
> different pixfmts, otherwise an extra field in AVFrame is needed for bit depth.
> 
> BTW there are the YUV420P variants for 10 / 12 / 14 / 16 bit in FFmpeg, it
> would
> be better to follow FFmpeg's style to introduce P012 format instead of reusing
> P016.

Sorry for picking up this old thread.

We'd like to add the support for 12bit decoding / encoding in VAAPI and QSV.  Is there any other concern if adding P012 in FFmpeg ? 

Thanks
Haihao
diff mbox series

Patch

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 8274713226..38297e2e83 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2147,6 +2147,30 @@  static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
         },
         .flags = AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_BE,
     },
+    [AV_PIX_FMT_P012LE] = {
+        .name = "p012le",
+        .nb_components = 3,
+        .log2_chroma_w = 1,
+        .log2_chroma_h = 1,
+        .comp = {
+            { 0, 2, 0, 4, 12, 1, 11, 1 },        /* Y */
+            { 1, 4, 0, 4, 12, 3, 11, 1 },        /* U */
+            { 1, 4, 2, 4, 12, 3, 11, 3 },        /* V */
+        },
+        .flags = AV_PIX_FMT_FLAG_PLANAR,
+    },
+    [AV_PIX_FMT_P012BE] = {
+        .name = "p012be",
+        .nb_components = 3,
+        .log2_chroma_w = 1,
+        .log2_chroma_h = 1,
+        .comp = {
+            { 0, 2, 0, 4, 12, 1, 11, 1 },        /* Y */
+            { 1, 4, 0, 4, 12, 3, 11, 1 },        /* U */
+            { 1, 4, 2, 4, 12, 3, 11, 3 },        /* V */
+        },
+        .flags = AV_PIX_FMT_FLAG_PLANAR | AV_PIX_FMT_FLAG_BE,
+    },
     [AV_PIX_FMT_P016LE] = {
         .name = "p016le",
         .nb_components = 3,
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index a46acf3c5e..261e1ea352 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -360,6 +360,10 @@  enum AVPixelFormat {
 
     AV_PIX_FMT_X2RGB10LE, ///< packed RGB 10:10:10, 30bpp, (msb)2X 10R 10G 10B(lsb), little-endian, X=unused/undefined
     AV_PIX_FMT_X2RGB10BE, ///< packed RGB 10:10:10, 30bpp, (msb)2X 10R 10G 10B(lsb), big-endian, X=unused/undefined
+
+    AV_PIX_FMT_P012LE, ///< like NV12, with 12bpp per component, little-endian
+    AV_PIX_FMT_P012BE, ///< like NV12, with 12bpp per component, 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
 };
 
@@ -450,6 +454,7 @@  enum AVPixelFormat {
 
 #define AV_PIX_FMT_Y210       AV_PIX_FMT_NE(Y210BE,  Y210LE)
 #define AV_PIX_FMT_X2RGB10    AV_PIX_FMT_NE(X2RGB10BE, X2RGB10LE)
+#define AV_PIX_FMT_P012       AV_PIX_FMT_NE(P012BE,  P012LE)
 
 /**
   * Chromaticity coordinates of the source primaries.
diff --git a/libavutil/version.h b/libavutil/version.h
index 3ce9b1831e..a63f79feb1 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  55
+#define LIBAVUTIL_VERSION_MINOR  56
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
diff --git a/tests/ref/fate/sws-pixdesc-query b/tests/ref/fate/sws-pixdesc-query
index c3cccfa492..f738a3ad74 100644
--- a/tests/ref/fate/sws-pixdesc-query
+++ b/tests/ref/fate/sws-pixdesc-query
@@ -57,6 +57,8 @@  isNBPS:
   nv20le
   p010be
   p010le
+  p012be
+  p012le
   x2rgb10be
   x2rgb10le
   xyz12be
@@ -137,6 +139,7 @@  isBE:
   grayf32be
   nv20be
   p010be
+  p012be
   p016be
   rgb444be
   rgb48be
@@ -188,6 +191,8 @@  isYUV:
   nv42
   p010be
   p010le
+  p012be
+  p012le
   p016be
   p016le
   uyvy422
@@ -282,6 +287,8 @@  isPlanarYUV:
   nv42
   p010be
   p010le
+  p012be
+  p012le
   p016be
   p016le
   yuv410p
@@ -365,6 +372,8 @@  isSemiPlanarYUV:
   nv42
   p010be
   p010le
+  p012be
+  p012le
   p016be
   p016le
 
@@ -740,6 +749,8 @@  Planar:
   nv42
   p010be
   p010le
+  p012be
+  p012le
   p016be
   p016le
   yuv410p