| Message ID | 20190202150521.6374-1-silvo@gmx.net |
|---|---|
| State | New |
| Headers | show |
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
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
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
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.
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.
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,
From: Thomas Volkert <thomas.volkert@net-zeal.com> --- libavcodec/libx264.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)