Re: [FFmpeg-devel] [PATCH 1/7] lavc: factor out encoder init/validation from avcodec_open2()
Quoting James Almer (2021-03-10 16:17:35) > On 3/10/2021 9:03 AM, Anton Khirnov wrote: > > avcodec_open2() is massive, splitting it makes it more readable. > > > > Also, add a missing error code to ticks_per_frame sanity check. > > --- > > libavcodec/encode.c | 157 + > > libavcodec/encode.h | 6 ++ > > libavcodec/utils.c | 166 +--- > > 3 files changed, 166 insertions(+), 163 deletions(-) > > > > diff --git a/libavcodec/encode.c b/libavcodec/encode.c > > index 282337e453..bbf03d62fc 100644 > > --- a/libavcodec/encode.c > > +++ b/libavcodec/encode.c > > @@ -462,3 +462,160 @@ int attribute_align_arg > > avcodec_encode_video2(AVCodecContext *avctx, > > return ret; > > } > > #endif > > + > > +int ff_encode_preinit(AVCodecContext *avctx) > > nit: Would prefer if this and ff_decode_preinit() could stay in the same > file as avcodec_open2() as static functions. This includes moving > decode_bsfs_init() there, too. > decode.c and encode.c seem to me that they should contain functions used > during decoding and encoding, and not initialization. For the record: discussed this on IRC and James dropped his objection to the move > > That being said, not related to this set and not really a priority, but > avcodec_open2() is not a "utility" function as much as a core lavc > function. av_get_bits_per_sample() for example is a util, as are > av_get_audio_frame_duration() and avcodec_align_dimensions2(). So > perhaps it, avcodec_alloc_context3() and avcodec_free_context() should > be together (options.c is also not exactly the best name for the file > currently hosting the latter two, so maybe it could be renamed to > avcodec.c while at it). I would certainly be in favor of that. utils.c is way too huge. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/7] lavc: factor out encoder init/validation from avcodec_open2()
On 3/10/2021 9:03 AM, Anton Khirnov wrote: avcodec_open2() is massive, splitting it makes it more readable. Also, add a missing error code to ticks_per_frame sanity check. --- libavcodec/encode.c | 157 + libavcodec/encode.h | 6 ++ libavcodec/utils.c | 166 +--- 3 files changed, 166 insertions(+), 163 deletions(-) diff --git a/libavcodec/encode.c b/libavcodec/encode.c index 282337e453..bbf03d62fc 100644 --- a/libavcodec/encode.c +++ b/libavcodec/encode.c @@ -462,3 +462,160 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx, return ret; } #endif + +int ff_encode_preinit(AVCodecContext *avctx) nit: Would prefer if this and ff_decode_preinit() could stay in the same file as avcodec_open2() as static functions. This includes moving decode_bsfs_init() there, too. decode.c and encode.c seem to me that they should contain functions used during decoding and encoding, and not initialization. That being said, not related to this set and not really a priority, but avcodec_open2() is not a "utility" function as much as a core lavc function. av_get_bits_per_sample() for example is a util, as are av_get_audio_frame_duration() and avcodec_align_dimensions2(). So perhaps it, avcodec_alloc_context3() and avcodec_free_context() should be together (options.c is also not exactly the best name for the file currently hosting the latter two, so maybe it could be renamed to avcodec.c while at it). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/7] lavc: factor out encoder init/validation from avcodec_open2()
avcodec_open2() is massive, splitting it makes it more readable.
Also, add a missing error code to ticks_per_frame sanity check.
---
libavcodec/encode.c | 157 +
libavcodec/encode.h | 6 ++
libavcodec/utils.c | 166 +---
3 files changed, 166 insertions(+), 163 deletions(-)
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 282337e453..bbf03d62fc 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -462,3 +462,160 @@ int attribute_align_arg
avcodec_encode_video2(AVCodecContext *avctx,
return ret;
}
#endif
+
+int ff_encode_preinit(AVCodecContext *avctx)
+{
+int i;
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+avctx->coded_frame = av_frame_alloc();
+if (!avctx->coded_frame) {
+return AVERROR(ENOMEM);
+}
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+if (avctx->time_base.num <= 0 || avctx->time_base.den <= 0) {
+av_log(avctx, AV_LOG_ERROR, "The encoder timebase is not set.\n");
+return AVERROR(EINVAL);
+}
+
+if (avctx->codec->sample_fmts) {
+for (i = 0; avctx->codec->sample_fmts[i] != AV_SAMPLE_FMT_NONE;
i++) {
+if (avctx->sample_fmt == avctx->codec->sample_fmts[i])
+break;
+if (avctx->channels == 1 &&
+av_get_planar_sample_fmt(avctx->sample_fmt) ==
+av_get_planar_sample_fmt(avctx->codec->sample_fmts[i])) {
+avctx->sample_fmt = avctx->codec->sample_fmts[i];
+break;
+}
+}
+if (avctx->codec->sample_fmts[i] == AV_SAMPLE_FMT_NONE) {
+char buf[128];
+snprintf(buf, sizeof(buf), "%d", avctx->sample_fmt);
+av_log(avctx, AV_LOG_ERROR, "Specified sample format %s is
invalid or not supported\n",
+ (char
*)av_x_if_null(av_get_sample_fmt_name(avctx->sample_fmt), buf));
+return AVERROR(EINVAL);
+}
+}
+if (avctx->codec->pix_fmts) {
+for (i = 0; avctx->codec->pix_fmts[i] != AV_PIX_FMT_NONE; i++)
+if (avctx->pix_fmt == avctx->codec->pix_fmts[i])
+break;
+if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_NONE
+&& !((avctx->codec_id == AV_CODEC_ID_MJPEG || avctx->codec_id
== AV_CODEC_ID_LJPEG)
+ && avctx->strict_std_compliance <=
FF_COMPLIANCE_UNOFFICIAL)) {
+char buf[128];
+snprintf(buf, sizeof(buf), "%d", avctx->pix_fmt);
+av_log(avctx, AV_LOG_ERROR, "Specified pixel format %s is
invalid or not supported\n",
+ (char
*)av_x_if_null(av_get_pix_fmt_name(avctx->pix_fmt), buf));
+return AVERROR(EINVAL);
+}
+if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_YUVJ420P ||
+avctx->codec->pix_fmts[i] == AV_PIX_FMT_YUVJ411P ||
+avctx->codec->pix_fmts[i] == AV_PIX_FMT_YUVJ422P ||
+avctx->codec->pix_fmts[i] == AV_PIX_FMT_YUVJ440P ||
+avctx->codec->pix_fmts[i] == AV_PIX_FMT_YUVJ444P)
+avctx->color_range = AVCOL_RANGE_JPEG;
+}
+if (avctx->codec->supported_samplerates) {
+for (i = 0; avctx->codec->supported_samplerates[i] != 0; i++)
+if (avctx->sample_rate ==
avctx->codec->supported_samplerates[i])
+break;
+if (avctx->codec->supported_samplerates[i] == 0) {
+av_log(avctx, AV_LOG_ERROR, "Specified sample rate %d is not
supported\n",
+ avctx->sample_rate);
+return AVERROR(EINVAL);
+}
+}
+if (avctx->sample_rate < 0) {
+av_log(avctx, AV_LOG_ERROR, "Specified sample rate %d is not
supported\n",
+avctx->sample_rate);
+return AVERROR(EINVAL);
+}
+if (avctx->codec->channel_layouts) {
+if (!avctx->channel_layout) {
+av_log(avctx, AV_LOG_WARNING, "Channel layout not
specified\n");
+} else {
+for (i = 0; avctx->codec->channel_layouts[i] != 0; i++)
+if (avctx->channel_layout ==
avctx->codec->channel_layouts[i])
+break;
+if (avctx->codec->channel_layouts[i] == 0) {
+char buf[512];
+av_get_channel_layout_string(buf, sizeof(buf), -1,
avctx->channel_layout);
+av_log(avctx, AV_LOG_ERROR, "Specified channel layout '%s'
is not supported\n", buf);
+return AVERROR(EINVAL);
+}
+}
+}
+if (avctx->channel_layout && avctx->channels) {
+int channels =
av_get_channel_layout_nb_channels(avctx->channel_layout);
+if
