diff mbox

[FFmpeg-devel,2/2] avcodec/libx264: fix compilation with x264 builds >= 153

Message ID 20171225225257.10044-2-jamrial@gmail.com
State Superseded
Headers show

Commit Message

James Almer Dec. 25, 2017, 10:52 p.m. UTC 
x264 now supports multibitdepth builds, with a slightly changed API to
request bitdepth during initialization.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/libx264.c | 45 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 8 deletions(-)

Comments

wm4 Dec. 25, 2017, 11:53 p.m. UTC | #1 
On Mon, 25 Dec 2017 19:52:57 -0300
James Almer <jamrial@gmail.com> wrote:

> x264 now supports multibitdepth builds, with a slightly changed API to
> request bitdepth during initialization.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/libx264.c | 45 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 2d36c5e566..29f60fa7b5 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -454,6 +454,7 @@ static av_cold int X264_init(AVCodecContext *avctx)
>  {
>      X264Context *x4 = avctx->priv_data;
>      AVCPBProperties *cpb_props;
> +    const av_unused AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
>      int sw,sh;
>  
>      if (avctx->global_quality > 0)
> @@ -724,6 +725,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  
>      x4->params.i_width          = avctx->width;
>      x4->params.i_height         = avctx->height;
> +#if X264_BUILD >= 153
> +    x4->params.i_bitdepth       = desc->comp[0].depth;
> +#endif
>      av_reduce(&sw, &sh, avctx->sample_aspect_ratio.num, avctx->sample_aspect_ratio.den, 4096);
>      x4->params.vui.i_sar_width  = sw;
>      x4->params.vui.i_sar_height = sh;
> @@ -851,6 +855,34 @@ static const enum AVPixelFormat pix_fmts_8bit[] = {
>  #endif
>      AV_PIX_FMT_NONE
>  };
> +
> +#if CONFIG_LIBX264RGB_ENCODER
> +static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
> +    AV_PIX_FMT_BGR0,
> +    AV_PIX_FMT_BGR24,
> +    AV_PIX_FMT_RGB24,
> +    AV_PIX_FMT_NONE
> +};
> +#endif
> +
> +#if X264_BUILD >= 153
> +static const enum AVPixelFormat pix_fmts[] = {
> +    AV_PIX_FMT_YUV420P,
> +    AV_PIX_FMT_YUVJ420P,
> +    AV_PIX_FMT_YUV422P,
> +    AV_PIX_FMT_YUVJ422P,
> +    AV_PIX_FMT_YUV444P,
> +    AV_PIX_FMT_YUVJ444P,
> +    AV_PIX_FMT_YUV420P10,
> +    AV_PIX_FMT_YUV422P10,
> +    AV_PIX_FMT_YUV444P10,
> +    AV_PIX_FMT_NV12,
> +    AV_PIX_FMT_NV16,
> +    AV_PIX_FMT_NV20,
> +    AV_PIX_FMT_NV21,
> +    AV_PIX_FMT_NONE
> +};
> +#else
>  static const enum AVPixelFormat pix_fmts_9bit[] = {
>      AV_PIX_FMT_YUV420P9,
>      AV_PIX_FMT_YUV444P9,
> @@ -863,14 +895,6 @@ static const enum AVPixelFormat pix_fmts_10bit[] = {
>      AV_PIX_FMT_NV20,
>      AV_PIX_FMT_NONE
>  };
> -#if CONFIG_LIBX264RGB_ENCODER
> -static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
> -    AV_PIX_FMT_BGR0,
> -    AV_PIX_FMT_BGR24,
> -    AV_PIX_FMT_RGB24,
> -    AV_PIX_FMT_NONE
> -};
> -#endif
>  
>  static av_cold void X264_init_static(AVCodec *codec)
>  {
> @@ -881,6 +905,7 @@ static av_cold void X264_init_static(AVCodec *codec)
>      else if (x264_bit_depth == 10)
>          codec->pix_fmts = pix_fmts_10bit;
>  }
> +#endif
>  
>  #define OFFSET(x) offsetof(X264Context, x)
>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> @@ -1024,7 +1049,11 @@ AVCodec ff_libx264_encoder = {
>      .capabilities     = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS,
>      .priv_class       = &x264_class,
>      .defaults         = x264_defaults,
> +#if X264_BUILD >= 153
> +    .pix_fmts         = pix_fmts,
> +#else
>      .init_static_data = X264_init_static,
> +#endif
>      .caps_internal    = FF_CODEC_CAP_INIT_THREADSAFE |
>                          FF_CODEC_CAP_INIT_CLEANUP,
>      .wrapper_name     = "libx264",

Do newer builds really support all pixfmts? I know we have this
init_static_data because it depended on runtime (and apparently x264
build settings). I assume new x264 just does away with setting this at
compile time, and always has code for all formats?

(Personally I welcome getting rid of init_static_data, since it'll be
the only reason why AVCodec still can't be in read-only memory. Though
libvpx has a similar problem too.)
James Almer Dec. 26, 2017, 12:01 a.m. UTC | #2 
On 12/25/2017 8:53 PM, wm4 wrote:
> On Mon, 25 Dec 2017 19:52:57 -0300
> James Almer <jamrial@gmail.com> wrote:
> 
>> x264 now supports multibitdepth builds, with a slightly changed API to
>> request bitdepth during initialization.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/libx264.c | 45 +++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 37 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
>> index 2d36c5e566..29f60fa7b5 100644
>> --- a/libavcodec/libx264.c
>> +++ b/libavcodec/libx264.c
>> @@ -454,6 +454,7 @@ static av_cold int X264_init(AVCodecContext *avctx)
>>  {
>>      X264Context *x4 = avctx->priv_data;
>>      AVCPBProperties *cpb_props;
>> +    const av_unused AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
>>      int sw,sh;
>>  
>>      if (avctx->global_quality > 0)
>> @@ -724,6 +725,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>  
>>      x4->params.i_width          = avctx->width;
>>      x4->params.i_height         = avctx->height;
>> +#if X264_BUILD >= 153
>> +    x4->params.i_bitdepth       = desc->comp[0].depth;
>> +#endif
>>      av_reduce(&sw, &sh, avctx->sample_aspect_ratio.num, avctx->sample_aspect_ratio.den, 4096);
>>      x4->params.vui.i_sar_width  = sw;
>>      x4->params.vui.i_sar_height = sh;
>> @@ -851,6 +855,34 @@ static const enum AVPixelFormat pix_fmts_8bit[] = {
>>  #endif
>>      AV_PIX_FMT_NONE
>>  };
>> +
>> +#if CONFIG_LIBX264RGB_ENCODER
>> +static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
>> +    AV_PIX_FMT_BGR0,
>> +    AV_PIX_FMT_BGR24,
>> +    AV_PIX_FMT_RGB24,
>> +    AV_PIX_FMT_NONE
>> +};
>> +#endif
>> +
>> +#if X264_BUILD >= 153
>> +static const enum AVPixelFormat pix_fmts[] = {
>> +    AV_PIX_FMT_YUV420P,
>> +    AV_PIX_FMT_YUVJ420P,
>> +    AV_PIX_FMT_YUV422P,
>> +    AV_PIX_FMT_YUVJ422P,
>> +    AV_PIX_FMT_YUV444P,
>> +    AV_PIX_FMT_YUVJ444P,
>> +    AV_PIX_FMT_YUV420P10,
>> +    AV_PIX_FMT_YUV422P10,
>> +    AV_PIX_FMT_YUV444P10,
>> +    AV_PIX_FMT_NV12,
>> +    AV_PIX_FMT_NV16,
>> +    AV_PIX_FMT_NV20,
>> +    AV_PIX_FMT_NV21,
>> +    AV_PIX_FMT_NONE
>> +};
>> +#else
>>  static const enum AVPixelFormat pix_fmts_9bit[] = {
>>      AV_PIX_FMT_YUV420P9,
>>      AV_PIX_FMT_YUV444P9,
>> @@ -863,14 +895,6 @@ static const enum AVPixelFormat pix_fmts_10bit[] = {
>>      AV_PIX_FMT_NV20,
>>      AV_PIX_FMT_NONE
>>  };
>> -#if CONFIG_LIBX264RGB_ENCODER
>> -static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
>> -    AV_PIX_FMT_BGR0,
>> -    AV_PIX_FMT_BGR24,
>> -    AV_PIX_FMT_RGB24,
>> -    AV_PIX_FMT_NONE
>> -};
>> -#endif
>>  
>>  static av_cold void X264_init_static(AVCodec *codec)
>>  {
>> @@ -881,6 +905,7 @@ static av_cold void X264_init_static(AVCodec *codec)
>>      else if (x264_bit_depth == 10)
>>          codec->pix_fmts = pix_fmts_10bit;
>>  }
>> +#endif
>>  
>>  #define OFFSET(x) offsetof(X264Context, x)
>>  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>> @@ -1024,7 +1049,11 @@ AVCodec ff_libx264_encoder = {
>>      .capabilities     = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS,
>>      .priv_class       = &x264_class,
>>      .defaults         = x264_defaults,
>> +#if X264_BUILD >= 153
>> +    .pix_fmts         = pix_fmts,
>> +#else
>>      .init_static_data = X264_init_static,
>> +#endif
>>      .caps_internal    = FF_CODEC_CAP_INIT_THREADSAFE |
>>                          FF_CODEC_CAP_INIT_CLEANUP,
>>      .wrapper_name     = "libx264",
> 
> Do newer builds really support all pixfmts? I know we have this
> init_static_data because it depended on runtime (and apparently x264
> build settings). I assume new x264 just does away with setting this at
> compile time, and always has code for all formats?

Old x264 supports either 8bit or 10bit pixfmts depending on compile time
configuration, so yes, init_static_data or something similar is needed
for it.

Even new x264, having support for both in the same build, could be
compiled with support for only one instead. I didn't take that into
consideration for this patch, now that you mention it. I made it set all
pix_fmts as supported.
Maybe i should check if X264_BIT_DEPTH is 0 (both bitdepths compiled
in), 8 or 10, before setting AVCodec.pix_fmts for it.

> 
> (Personally I welcome getting rid of init_static_data, since it'll be
> the only reason why AVCodec still can't be in read-only memory. Though
> libvpx has a similar problem too.)
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 2d36c5e566..29f60fa7b5 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -454,6 +454,7 @@  static av_cold int X264_init(AVCodecContext *avctx)
 {
     X264Context *x4 = avctx->priv_data;
     AVCPBProperties *cpb_props;
+    const av_unused AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
     int sw,sh;
 
     if (avctx->global_quality > 0)
@@ -724,6 +725,9 @@  FF_ENABLE_DEPRECATION_WARNINGS
 
     x4->params.i_width          = avctx->width;
     x4->params.i_height         = avctx->height;
+#if X264_BUILD >= 153
+    x4->params.i_bitdepth       = desc->comp[0].depth;
+#endif
     av_reduce(&sw, &sh, avctx->sample_aspect_ratio.num, avctx->sample_aspect_ratio.den, 4096);
     x4->params.vui.i_sar_width  = sw;
     x4->params.vui.i_sar_height = sh;
@@ -851,6 +855,34 @@  static const enum AVPixelFormat pix_fmts_8bit[] = {
 #endif
     AV_PIX_FMT_NONE
 };
+
+#if CONFIG_LIBX264RGB_ENCODER
+static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
+    AV_PIX_FMT_BGR0,
+    AV_PIX_FMT_BGR24,
+    AV_PIX_FMT_RGB24,
+    AV_PIX_FMT_NONE
+};
+#endif
+
+#if X264_BUILD >= 153
+static const enum AVPixelFormat pix_fmts[] = {
+    AV_PIX_FMT_YUV420P,
+    AV_PIX_FMT_YUVJ420P,
+    AV_PIX_FMT_YUV422P,
+    AV_PIX_FMT_YUVJ422P,
+    AV_PIX_FMT_YUV444P,
+    AV_PIX_FMT_YUVJ444P,
+    AV_PIX_FMT_YUV420P10,
+    AV_PIX_FMT_YUV422P10,
+    AV_PIX_FMT_YUV444P10,
+    AV_PIX_FMT_NV12,
+    AV_PIX_FMT_NV16,
+    AV_PIX_FMT_NV20,
+    AV_PIX_FMT_NV21,
+    AV_PIX_FMT_NONE
+};
+#else
 static const enum AVPixelFormat pix_fmts_9bit[] = {
     AV_PIX_FMT_YUV420P9,
     AV_PIX_FMT_YUV444P9,
@@ -863,14 +895,6 @@  static const enum AVPixelFormat pix_fmts_10bit[] = {
     AV_PIX_FMT_NV20,
     AV_PIX_FMT_NONE
 };
-#if CONFIG_LIBX264RGB_ENCODER
-static const enum AVPixelFormat pix_fmts_8bit_rgb[] = {
-    AV_PIX_FMT_BGR0,
-    AV_PIX_FMT_BGR24,
-    AV_PIX_FMT_RGB24,
-    AV_PIX_FMT_NONE
-};
-#endif
 
 static av_cold void X264_init_static(AVCodec *codec)
 {
@@ -881,6 +905,7 @@  static av_cold void X264_init_static(AVCodec *codec)
     else if (x264_bit_depth == 10)
         codec->pix_fmts = pix_fmts_10bit;
 }
+#endif
 
 #define OFFSET(x) offsetof(X264Context, x)
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
@@ -1024,7 +1049,11 @@  AVCodec ff_libx264_encoder = {
     .capabilities     = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS,
     .priv_class       = &x264_class,
     .defaults         = x264_defaults,
+#if X264_BUILD >= 153
+    .pix_fmts         = pix_fmts,
+#else
     .init_static_data = X264_init_static,
+#endif
     .caps_internal    = FF_CODEC_CAP_INIT_THREADSAFE |
                         FF_CODEC_CAP_INIT_CLEANUP,
     .wrapper_name     = "libx264",