Re: [FFmpeg-devel] [PATCH] lavc/libopenh264: Check for noopenh264

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Akihiko Odaki:
> noopenh264 is a "fake implementation of the OpenH264 library we can link
> from regardless of the actual library being available":
> https://gitlab.com/freedesktop-sdk/noopenh264
> 
> A distributor may wish to link against openh264/noopenh264 and let
> the decoder and encoder work only if the actual library is available.
> 
> On the other hand, an application may want to know if the decoder or
> encoder is available beforehand so that it can determine what format to
> download for decoding, or what format to advertise for the peer
> receiving the encoded video.
> 
> Check the availability of the actual library at runtime initialization
> and do not expose the encoder and decoder if they are not available.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
>  libavcodec/codec_internal.h |  2 ++
>  libavcodec/libopenh264dec.c | 36 +++++++++++++++++++++++------------
>  libavcodec/libopenh264enc.c | 46 ++++++++++++++++++++++++++++-----------------
>  libavcodec/tests/avcodec.c  |  2 ++
>  4 files changed, 57 insertions(+), 29 deletions(-)
> 
> diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h
> index 130a7dc3cd..635af644fa 100644
> --- a/libavcodec/codec_internal.h
> +++ b/libavcodec/codec_internal.h
> @@ -122,6 +122,8 @@ enum FFCodecType {
>      /* The codec is an encoder using the receive_packet callback;
>       * audio and video codecs only. */
>      FF_CODEC_CB_TYPE_RECEIVE_PACKET,
> +    /* The codec is not available. */
> +    FF_CODEC_CB_TYPE_NONE,
>  };
>  
>  typedef struct FFCodec {
> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
> index b6a9bba2dc..f5544142aa 100644
> --- a/libavcodec/libopenh264dec.c
> +++ b/libavcodec/libopenh264dec.c
> @@ -48,6 +48,17 @@ static av_cold int svc_decode_close(AVCodecContext *avctx)
>      return 0;
>  }
>  
> +static av_cold void svc_decode_init_static_data(FFCodec *codec)
> +{
> +    ISVCDecoder *decoder;
> +
> +    if (WelsCreateDecoder(&decoder)) {
> +        codec->cb_type = FF_CODEC_CB_TYPE_NONE;
> +    } else {
> +        WelsDestroyDecoder(decoder);
> +    }
> +}
> +
>  static av_cold int svc_decode_init(AVCodecContext *avctx)
>  {
>      SVCContext *s = avctx->priv_data;
> @@ -153,18 +164,19 @@ static int svc_decode_frame(AVCodecContext *avctx, AVFrame *avframe,
>      return avpkt->size;
>  }
>  
> -const FFCodec ff_libopenh264_decoder = {
> -    .p.name         = "libopenh264",
> +FFCodec ff_libopenh264_decoder = {
> +    .p.name           = "libopenh264",
>      CODEC_LONG_NAME("OpenH264 H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10"),
> -    .p.type         = AVMEDIA_TYPE_VIDEO,
> -    .p.id           = AV_CODEC_ID_H264,
> -    .priv_data_size = sizeof(SVCContext),
> -    .init           = svc_decode_init,
> +    .p.type           = AVMEDIA_TYPE_VIDEO,
> +    .p.id             = AV_CODEC_ID_H264,
> +    .priv_data_size   = sizeof(SVCContext),
> +    .init_static_data = svc_decode_init_static_data,
> +    .init             = svc_decode_init,
>      FF_CODEC_DECODE_CB(svc_decode_frame),
> -    .close          = svc_decode_close,
> -    .p.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1,
> -    .caps_internal  = FF_CODEC_CAP_SETS_PKT_DTS |
> -                      FF_CODEC_CAP_INIT_CLEANUP,
> -    .bsfs           = "h264_mp4toannexb",
> -    .p.wrapper_name = "libopenh264",
> +    .close            = svc_decode_close,
> +    .p.capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1,
> +    .caps_internal    = FF_CODEC_CAP_SETS_PKT_DTS |
> +                        FF_CODEC_CAP_INIT_CLEANUP,
> +    .bsfs             = "h264_mp4toannexb",
> +    .p.wrapper_name   = "libopenh264",
>  };
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index 6f231d22b2..1963f2cf5c 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -106,6 +106,17 @@ static av_cold int svc_encode_close(AVCodecContext *avctx)
>      return 0;
>  }
>  
> +static av_cold void svc_encode_init_static_data(FFCodec *codec)
> +{
> +    ISVCEncoder *encoder;
> +
> +    if (WelsCreateSVCEncoder(&encoder)) {
> +        codec->cb_type = FF_CODEC_CB_TYPE_NONE;
> +    } else {
> +        WelsDestroySVCEncoder(encoder);
> +    }
> +}
> +
>  static av_cold int svc_encode_init(AVCodecContext *avctx)
>  {
>      SVCContext *s = avctx->priv_data;
> @@ -429,23 +440,24 @@ static const FFCodecDefault svc_enc_defaults[] = {
>      { NULL },
>  };
>  
> -const FFCodec ff_libopenh264_encoder = {
> -    .p.name         = "libopenh264",
> +FFCodec ff_libopenh264_encoder = {
> +    .p.name           = "libopenh264",
>      CODEC_LONG_NAME("OpenH264 H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10"),
> -    .p.type         = AVMEDIA_TYPE_VIDEO,
> -    .p.id           = AV_CODEC_ID_H264,
> -    .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_OTHER_THREADS |
> -                      AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
> -    .priv_data_size = sizeof(SVCContext),
> -    .init           = svc_encode_init,
> +    .p.type           = AVMEDIA_TYPE_VIDEO,
> +    .p.id             = AV_CODEC_ID_H264,
> +    .p.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_OTHER_THREADS |
> +                        AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE,
> +    .priv_data_size   = sizeof(SVCContext),
> +    .init_static_data = svc_encode_init_static_data,
> +    .init             = svc_encode_init,
>      FF_CODEC_ENCODE_CB(svc_encode_frame),
> -    .close          = svc_encode_close,
> -    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP |
> -                      FF_CODEC_CAP_AUTO_THREADS,
> -    .p.pix_fmts     = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P,
> -                                                    AV_PIX_FMT_YUVJ420P,
> -                                                    AV_PIX_FMT_NONE },
> -    .defaults       = svc_enc_defaults,
> -    .p.priv_class   = &class,
> -    .p.wrapper_name = "libopenh264",
> +    .close            = svc_encode_close,
> +    .caps_internal    = FF_CODEC_CAP_INIT_CLEANUP |
> +                        FF_CODEC_CAP_AUTO_THREADS,
> +    .p.pix_fmts       = (const enum AVPixelFormat[]){ AV_PIX_FMT_YUV420P,
> +                                                      AV_PIX_FMT_YUVJ420P,
> +                                                      AV_PIX_FMT_NONE },
> +    .defaults         = svc_enc_defaults,
> +    .p.priv_class     = &class,
> +    .p.wrapper_name   = "libopenh264",
>  };
> diff --git a/libavcodec/tests/avcodec.c b/libavcodec/tests/avcodec.c
> index 08ca507bf0..0e2ecccbf9 100644
> --- a/libavcodec/tests/avcodec.c
> +++ b/libavcodec/tests/avcodec.c
> @@ -112,6 +112,8 @@ int main(void){
>          case FF_CODEC_CB_TYPE_RECEIVE_PACKET:
>              is_encoder = 1;
>              break;
> +        case FF_CODEC_CB_TYPE_NONE:
> +            continue;
>          default:
>              ERR("Codec %s has unknown cb_type\n");
>              continue;
> 
> ---
> base-commit: 81c2557691b12ceb79b3ba92aa496f2301ab4d18
> change-id: 20240210-noopenh264-650461efbc33
> 
> Best regards,

1. Your patch will only make these codecs unavailable when searching for
them by name or by codec id, but not when iterating over all codecs.
2. The init_static_data callbacks are supposed to only perform very
light work and not create encoder/decoders. After all, they are run
whenever one requests any codec, even when one is not interested in
these codecs.
3. In case of e.g. a temporary allocation failure, creating these codec
contexts can fail, even if the proper library is present.
4. The reindentation of the other initializers should be performed in a
separate commit (if at all).

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".