From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 136BB471AC for ; Sun, 25 Feb 2024 09:42:58 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 35CEB68C67B; Sun, 25 Feb 2024 11:42:55 +0200 (EET) Received: from mail-oi1-f175.google.com (mail-oi1-f175.google.com [209.85.167.175]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id CAB6368BDD3 for ; Sun, 25 Feb 2024 11:42:48 +0200 (EET) Received: by mail-oi1-f175.google.com with SMTP id 5614622812f47-3c15b7b36bbso831753b6e.1 for ; Sun, 25 Feb 2024 01:42:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708854166; x=1709458966; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=MtI0xQC/O3o0Hn+TPc21kguRnr5CcFzwerq8C7xZTkA=; b=P4KXOLGItWIWkug+KqehpDJbdcXq3t441jK/iVWVHsrlRh5QrK4lShQ8ugnkcVGFGj bjzBXEsj0YmYE5sE+/G5IRw7K4SL9ddrYuZMZiFbzjsvH5al0gI7TmWnn+ZBgYwaE2db 75IGA9UXREdHY7roRGZvocPWWhWpkXD0Hpc7EZtpvrUtrHpSPp2ttha3buz8hw82PEkD ZZpc7AWlG4VJi7DUorugwVRc+RMwkdwrhPErYkRLAu8sxjIJYsV63O4v+jZnnvYaU46l pBCXd/VuzvKP+Irzj6Ij9vXKzbvdmA5aq8/r3aUOshr3NYnubi/4xd4oC5k4D5V+kiid uGKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708854166; x=1709458966; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MtI0xQC/O3o0Hn+TPc21kguRnr5CcFzwerq8C7xZTkA=; b=hqVbnsCsxJR6VNvtZtcRfM6/VcdMawCWCumZM7UuQIv9+shwbret/hvgXi1w1B81IF UV+IcerWNjXadxWsUTTiAW78SRg0cnHFNJPuGRTzHetZsg2UldTLy54f+a0j/IENdWqQ ooFCOaxaA7PlU8545kNeGN4Q9F4yH+thr/9g+uG8wRUGhO9VzhrAgw6Uf/2R6TCo1+MB WcPltashklQ8H24ipSYZGIYn4bvn2huS6TnvNM82dMBXt9VEN8rYfSl+KiNLlNtF2Qxm xUtYCtctZtKfHDPjxCo+NeV2tyES90UrGMG+KLF45ZZQ5NSk52of0o2cAoTbPxBT2OeP Q46w== X-Gm-Message-State: AOJu0YysoLpi1nBesGa8W2SAxnr/1dmWIyuqxnXiTWMblk8//XmcVuVh 4oy4CjvTl4nG4BBglSCyeZDBRFxKvCYWGnz+TSwJ1wbQBW6oCnj+2C1sfEIP X-Google-Smtp-Source: AGHT+IH4xAndq/uVSPN3OU3uwIZ7GamNSevv6wfglLLS+UuqtmZTXYXH3XV4bPR2NWduLP1ZEf/Iow== X-Received: by 2002:a05:6808:1493:b0:3c1:9b7b:5836 with SMTP id e19-20020a056808149300b003c19b7b5836mr1988038oiw.12.1708854165580; Sun, 25 Feb 2024 01:42:45 -0800 (PST) Received: from [157.82.203.206] ([157.82.203.206]) by smtp.gmail.com with ESMTPSA id mf16-20020a17090b185000b00299db19a81asm4348496pjb.56.2024.02.25.01.42.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 25 Feb 2024 01:42:45 -0800 (PST) Message-ID: <52948e35-7afe-4410-8588-ca755adfbd66@gmail.com> Date: Sun, 25 Feb 2024 18:42:42 +0900 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: FFmpeg development discussions and patches , Andreas Rheinhardt References: <20240211-noopenh264-v1-1-e18090bdcfed@gmail.com> From: Akihiko Odaki In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] lavc/libopenh264: Check for noopenh264 X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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 >> --- >> 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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ 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".