[FFmpeg-devel] avcodec/libx264: support BGRA pixel format

Submitted by Thomas Volkert on Feb. 2, 2019, 3:05 p.m.

Details

Message ID 20190202150521.6374-1-silvo@gmx.net
State New
Headers show

Commit Message

Thomas Volkert Feb. 2, 2019, 3:05 p.m.
From: Thomas Volkert <thomas.volkert@net-zeal.com>

---
 libavcodec/libx264.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Carl Eugen Hoyos Feb. 2, 2019, 3:14 p.m.
2019-02-02 16:05 GMT+01:00, Thomas Volkert <silvo@gmx.net>:
> From: Thomas Volkert <thomas.volkert@net-zeal.com>
>
> ---
>  libavcodec/libx264.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index a3493f393d..dd51fdc6dc 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -158,6 +158,9 @@ static int encode_nals(AVCodecContext *ctx, AVPacket
> *pkt,
>  static int avfmt2_num_planes(int avfmt)
>  {
>      switch (avfmt) {
> +    case AV_PIX_FMT_BGRA:

I was / am completely convinced that libx264 does not
support AV_PIX_FMT_BGRA.
Why do you believe it does / when was support added?

Carl Eugen
Jan Ekström Feb. 2, 2019, 3:18 p.m.
On Sat, Feb 2, 2019 at 5:14 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> 2019-02-02 16:05 GMT+01:00, Thomas Volkert <silvo@gmx.net>:
> > From: Thomas Volkert <thomas.volkert@net-zeal.com>
> >
> > ---
> >  libavcodec/libx264.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > index a3493f393d..dd51fdc6dc 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -158,6 +158,9 @@ static int encode_nals(AVCodecContext *ctx, AVPacket
> > *pkt,
> >  static int avfmt2_num_planes(int avfmt)
> >  {
> >      switch (avfmt) {
> > +    case AV_PIX_FMT_BGRA:
>
> I was / am completely convinced that libx264 does not
> support AV_PIX_FMT_BGRA.
> Why do you believe it does / when was support added?

It seems to support BGRA planes, of which it will only utilize BGR (as
there is no fourth plane in H.264 as far as I can tell).

Might want to add a warning that the alpha will be completely ignored
if this pix_fmt is added as supported.

Jan
Carl Eugen Hoyos Feb. 2, 2019, 3:29 p.m.
2019-02-02 16:18 GMT+01:00, Jan Ekström <jeebjp@gmail.com>:
> On Sat, Feb 2, 2019 at 5:14 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>
>> 2019-02-02 16:05 GMT+01:00, Thomas Volkert <silvo@gmx.net>:
>> > From: Thomas Volkert <thomas.volkert@net-zeal.com>
>> >
>> > ---
>> >  libavcodec/libx264.c | 15 ++++++++-------
>> >  1 file changed, 8 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
>> > index a3493f393d..dd51fdc6dc 100644
>> > --- a/libavcodec/libx264.c
>> > +++ b/libavcodec/libx264.c
>> > @@ -158,6 +158,9 @@ static int encode_nals(AVCodecContext *ctx, AVPacket
>> > *pkt,
>> >  static int avfmt2_num_planes(int avfmt)
>> >  {
>> >      switch (avfmt) {
>> > +    case AV_PIX_FMT_BGRA:
>>
>> I was / am completely convinced that libx264 does not
>> support AV_PIX_FMT_BGRA.
>> Why do you believe it does / when was support added?
>
> It seems to support BGRA planes, of which it will only utilize
> BGR (as there is no fourth plane in H.264 as far as I can tell).

So - as expected - it supports BGR0 but not BGRA.
(It calls BGR0 BGRA.)

Carl Eugen
Thomas Volkert Feb. 2, 2019, 3:39 p.m.
On 02.02.2019 16:14, Carl Eugen Hoyos wrote:
> 2019-02-02 16:05 GMT+01:00, Thomas Volkert <silvo@gmx.net>:
>> From: Thomas Volkert <thomas.volkert@net-zeal.com>
>>
>> ---
>>  libavcodec/libx264.c | 15 ++++++++-------
>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
>> index a3493f393d..dd51fdc6dc 100644
>> --- a/libavcodec/libx264.c
>> +++ b/libavcodec/libx264.c
>> @@ -158,6 +158,9 @@ static int encode_nals(AVCodecContext *ctx, AVPacket
>> *pkt,
>>  static int avfmt2_num_planes(int avfmt)
>>  {
>>      switch (avfmt) {
>> +    case AV_PIX_FMT_BGRA:
> I was / am completely convinced that libx264 does not
> support AV_PIX_FMT_BGRA.
> Why do you believe it does / when was support added?

I can see the special handling of X264_CSP_BGRA in the code of libx264.
So, I wanted to get feedback from our list here.

But I have seen another mistake, the number of planes is wrong. It
should be 1 instead of 4.

Best regards,
Thomas.
Thomas Volkert Feb. 2, 2019, 4:22 p.m.
On 02.02.2019 16:18, Jan Ekström wrote:
> On Sat, Feb 2, 2019 at 5:14 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2019-02-02 16:05 GMT+01:00, Thomas Volkert <silvo@gmx.net>:
>>> From: Thomas Volkert <thomas.volkert@net-zeal.com>
>>>
>>> ---
>>>  libavcodec/libx264.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
>>> index a3493f393d..dd51fdc6dc 100644
>>> --- a/libavcodec/libx264.c
>>> +++ b/libavcodec/libx264.c
>>> @@ -158,6 +158,9 @@ static int encode_nals(AVCodecContext *ctx, AVPacket
>>> *pkt,
>>>  static int avfmt2_num_planes(int avfmt)
>>>  {
>>>      switch (avfmt) {
>>> +    case AV_PIX_FMT_BGRA:
>> I was / am completely convinced that libx264 does not
>> support AV_PIX_FMT_BGRA.
>> Why do you believe it does / when was support added?
> It seems to support BGRA planes, of which it will only utilize BGR (as
> there is no fourth plane in H.264 as far as I can tell).

I was digging in the code of libx264 now. I think you are right - only 3
planes are used in case of AV_PIX_FMT_BGRA.


> Might want to add a warning that the alpha will be completely ignored
> if this pix_fmt is added as supported.

I changed my opinion - I would rather drop this patch.
Thanks for the feedback.

Best regards,
Thomas.

Patch hide | download patch | download mbox

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index a3493f393d..dd51fdc6dc 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -158,6 +158,9 @@  static int encode_nals(AVCodecContext *ctx, AVPacket *pkt,
 static int avfmt2_num_planes(int avfmt)
 {
     switch (avfmt) {
+    case AV_PIX_FMT_BGRA:
+    	return 4;
+
     case AV_PIX_FMT_YUV420P:
     case AV_PIX_FMT_YUVJ420P:
     case AV_PIX_FMT_YUV420P9:
@@ -511,13 +514,10 @@  static int convert_pix_fmt(enum AVPixelFormat pix_fmt)
     case AV_PIX_FMT_YUV444P9:
     case AV_PIX_FMT_YUV444P10: return X264_CSP_I444;
 #if CONFIG_LIBX264RGB_ENCODER
-    case AV_PIX_FMT_BGR0:
-        return X264_CSP_BGRA;
-    case AV_PIX_FMT_BGR24:
-        return X264_CSP_BGR;
-
-    case AV_PIX_FMT_RGB24:
-        return X264_CSP_RGB;
+    case AV_PIX_FMT_BGRA:      return X264_CSP_BGRA;
+    case AV_PIX_FMT_BGR0:      return X264_CSP_BGRA;
+    case AV_PIX_FMT_BGR24:     return X264_CSP_BGR;
+    case AV_PIX_FMT_RGB24:     return X264_CSP_RGB;
 #endif
     case AV_PIX_FMT_NV12:      return X264_CSP_NV12;
     case AV_PIX_FMT_NV16:
@@ -987,6 +987,7 @@  static const enum AVPixelFormat pix_fmts_all[] = {
 };
 #if CONFIG_LIBX264RGB_ENCODER
 static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
+    AV_PIX_FMT_BGRA,
     AV_PIX_FMT_BGR0,
     AV_PIX_FMT_BGR24,
     AV_PIX_FMT_RGB24,