[FFmpeg-devel] [PATCH] lavc/libopenh264: Check for noopenh264
Akihiko Odaki
akihiko.odaki at gmail.com
Sun Feb 25 11:42:42 EET 2024
On 2024/02/11 19:24, Andreas Rheinhardt wrote:
> 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 at 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,
Hi,
Thank you for reviewing patch.
>
> 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.
Right. What about filtering in av_codec_iterate()?
> 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.
Creating encoder/decoders sounds very expensive and prone to allocation
failure, but in reality it only allocates memory and fill vtable, which
should never fail in a healthy platform. The real initialization work is
done by ISVCEncoder::InitializeExt() or ISVCDecoder::Initialize(), where
parameters necessary to e.g., allocate buffers are given.
That said, I'd like to hear if you have an alternative idea.
> 4. The reindentation of the other initializers should be performed in a
> separate commit (if at all).
I'll do so if I'm going to submit v2.
Regards,
Akihiko Odaki
>
> - Andreas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list