diff mbox series

[FFmpeg-devel,v2,3/3] avcodec/libx264: remove separate libx264rgb RGB wrapper

Message ID 20210702112513.36348-4-jeebjp@gmail.com
State New
Headers show
Series libx264 configure check clean-up, removal of libx264rgb
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Jan Ekström July 2, 2021, 11:25 a.m. UTC
No other encoder wrapper in libavcodec is split like this, and
with RGB input this currently does not lead to 4:2:0 (which would
be generally supported in most hardware and software implementations),
but rather 4:4:4.

The libx262 encoder definition was not touched, as it already has
4:4:4 YCbCr defined for it, which as far as I can tell is not supported.
---
 configure              |  2 --
 doc/encoders.texi      |  7 ++++---
 libavcodec/allcodecs.c |  1 -
 libavcodec/libx264.c   | 43 ++++++------------------------------------
 libavcodec/version.h   |  2 +-
 5 files changed, 11 insertions(+), 44 deletions(-)

Comments

Michael Niedermayer July 2, 2021, 3:25 p.m. UTC | #1
On Fri, Jul 02, 2021 at 02:25:13PM +0300, Jan Ekström wrote:
> No other encoder wrapper in libavcodec is split like this, and
> with RGB input this currently does not lead to 4:2:0 (which would
> be generally supported in most hardware and software implementations),
> but rather 4:4:4.
> 
> The libx262 encoder definition was not touched, as it already has
> 4:4:4 YCbCr defined for it, which as far as I can tell is not supported.
> ---
>  configure              |  2 --
>  doc/encoders.texi      |  7 ++++---
>  libavcodec/allcodecs.c |  1 -
>  libavcodec/libx264.c   | 43 ++++++------------------------------------
>  libavcodec/version.h   |  2 +-
>  5 files changed, 11 insertions(+), 44 deletions(-)

breaks:
./ffmpeg -i lena.pnm -vf format=bgr24 -vcodec libx264rgb  file-bgr.nut

also it breaks creating files which are playable on devices not supporting rgb
with a plain libx264 encoder selection

thx


[...]
Jan Ekström July 2, 2021, 3:37 p.m. UTC | #2
On Fri, Jul 2, 2021 at 6:25 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Fri, Jul 02, 2021 at 02:25:13PM +0300, Jan Ekström wrote:
> > No other encoder wrapper in libavcodec is split like this, and
> > with RGB input this currently does not lead to 4:2:0 (which would
> > be generally supported in most hardware and software implementations),
> > but rather 4:4:4.
> >
> > The libx262 encoder definition was not touched, as it already has
> > 4:4:4 YCbCr defined for it, which as far as I can tell is not supported.
> > ---
> >  configure              |  2 --
> >  doc/encoders.texi      |  7 ++++---
> >  libavcodec/allcodecs.c |  1 -
> >  libavcodec/libx264.c   | 43 ++++++------------------------------------
> >  libavcodec/version.h   |  2 +-
> >  5 files changed, 11 insertions(+), 44 deletions(-)
>
> breaks:
> ./ffmpeg -i lena.pnm -vf format=bgr24 -vcodec libx264rgb  file-bgr.nut
>

That is expected since the encoder is no longer there and standard
"libx264" moniker is to be utilized.

> also it breaks creating files which are playable on devices not supporting rgb
> with a plain libx264 encoder selection
>

So things that:
1. Support 4:4:4
2. But don't support reading the BGR metadata.

I would say that is a rather limited set of things. For most users I
expect they already had to specify the pixel format they required
since the vast majority require 4:2:0 if they want to support HW
decoding on most devices.

The ticket that was pointed towards when this wrapper was added
(https://trac.ffmpeg.org/ticket/658) specifically requested that 4:2:0
YCbCr should be the default for hw and sw implementations that are
more limited. The result currently is that for RGB input you get 4:4:4
YCbCr, and most likely this was the case back then as well (albeit I
can be incorrect in this). In any case, the usefulness of having these
two things split seems to be /very/ limited at this point.

Jan
diff mbox series

Patch

diff --git a/configure b/configure
index b3b8065188..9e8d219449 100755
--- a/configure
+++ b/configure
@@ -3316,8 +3316,6 @@  libwebp_anim_encoder_deps="libwebp"
 libx262_encoder_deps="libx262"
 libx264_encoder_deps="libx264"
 libx264_encoder_select="atsc_a53"
-libx264rgb_encoder_deps="libx264"
-libx264rgb_encoder_select="libx264_encoder"
 libx265_encoder_deps="libx265"
 libxavs_encoder_deps="libxavs"
 libxavs2_encoder_deps="libxavs2"
diff --git a/doc/encoders.texi b/doc/encoders.texi
index 4c38996372..e60ffe9c7d 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -2280,7 +2280,7 @@  Text-like
 
 @end table
 
-@section libx264, libx264rgb
+@section libx264
 
 x264 H.264/MPEG-4 AVC encoder wrapper.
 
@@ -2302,8 +2302,9 @@  by the libx264 @code{x264_param_parse} function.
 The x264 project website is at
 @url{http://www.videolan.org/developers/x264.html}.
 
-The libx264rgb encoder is the same as libx264, except it accepts packed RGB
-pixel formats as input instead of YUV.
+Since libavcodec 59.4.100 the libx264 encoder wrapper now supports both
+YCbCr as well as packed RGB pixel formats, and the separate libx264rgb
+wrapper has been removed.
 
 @subsection Supported Pixel Formats
 
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 623db2a9fa..d1a5dfdb75 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -772,7 +772,6 @@  extern const AVCodec ff_libx262_encoder;
 #endif
 extern LIBX264_CONST AVCodec ff_libx264_encoder;
 #endif
-extern const AVCodec ff_libx264rgb_encoder;
 extern AVCodec ff_libx265_encoder;
 extern const AVCodec ff_libxavs_encoder;
 extern const AVCodec ff_libxavs2_encoder;
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index fdb9e285a6..b31814bd4b 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -975,6 +975,9 @@  static const enum AVPixelFormat pix_fmts_8bit[] = {
     AV_PIX_FMT_YUVJ422P,
     AV_PIX_FMT_YUV444P,
     AV_PIX_FMT_YUVJ444P,
+    AV_PIX_FMT_BGR0,
+    AV_PIX_FMT_BGR24,
+    AV_PIX_FMT_RGB24,
     AV_PIX_FMT_NV12,
     AV_PIX_FMT_NV16,
 #ifdef X264_CSP_NV21
@@ -1001,6 +1004,9 @@  static const enum AVPixelFormat pix_fmts_all[] = {
     AV_PIX_FMT_YUVJ422P,
     AV_PIX_FMT_YUV444P,
     AV_PIX_FMT_YUVJ444P,
+    AV_PIX_FMT_BGR0,
+    AV_PIX_FMT_BGR24,
+    AV_PIX_FMT_RGB24,
     AV_PIX_FMT_NV12,
     AV_PIX_FMT_NV16,
 #ifdef X264_CSP_NV21
@@ -1017,13 +1023,6 @@  static const enum AVPixelFormat pix_fmts_all[] = {
     AV_PIX_FMT_NONE
 };
 
-static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
-    AV_PIX_FMT_BGR0,
-    AV_PIX_FMT_BGR24,
-    AV_PIX_FMT_RGB24,
-    AV_PIX_FMT_NONE
-};
-
 #if X264_BUILD < 153
 static av_cold void X264_init_static(AVCodec *codec)
 {
@@ -1183,36 +1182,6 @@  AVCodec ff_libx264_encoder = {
                       ,
     .wrapper_name     = "libx264",
 };
-
-static const AVClass rgbclass = {
-    .class_name = "libx264rgb",
-    .item_name  = av_default_item_name,
-    .option     = options,
-    .version    = LIBAVUTIL_VERSION_INT,
-};
-
-const AVCodec ff_libx264rgb_encoder = {
-    .name           = "libx264rgb",
-    .long_name      = NULL_IF_CONFIG_SMALL("libx264 H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10 RGB"),
-    .type           = AVMEDIA_TYPE_VIDEO,
-    .id             = AV_CODEC_ID_H264,
-    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
-                      AV_CODEC_CAP_OTHER_THREADS |
-                      AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
-    .priv_data_size = sizeof(X264Context),
-    .init           = X264_init,
-    .encode2        = X264_frame,
-    .close          = X264_close,
-    .priv_class     = &rgbclass,
-    .defaults       = x264_defaults,
-    .pix_fmts       = pix_fmts_8bit_rgb,
-    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP | FF_CODEC_CAP_AUTO_THREADS
-#if X264_BUILD >= 158
-                      | FF_CODEC_CAP_INIT_THREADSAFE
-#endif
-                      ,
-    .wrapper_name   = "libx264",
-};
 #endif
 
 #if CONFIG_LIBX262_ENCODER
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 2a420a7e28..554f293aad 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@ 
 #include "libavutil/version.h"
 
 #define LIBAVCODEC_VERSION_MAJOR  59
-#define LIBAVCODEC_VERSION_MINOR   3
+#define LIBAVCODEC_VERSION_MINOR   4
 #define LIBAVCODEC_VERSION_MICRO 100
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \