diff mbox

[FFmpeg-devel,2/8] swscale: Add swscale input support for Y210

Message ID 1577636941-12597-1-git-send-email-linjie.fu@intel.com
State New
Headers show

Commit Message

Fu, Linjie Dec. 29, 2019, 4:29 p.m. UTC
Add swscale input support for Y210, output support and fate
test could be added later if there is requirement for software
CSC to this packed format.

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libswscale/input.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 libswscale/utils.c |  2 ++
 2 files changed, 50 insertions(+)

Comments

Carl Eugen Hoyos Jan. 12, 2020, 4:40 p.m. UTC | #1
Am So., 29. Dez. 2019 um 17:40 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:
>
> Add swscale input support for Y210, output support and fate
> test could be added later if there is requirement for software
> CSC to this packed format.
>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libswscale/input.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  libswscale/utils.c |  2 ++
>  2 files changed, 50 insertions(+)
>
> diff --git a/libswscale/input.c b/libswscale/input.c
> index 064f8da..f79b983 100644
> --- a/libswscale/input.c
> +++ b/libswscale/input.c
> @@ -552,6 +552,42 @@ static void yvy2ToUV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, con
>      av_assert1(src1 == src2);
>  }
>
> +static void y210le_UV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, const uint8_t *src,
> +                        const uint8_t *unused1, int width, uint32_t *unused2)
> +{
> +    int i;
> +    for (i = 0; i < width; i++) {
> +        AV_WN16(dstU + i * 2, AV_RL16(src + i * 8 + 2) >> 6);
> +        AV_WN16(dstV + i * 2, AV_RL16(src + i * 8 + 6) >> 6);
> +    }
> +}
> +
> +static void y210be_UV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, const uint8_t *src,
> +                        const uint8_t *unused1, int width, uint32_t *unused2)
> +{
> +    int i;
> +    for (i = 0; i < width; i++) {
> +        AV_WN16(dstU + i * 2, AV_RB16(src + i * 8 + 2) >> 6);
> +        AV_WN16(dstV + i * 2, AV_RB16(src + i * 8 + 6) >> 6);
> +    }
> +}
> +
> +static void y210le_Y_c(uint8_t *dst, const uint8_t *src, const uint8_t *unused0,
> +                       const uint8_t *unused1, int width, uint32_t *unused2)
> +{
> +    int i;
> +    for (i = 0; i < width; i++)
> +        AV_WN16(dst + i * 2, AV_RL16(src + i * 4) >> 6);
> +}
> +
> +static void y210be_Y_c(uint8_t *dst, const uint8_t *src, const uint8_t *unused0,
> +                       const uint8_t *unused1, int width, uint32_t *unused2)
> +{
> +    int i;
> +    for (i = 0; i < width; i++)
> +        AV_WN16(dst + i * 2 ,AV_RB16(src + i * 4) >> 6);
> +}
> +
>  static void bswap16Y_c(uint8_t *_dst, const uint8_t *_src, const uint8_t *unused1, const uint8_t *unused2, int width,
>                         uint32_t *unused)
>  {
> @@ -1154,6 +1190,12 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
>      case AV_PIX_FMT_P016BE:
>          c->chrToYV12 = p016BEToUV_c;
>          break;
> +    case AV_PIX_FMT_Y210LE:
> +        c->chrToYV12 = y210le_UV_c;
> +        break;
> +    case AV_PIX_FMT_Y210BE:
> +        c->chrToYV12 = y210be_UV_c;
> +        break;
>      }
>      if (c->chrSrcHSubSample) {
>          switch (srcFormat) {
> @@ -1586,6 +1628,12 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
>          c->lumToYV12 = grayf32ToY16_bswap_c;
>  #endif
>          break;
> +    case AV_PIX_FMT_Y210LE:
> +        c->lumToYV12 = y210le_Y_c;
> +        break;
> +    case AV_PIX_FMT_Y210BE:
> +        c->chrToYV12 = y210be_Y_c;
> +        break;
>      }
>      if (c->needAlpha) {
>          if (is16BPS(srcFormat) || isNBPS(srcFormat)) {
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index 57c4fd2..bff498f 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -266,6 +266,8 @@ static const FormatEntry format_entries[AV_PIX_FMT_NB] = {
>      [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
>      [AV_PIX_FMT_NV24]        = { 1, 1 },
>      [AV_PIX_FMT_NV42]        = { 1, 1 },
> +    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
> +    [AV_PIX_FMT_Y210LE]      = { 1, 0 },

Am I correct that this functions only work on LE because the vaapi drivers
only exist for LE?
Or do I misunderstand the code?

Carl Eugen
Fu, Linjie Jan. 14, 2020, 3:20 a.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Carl Eugen Hoyos
> Sent: Monday, January 13, 2020 00:40
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input support
> for Y210
> 
> Am So., 29. Dez. 2019 um 17:40 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:
> >
> > Add swscale input support for Y210, output support and fate
> > test could be added later if there is requirement for software
> > CSC to this packed format.
> >
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> format_entries[AV_PIX_FMT_NB] = {
> >      [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
> >      [AV_PIX_FMT_NV24]        = { 1, 1 },
> >      [AV_PIX_FMT_NV42]        = { 1, 1 },
> > +    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
> > +    [AV_PIX_FMT_Y210LE]      = { 1, 0 },
> 
> Am I correct that this functions only work on LE because the vaapi drivers
> only exist for LE?

The only output from VAAPI driver is Y210LE. Y210BE is not available for
the hardware.(swscale input support for BE is identical to LE, and should also work)

The Y210BE definition and software scale support could be hold currently and maybe
add later if there is a requirement.

Will update a new version.
Thanks.
Carl Eugen Hoyos Jan. 14, 2020, 4:41 a.m. UTC | #3
> Am 14.01.2020 um 04:20 schrieb Fu, Linjie <linjie.fu@intel.com>:
> 
> Zitierten Inhalt anzeigen
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Carl Eugen Hoyos
>> Sent: Monday, January 13, 2020 00:40
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input support
>> for Y210
>> 
>>> Am So., 29. Dez. 2019 um 17:40 Uhr schrieb Linjie Fu <linjie.fu@intel.com>:
>>> 
>>> Add swscale input support for Y210, output support and fate
>>> test could be added later if there is requirement for software
>>> CSC to this packed format.
>>> 
>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
>>> ---
>> format_entries[AV_PIX_FMT_NB] = {
>>>     [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
>>>     [AV_PIX_FMT_NV24]        = { 1, 1 },
>>>     [AV_PIX_FMT_NV42]        = { 1, 1 },
>>> +    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
>>> +    [AV_PIX_FMT_Y210LE]      = { 1, 0 },
>> 
>> Am I correct that this functions only work on LE because the vaapi drivers
>> only exist for LE?
> 
> 
> The only output from VAAPI driver is Y210LE.

But does the driver also exist on big-endian hardware? And was your conversion routine tested on big-endian hardware?

Carl Eugen
Fu, Linjie Jan. 14, 2020, 7:25 a.m. UTC | #4
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Carl Eugen Hoyos
> Sent: Tuesday, January 14, 2020 12:42
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input support
> for Y210
> 
> 
> 
> > Am 14.01.2020 um 04:20 schrieb Fu, Linjie <linjie.fu@intel.com>:
> >
> > Zitierten Inhalt anzeigen
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Carl Eugen Hoyos
> >> Sent: Monday, January 13, 2020 00:40
> >> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input
> support
> >> for Y210
> >>
> >>> Am So., 29. Dez. 2019 um 17:40 Uhr schrieb Linjie Fu
> <linjie.fu@intel.com>:
> >>>
> >>> Add swscale input support for Y210, output support and fate
> >>> test could be added later if there is requirement for software
> >>> CSC to this packed format.
> >>>
> >>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> >>> ---
> >> format_entries[AV_PIX_FMT_NB] = {
> >>>     [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
> >>>     [AV_PIX_FMT_NV24]        = { 1, 1 },
> >>>     [AV_PIX_FMT_NV42]        = { 1, 1 },
> >>> +    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
> >>> +    [AV_PIX_FMT_Y210LE]      = { 1, 0 },
> >>
> >> Am I correct that this functions only work on LE because the vaapi drivers
> >> only exist for LE?
> >
> >
> > The only output from VAAPI driver is Y210LE.
> 
> But does the driver also exist on big-endian hardware? And was your
> conversion routine tested on big-endian hardware?

As far as I know from media-driver, there is no support on big-endian hardware,
hence no testing for big endian conversion locally. To avoid any uncertainty, we
can hold the big-endian support unless it's demanded someday.
Carl Eugen Hoyos Jan. 14, 2020, 3:55 p.m. UTC | #5
Am Di., 14. Jan. 2020 um 08:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>:
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Carl Eugen Hoyos
> > Sent: Tuesday, January 14, 2020 12:42
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input support
> > for Y210
> >
> >
> >
> > > Am 14.01.2020 um 04:20 schrieb Fu, Linjie <linjie.fu@intel.com>:
> > >
> > > Zitierten Inhalt anzeigen
> > >> -----Original Message-----
> > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > >> Carl Eugen Hoyos
> > >> Sent: Monday, January 13, 2020 00:40
> > >> To: FFmpeg development discussions and patches <ffmpeg-
> > >> devel@ffmpeg.org>
> > >> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input
> > support
> > >> for Y210
> > >>
> > >>> Am So., 29. Dez. 2019 um 17:40 Uhr schrieb Linjie Fu
> > <linjie.fu@intel.com>:
> > >>>
> > >>> Add swscale input support for Y210, output support and fate
> > >>> test could be added later if there is requirement for software
> > >>> CSC to this packed format.
> > >>>
> > >>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > >>> ---
> > >> format_entries[AV_PIX_FMT_NB] = {
> > >>>     [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
> > >>>     [AV_PIX_FMT_NV24]        = { 1, 1 },
> > >>>     [AV_PIX_FMT_NV42]        = { 1, 1 },
> > >>> +    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
> > >>> +    [AV_PIX_FMT_Y210LE]      = { 1, 0 },
> > >>
> > >> Am I correct that this functions only work on LE because the vaapi drivers
> > >> only exist for LE?
> > >
> > >
> > > The only output from VAAPI driver is Y210LE.
> >
> > But does the driver also exist on big-endian hardware? And was your
> > conversion routine tested on big-endian hardware?
>
> As far as I know from media-driver, there is no support on big-endian hardware,
> hence no testing for big endian conversion locally. To avoid any uncertainty, we
> can hold the big-endian support unless it's demanded someday.

Sorry for being unclear:
This patch is about libswscale and (by itself) unrelated to vaapi.
The patch must work on all platforms for which libswscale can be
compiled, that includes big-endian hardware. It looks to me as if
the patch will not work correctly (and in this case hopefully break
fate) on big-endian. If I am correct, the patch can not be committed.
(I did not test myself and could of course be wrong.)

I have no strong opinion if it makes sense to add code for
Y210BE or not, it may even be a possibility to only add
Y210 (native) which will likely simplify the patch, but I
fear fate will not like it.

Carl Eugen

PS:
As always, this would of course be much easier if no new
pix_fmt would be needed, apart from other developers'
opinions I still wonder if it wouldn't be much simpler for you
to do the conversion in your wrapper code...
Fu, Linjie Jan. 15, 2020, 5:15 a.m. UTC | #6
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Carl Eugen Hoyos
> Sent: Tuesday, January 14, 2020 23:55
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input support
> for Y210
> 
> Am Di., 14. Jan. 2020 um 08:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>:
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Carl Eugen Hoyos
> > > Sent: Tuesday, January 14, 2020 12:42
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input
> support
> > > for Y210
> > >
> > >
> > >
> > > > Am 14.01.2020 um 04:20 schrieb Fu, Linjie <linjie.fu@intel.com>:
> > > >
> > > > Zitierten Inhalt anzeigen
> > > >> -----Original Message-----
> > > >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> Of
> > > >> Carl Eugen Hoyos
> > > >> Sent: Monday, January 13, 2020 00:40
> > > >> To: FFmpeg development discussions and patches <ffmpeg-
> > > >> devel@ffmpeg.org>
> > > >> Subject: Re: [FFmpeg-devel] [PATCH 2/8] swscale: Add swscale input
> > > support
> > > >> for Y210
> > > >>
> > > >>> Am So., 29. Dez. 2019 um 17:40 Uhr schrieb Linjie Fu
> > > <linjie.fu@intel.com>:
> > > >>>
> > > >>> Add swscale input support for Y210, output support and fate
> > > >>> test could be added later if there is requirement for software
> > > >>> CSC to this packed format.
> > > >>>
> > > >>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > > >>> ---
> > > >> format_entries[AV_PIX_FMT_NB] = {
> > > >>>     [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
> > > >>>     [AV_PIX_FMT_NV24]        = { 1, 1 },
> > > >>>     [AV_PIX_FMT_NV42]        = { 1, 1 },
> > > >>> +    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
> > > >>> +    [AV_PIX_FMT_Y210LE]      = { 1, 0 },
> > > >>
> > > >> Am I correct that this functions only work on LE because the vaapi
> drivers
> > > >> only exist for LE?
> > > >
> > > >
> > > > The only output from VAAPI driver is Y210LE.
> > >
> > > But does the driver also exist on big-endian hardware? And was your
> > > conversion routine tested on big-endian hardware?
> >
> > As far as I know from media-driver, there is no support on big-endian
> hardware,
> > hence no testing for big endian conversion locally. To avoid any uncertainty,
> we
> > can hold the big-endian support unless it's demanded someday.
> 
> Sorry for being unclear:
> This patch is about libswscale and (by itself) unrelated to vaapi.
> The patch must work on all platforms for which libswscale can be
> compiled, that includes big-endian hardware. It looks to me as if
> the patch will not work correctly (and in this case hopefully break
> fate) on big-endian. If I am correct, the patch can not be committed.
> (I did not test myself and could of course be wrong.)

> I have no strong opinion if it makes sense to add code for
> Y210BE or not, it may even be a possibility to only add
> Y210 (native) which will likely simplify the patch, but I
> fear fate will not like it.

Thanks, I agree this should be verified on big-endian hardware and
should not break fate.
While seeing the behavior of AYUV64LE in 052f64ecb2, one better solution came
to mind:
Add pixel format for both Y210BE and Y210LE,  and only add swscale input support
for Y210LE.
This is what currently AYUV64LE does, and is more friendly to FATE. (Also without
potential issues if Y210BE is going to be added in pixfmt.h in the future)
diff mbox

Patch

diff --git a/libswscale/input.c b/libswscale/input.c
index 064f8da..f79b983 100644
--- a/libswscale/input.c
+++ b/libswscale/input.c
@@ -552,6 +552,42 @@  static void yvy2ToUV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, con
     av_assert1(src1 == src2);
 }
 
+static void y210le_UV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, const uint8_t *src,
+                        const uint8_t *unused1, int width, uint32_t *unused2)
+{
+    int i;
+    for (i = 0; i < width; i++) {
+        AV_WN16(dstU + i * 2, AV_RL16(src + i * 8 + 2) >> 6);
+        AV_WN16(dstV + i * 2, AV_RL16(src + i * 8 + 6) >> 6);
+    }
+}
+
+static void y210be_UV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, const uint8_t *src,
+                        const uint8_t *unused1, int width, uint32_t *unused2)
+{
+    int i;
+    for (i = 0; i < width; i++) {
+        AV_WN16(dstU + i * 2, AV_RB16(src + i * 8 + 2) >> 6);
+        AV_WN16(dstV + i * 2, AV_RB16(src + i * 8 + 6) >> 6);
+    }
+}
+
+static void y210le_Y_c(uint8_t *dst, const uint8_t *src, const uint8_t *unused0,
+                       const uint8_t *unused1, int width, uint32_t *unused2)
+{
+    int i;
+    for (i = 0; i < width; i++)
+        AV_WN16(dst + i * 2, AV_RL16(src + i * 4) >> 6);
+}
+
+static void y210be_Y_c(uint8_t *dst, const uint8_t *src, const uint8_t *unused0,
+                       const uint8_t *unused1, int width, uint32_t *unused2)
+{
+    int i;
+    for (i = 0; i < width; i++)
+        AV_WN16(dst + i * 2 ,AV_RB16(src + i * 4) >> 6);
+}
+
 static void bswap16Y_c(uint8_t *_dst, const uint8_t *_src, const uint8_t *unused1, const uint8_t *unused2, int width,
                        uint32_t *unused)
 {
@@ -1154,6 +1190,12 @@  av_cold void ff_sws_init_input_funcs(SwsContext *c)
     case AV_PIX_FMT_P016BE:
         c->chrToYV12 = p016BEToUV_c;
         break;
+    case AV_PIX_FMT_Y210LE:
+        c->chrToYV12 = y210le_UV_c;
+        break;
+    case AV_PIX_FMT_Y210BE:
+        c->chrToYV12 = y210be_UV_c;
+        break;
     }
     if (c->chrSrcHSubSample) {
         switch (srcFormat) {
@@ -1586,6 +1628,12 @@  av_cold void ff_sws_init_input_funcs(SwsContext *c)
         c->lumToYV12 = grayf32ToY16_bswap_c;
 #endif
         break;
+    case AV_PIX_FMT_Y210LE:
+        c->lumToYV12 = y210le_Y_c;
+        break;
+    case AV_PIX_FMT_Y210BE:
+        c->chrToYV12 = y210be_Y_c;
+        break;
     }
     if (c->needAlpha) {
         if (is16BPS(srcFormat) || isNBPS(srcFormat)) {
diff --git a/libswscale/utils.c b/libswscale/utils.c
index 57c4fd2..bff498f 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -266,6 +266,8 @@  static const FormatEntry format_entries[AV_PIX_FMT_NB] = {
     [AV_PIX_FMT_YUVA444P12LE] = { 1, 1 },
     [AV_PIX_FMT_NV24]        = { 1, 1 },
     [AV_PIX_FMT_NV42]        = { 1, 1 },
+    [AV_PIX_FMT_Y210BE]      = { 1, 0 },
+    [AV_PIX_FMT_Y210LE]      = { 1, 0 },
 };
 
 int sws_isSupportedInput(enum AVPixelFormat pix_fmt)