* [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
@ 2022-03-23 15:57 Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders Anton Khirnov
` (9 more replies)
0 siblings, 10 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
On entry to avcodec_open2(), the type and id either have to be
UNKNOWN/NONE or have to match the codec to be used.
---
libavcodec/avcodec.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index fbe4a5e413..dbaa9f78a2 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -158,17 +158,15 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
codec = avctx->codec;
codec2 = ffcodec(codec);
- if ((avctx->codec_type == AVMEDIA_TYPE_UNKNOWN || avctx->codec_type == codec->type) &&
- avctx->codec_id == AV_CODEC_ID_NONE) {
- avctx->codec_type = codec->type;
- avctx->codec_id = codec->id;
- }
- if (avctx->codec_id != codec->id || (avctx->codec_type != codec->type &&
- avctx->codec_type != AVMEDIA_TYPE_ATTACHMENT)) {
+ if ((avctx->codec_type != AVMEDIA_TYPE_UNKNOWN && avctx->codec_type != codec->type) ||
+ (avctx->codec_id != AV_CODEC_ID_NONE && avctx->codec_id != codec->id)) {
av_log(avctx, AV_LOG_ERROR, "Codec type or id mismatches\n");
return AVERROR(EINVAL);
}
- avctx->codec = codec;
+
+ avctx->codec_type = codec->type;
+ avctx->codec_id = codec->id;
+ avctx->codec = codec;
if (avctx->extradata_size < 0 || avctx->extradata_size >= FF_MAX_EXTRADATA_SIZE)
return AVERROR(EINVAL);
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-03-23 16:29 ` James Almer
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 3/8] lavc: move default get_buffer2() to its own file Anton Khirnov
` (8 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
And only when needed, i.e. for encoders using the simple API.
---
libavcodec/avcodec.c | 4 +---
libavcodec/encode.c | 7 +++++++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index dbaa9f78a2..c7daa385e7 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -180,14 +180,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
avci->buffer_frame = av_frame_alloc();
avci->buffer_pkt = av_packet_alloc();
- avci->es.in_frame = av_frame_alloc();
avci->in_pkt = av_packet_alloc();
avci->last_pkt_props = av_packet_alloc();
avci->pkt_props = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
AV_FIFO_FLAG_AUTO_GROW);
if (!avci->buffer_frame || !avci->buffer_pkt ||
- !avci->es.in_frame || !avci->in_pkt ||
- !avci->last_pkt_props || !avci->pkt_props) {
+ !avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props) {
ret = AVERROR(ENOMEM);
goto free_and_end;
}
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 70bd8da81f..837ffaa40d 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -412,6 +412,7 @@ int attribute_align_arg avcodec_receive_packet(AVCodecContext *avctx, AVPacket *
int ff_encode_preinit(AVCodecContext *avctx)
{
+ AVCodecInternal *avci = avctx->internal;
int i;
if (avctx->time_base.num <= 0 || avctx->time_base.den <= 0) {
@@ -563,5 +564,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
if (avctx->codec_descriptor->props & AV_CODEC_PROP_INTRA_ONLY)
avctx->internal->intra_only_flag = AV_PKT_FLAG_KEY;
+ if (ffcodec(avctx->codec)->encode2) {
+ avci->es.in_frame = av_frame_alloc();
+ if (!avci->es.in_frame)
+ return AVERROR(ENOMEM);
+ }
+
return 0;
}
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 3/8] lavc: move default get_buffer2() to its own file
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 4/8] lavc/snow: only allocate mconly_picture for decoding Anton Khirnov
` (7 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
It is also used by some encoders, so decode.c is not the right place for
it.
---
libavcodec/Makefile | 1 +
libavcodec/decode.c | 278 -----------------------------------
libavcodec/get_buffer.c | 312 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 313 insertions(+), 278 deletions(-)
create mode 100644 libavcodec/get_buffer.c
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index fb8b0e824b..a24d8a3873 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -43,6 +43,7 @@ OBJS = ac3_parser.o \
dirac.o \
dv_profile.o \
encode.o \
+ get_buffer.o \
imgconvert.o \
jni.o \
mathtables.o \
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index f9fdb935f6..fffad5f700 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -48,25 +48,6 @@
#include "internal.h"
#include "thread.h"
-typedef struct FramePool {
- /**
- * Pools for each data plane. For audio all the planes have the same size,
- * so only pools[0] is used.
- */
- AVBufferPool *pools[4];
-
- /*
- * Pool parameters
- */
- int format;
- int width, height;
- int stride_align[AV_NUM_DATA_POINTERS];
- int linesize[4];
- int planes;
- int channels;
- int samples;
-} FramePool;
-
static int apply_param_change(AVCodecContext *avctx, const AVPacket *avpkt)
{
int ret;
@@ -1250,265 +1231,6 @@ int ff_get_format(AVCodecContext *avctx, const enum AVPixelFormat *fmt)
return ret;
}
-static void frame_pool_free(void *opaque, uint8_t *data)
-{
- FramePool *pool = (FramePool*)data;
- int i;
-
- for (i = 0; i < FF_ARRAY_ELEMS(pool->pools); i++)
- av_buffer_pool_uninit(&pool->pools[i]);
-
- av_freep(&data);
-}
-
-static AVBufferRef *frame_pool_alloc(void)
-{
- FramePool *pool = av_mallocz(sizeof(*pool));
- AVBufferRef *buf;
-
- if (!pool)
- return NULL;
-
- buf = av_buffer_create((uint8_t*)pool, sizeof(*pool),
- frame_pool_free, NULL, 0);
- if (!buf) {
- av_freep(&pool);
- return NULL;
- }
-
- return buf;
-}
-
-static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
-{
- FramePool *pool = avctx->internal->pool ?
- (FramePool*)avctx->internal->pool->data : NULL;
- AVBufferRef *pool_buf;
- int i, ret, ch, planes;
-
- if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) {
- int planar = av_sample_fmt_is_planar(frame->format);
- ch = frame->ch_layout.nb_channels;
-#if FF_API_OLD_CHANNEL_LAYOUT
-FF_DISABLE_DEPRECATION_WARNINGS
- if (!ch)
- ch = frame->channels;
-FF_ENABLE_DEPRECATION_WARNINGS
-#endif
- planes = planar ? ch : 1;
- }
-
- if (pool && pool->format == frame->format) {
- if (avctx->codec_type == AVMEDIA_TYPE_VIDEO &&
- pool->width == frame->width && pool->height == frame->height)
- return 0;
- if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && pool->planes == planes &&
- pool->channels == ch && frame->nb_samples == pool->samples)
- return 0;
- }
-
- pool_buf = frame_pool_alloc();
- if (!pool_buf)
- return AVERROR(ENOMEM);
- pool = (FramePool*)pool_buf->data;
-
- switch (avctx->codec_type) {
- case AVMEDIA_TYPE_VIDEO: {
- int linesize[4];
- int w = frame->width;
- int h = frame->height;
- int unaligned;
- ptrdiff_t linesize1[4];
- size_t size[4];
-
- avcodec_align_dimensions2(avctx, &w, &h, pool->stride_align);
-
- do {
- // NOTE: do not align linesizes individually, this breaks e.g. assumptions
- // that linesize[0] == 2*linesize[1] in the MPEG-encoder for 4:2:2
- ret = av_image_fill_linesizes(linesize, avctx->pix_fmt, w);
- if (ret < 0)
- goto fail;
- // increase alignment of w for next try (rhs gives the lowest bit set in w)
- w += w & ~(w - 1);
-
- unaligned = 0;
- for (i = 0; i < 4; i++)
- unaligned |= linesize[i] % pool->stride_align[i];
- } while (unaligned);
-
- for (i = 0; i < 4; i++)
- linesize1[i] = linesize[i];
- ret = av_image_fill_plane_sizes(size, avctx->pix_fmt, h, linesize1);
- if (ret < 0)
- goto fail;
-
- for (i = 0; i < 4; i++) {
- pool->linesize[i] = linesize[i];
- if (size[i]) {
- if (size[i] > INT_MAX - (16 + STRIDE_ALIGN - 1)) {
- ret = AVERROR(EINVAL);
- goto fail;
- }
- pool->pools[i] = av_buffer_pool_init(size[i] + 16 + STRIDE_ALIGN - 1,
- CONFIG_MEMORY_POISONING ?
- NULL :
- av_buffer_allocz);
- if (!pool->pools[i]) {
- ret = AVERROR(ENOMEM);
- goto fail;
- }
- }
- }
- pool->format = frame->format;
- pool->width = frame->width;
- pool->height = frame->height;
-
- break;
- }
- case AVMEDIA_TYPE_AUDIO: {
- ret = av_samples_get_buffer_size(&pool->linesize[0], ch,
- frame->nb_samples, frame->format, 0);
- if (ret < 0)
- goto fail;
-
- pool->pools[0] = av_buffer_pool_init(pool->linesize[0], NULL);
- if (!pool->pools[0]) {
- ret = AVERROR(ENOMEM);
- goto fail;
- }
-
- pool->format = frame->format;
- pool->planes = planes;
- pool->channels = ch;
- pool->samples = frame->nb_samples;
- break;
- }
- default: av_assert0(0);
- }
-
- av_buffer_unref(&avctx->internal->pool);
- avctx->internal->pool = pool_buf;
-
- return 0;
-fail:
- av_buffer_unref(&pool_buf);
- return ret;
-}
-
-static int audio_get_buffer(AVCodecContext *avctx, AVFrame *frame)
-{
- FramePool *pool = (FramePool*)avctx->internal->pool->data;
- int planes = pool->planes;
- int i;
-
- frame->linesize[0] = pool->linesize[0];
-
- if (planes > AV_NUM_DATA_POINTERS) {
- frame->extended_data = av_calloc(planes, sizeof(*frame->extended_data));
- frame->nb_extended_buf = planes - AV_NUM_DATA_POINTERS;
- frame->extended_buf = av_calloc(frame->nb_extended_buf,
- sizeof(*frame->extended_buf));
- if (!frame->extended_data || !frame->extended_buf) {
- av_freep(&frame->extended_data);
- av_freep(&frame->extended_buf);
- return AVERROR(ENOMEM);
- }
- } else {
- frame->extended_data = frame->data;
- av_assert0(frame->nb_extended_buf == 0);
- }
-
- for (i = 0; i < FFMIN(planes, AV_NUM_DATA_POINTERS); i++) {
- frame->buf[i] = av_buffer_pool_get(pool->pools[0]);
- if (!frame->buf[i])
- goto fail;
- frame->extended_data[i] = frame->data[i] = frame->buf[i]->data;
- }
- for (i = 0; i < frame->nb_extended_buf; i++) {
- frame->extended_buf[i] = av_buffer_pool_get(pool->pools[0]);
- if (!frame->extended_buf[i])
- goto fail;
- frame->extended_data[i + AV_NUM_DATA_POINTERS] = frame->extended_buf[i]->data;
- }
-
- if (avctx->debug & FF_DEBUG_BUFFERS)
- av_log(avctx, AV_LOG_DEBUG, "default_get_buffer called on frame %p", frame);
-
- return 0;
-fail:
- av_frame_unref(frame);
- return AVERROR(ENOMEM);
-}
-
-static int video_get_buffer(AVCodecContext *s, AVFrame *pic)
-{
- FramePool *pool = (FramePool*)s->internal->pool->data;
- const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pic->format);
- int i;
-
- if (pic->data[0] || pic->data[1] || pic->data[2] || pic->data[3]) {
- av_log(s, AV_LOG_ERROR, "pic->data[*]!=NULL in avcodec_default_get_buffer\n");
- return -1;
- }
-
- if (!desc) {
- av_log(s, AV_LOG_ERROR,
- "Unable to get pixel format descriptor for format %s\n",
- av_get_pix_fmt_name(pic->format));
- return AVERROR(EINVAL);
- }
-
- memset(pic->data, 0, sizeof(pic->data));
- pic->extended_data = pic->data;
-
- for (i = 0; i < 4 && pool->pools[i]; i++) {
- pic->linesize[i] = pool->linesize[i];
-
- pic->buf[i] = av_buffer_pool_get(pool->pools[i]);
- if (!pic->buf[i])
- goto fail;
-
- pic->data[i] = pic->buf[i]->data;
- }
- for (; i < AV_NUM_DATA_POINTERS; i++) {
- pic->data[i] = NULL;
- pic->linesize[i] = 0;
- }
-
- if (s->debug & FF_DEBUG_BUFFERS)
- av_log(s, AV_LOG_DEBUG, "default_get_buffer called on pic %p\n", pic);
-
- return 0;
-fail:
- av_frame_unref(pic);
- return AVERROR(ENOMEM);
-}
-
-int avcodec_default_get_buffer2(AVCodecContext *avctx, AVFrame *frame, int flags)
-{
- int ret;
-
- if (avctx->hw_frames_ctx) {
- ret = av_hwframe_get_buffer(avctx->hw_frames_ctx, frame, 0);
- frame->width = avctx->coded_width;
- frame->height = avctx->coded_height;
- return ret;
- }
-
- if ((ret = update_frame_pool(avctx, frame)) < 0)
- return ret;
-
- switch (avctx->codec_type) {
- case AVMEDIA_TYPE_VIDEO:
- return video_get_buffer(avctx, frame);
- case AVMEDIA_TYPE_AUDIO:
- return audio_get_buffer(avctx, frame);
- default:
- return -1;
- }
-}
-
static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
{
size_t size;
diff --git a/libavcodec/get_buffer.c b/libavcodec/get_buffer.c
new file mode 100644
index 0000000000..3e45a0479f
--- /dev/null
+++ b/libavcodec/get_buffer.c
@@ -0,0 +1,312 @@
+/*
+ * The default get_buffer2() implementation
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdint.h>
+
+#include "libavutil/avassert.h"
+#include "libavutil/avutil.h"
+#include "libavutil/buffer.h"
+#include "libavutil/frame.h"
+#include "libavutil/hwcontext.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/mem.h"
+#include "libavutil/samplefmt.h"
+#include "libavutil/version.h"
+
+#include "avcodec.h"
+#include "internal.h"
+
+typedef struct FramePool {
+ /**
+ * Pools for each data plane. For audio all the planes have the same size,
+ * so only pools[0] is used.
+ */
+ AVBufferPool *pools[4];
+
+ /*
+ * Pool parameters
+ */
+ int format;
+ int width, height;
+ int stride_align[AV_NUM_DATA_POINTERS];
+ int linesize[4];
+ int planes;
+ int channels;
+ int samples;
+} FramePool;
+
+static void frame_pool_free(void *opaque, uint8_t *data)
+{
+ FramePool *pool = (FramePool*)data;
+ int i;
+
+ for (i = 0; i < FF_ARRAY_ELEMS(pool->pools); i++)
+ av_buffer_pool_uninit(&pool->pools[i]);
+
+ av_freep(&data);
+}
+
+static AVBufferRef *frame_pool_alloc(void)
+{
+ FramePool *pool = av_mallocz(sizeof(*pool));
+ AVBufferRef *buf;
+
+ if (!pool)
+ return NULL;
+
+ buf = av_buffer_create((uint8_t*)pool, sizeof(*pool),
+ frame_pool_free, NULL, 0);
+ if (!buf) {
+ av_freep(&pool);
+ return NULL;
+ }
+
+ return buf;
+}
+
+static int update_frame_pool(AVCodecContext *avctx, AVFrame *frame)
+{
+ FramePool *pool = avctx->internal->pool ?
+ (FramePool*)avctx->internal->pool->data : NULL;
+ AVBufferRef *pool_buf;
+ int i, ret, ch, planes;
+
+ if (avctx->codec_type == AVMEDIA_TYPE_AUDIO) {
+ int planar = av_sample_fmt_is_planar(frame->format);
+ ch = frame->ch_layout.nb_channels;
+#if FF_API_OLD_CHANNEL_LAYOUT
+FF_DISABLE_DEPRECATION_WARNINGS
+ if (!ch)
+ ch = frame->channels;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+ planes = planar ? ch : 1;
+ }
+
+ if (pool && pool->format == frame->format) {
+ if (avctx->codec_type == AVMEDIA_TYPE_VIDEO &&
+ pool->width == frame->width && pool->height == frame->height)
+ return 0;
+ if (avctx->codec_type == AVMEDIA_TYPE_AUDIO && pool->planes == planes &&
+ pool->channels == ch && frame->nb_samples == pool->samples)
+ return 0;
+ }
+
+ pool_buf = frame_pool_alloc();
+ if (!pool_buf)
+ return AVERROR(ENOMEM);
+ pool = (FramePool*)pool_buf->data;
+
+ switch (avctx->codec_type) {
+ case AVMEDIA_TYPE_VIDEO: {
+ int linesize[4];
+ int w = frame->width;
+ int h = frame->height;
+ int unaligned;
+ ptrdiff_t linesize1[4];
+ size_t size[4];
+
+ avcodec_align_dimensions2(avctx, &w, &h, pool->stride_align);
+
+ do {
+ // NOTE: do not align linesizes individually, this breaks e.g. assumptions
+ // that linesize[0] == 2*linesize[1] in the MPEG-encoder for 4:2:2
+ ret = av_image_fill_linesizes(linesize, avctx->pix_fmt, w);
+ if (ret < 0)
+ goto fail;
+ // increase alignment of w for next try (rhs gives the lowest bit set in w)
+ w += w & ~(w - 1);
+
+ unaligned = 0;
+ for (i = 0; i < 4; i++)
+ unaligned |= linesize[i] % pool->stride_align[i];
+ } while (unaligned);
+
+ for (i = 0; i < 4; i++)
+ linesize1[i] = linesize[i];
+ ret = av_image_fill_plane_sizes(size, avctx->pix_fmt, h, linesize1);
+ if (ret < 0)
+ goto fail;
+
+ for (i = 0; i < 4; i++) {
+ pool->linesize[i] = linesize[i];
+ if (size[i]) {
+ if (size[i] > INT_MAX - (16 + STRIDE_ALIGN - 1)) {
+ ret = AVERROR(EINVAL);
+ goto fail;
+ }
+ pool->pools[i] = av_buffer_pool_init(size[i] + 16 + STRIDE_ALIGN - 1,
+ CONFIG_MEMORY_POISONING ?
+ NULL :
+ av_buffer_allocz);
+ if (!pool->pools[i]) {
+ ret = AVERROR(ENOMEM);
+ goto fail;
+ }
+ }
+ }
+ pool->format = frame->format;
+ pool->width = frame->width;
+ pool->height = frame->height;
+
+ break;
+ }
+ case AVMEDIA_TYPE_AUDIO: {
+ ret = av_samples_get_buffer_size(&pool->linesize[0], ch,
+ frame->nb_samples, frame->format, 0);
+ if (ret < 0)
+ goto fail;
+
+ pool->pools[0] = av_buffer_pool_init(pool->linesize[0], NULL);
+ if (!pool->pools[0]) {
+ ret = AVERROR(ENOMEM);
+ goto fail;
+ }
+
+ pool->format = frame->format;
+ pool->planes = planes;
+ pool->channels = ch;
+ pool->samples = frame->nb_samples;
+ break;
+ }
+ default: av_assert0(0);
+ }
+
+ av_buffer_unref(&avctx->internal->pool);
+ avctx->internal->pool = pool_buf;
+
+ return 0;
+fail:
+ av_buffer_unref(&pool_buf);
+ return ret;
+}
+
+static int audio_get_buffer(AVCodecContext *avctx, AVFrame *frame)
+{
+ FramePool *pool = (FramePool*)avctx->internal->pool->data;
+ int planes = pool->planes;
+ int i;
+
+ frame->linesize[0] = pool->linesize[0];
+
+ if (planes > AV_NUM_DATA_POINTERS) {
+ frame->extended_data = av_calloc(planes, sizeof(*frame->extended_data));
+ frame->nb_extended_buf = planes - AV_NUM_DATA_POINTERS;
+ frame->extended_buf = av_calloc(frame->nb_extended_buf,
+ sizeof(*frame->extended_buf));
+ if (!frame->extended_data || !frame->extended_buf) {
+ av_freep(&frame->extended_data);
+ av_freep(&frame->extended_buf);
+ return AVERROR(ENOMEM);
+ }
+ } else {
+ frame->extended_data = frame->data;
+ av_assert0(frame->nb_extended_buf == 0);
+ }
+
+ for (i = 0; i < FFMIN(planes, AV_NUM_DATA_POINTERS); i++) {
+ frame->buf[i] = av_buffer_pool_get(pool->pools[0]);
+ if (!frame->buf[i])
+ goto fail;
+ frame->extended_data[i] = frame->data[i] = frame->buf[i]->data;
+ }
+ for (i = 0; i < frame->nb_extended_buf; i++) {
+ frame->extended_buf[i] = av_buffer_pool_get(pool->pools[0]);
+ if (!frame->extended_buf[i])
+ goto fail;
+ frame->extended_data[i + AV_NUM_DATA_POINTERS] = frame->extended_buf[i]->data;
+ }
+
+ if (avctx->debug & FF_DEBUG_BUFFERS)
+ av_log(avctx, AV_LOG_DEBUG, "default_get_buffer called on frame %p", frame);
+
+ return 0;
+fail:
+ av_frame_unref(frame);
+ return AVERROR(ENOMEM);
+}
+
+static int video_get_buffer(AVCodecContext *s, AVFrame *pic)
+{
+ FramePool *pool = (FramePool*)s->internal->pool->data;
+ const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pic->format);
+ int i;
+
+ if (pic->data[0] || pic->data[1] || pic->data[2] || pic->data[3]) {
+ av_log(s, AV_LOG_ERROR, "pic->data[*]!=NULL in avcodec_default_get_buffer\n");
+ return -1;
+ }
+
+ if (!desc) {
+ av_log(s, AV_LOG_ERROR,
+ "Unable to get pixel format descriptor for format %s\n",
+ av_get_pix_fmt_name(pic->format));
+ return AVERROR(EINVAL);
+ }
+
+ memset(pic->data, 0, sizeof(pic->data));
+ pic->extended_data = pic->data;
+
+ for (i = 0; i < 4 && pool->pools[i]; i++) {
+ pic->linesize[i] = pool->linesize[i];
+
+ pic->buf[i] = av_buffer_pool_get(pool->pools[i]);
+ if (!pic->buf[i])
+ goto fail;
+
+ pic->data[i] = pic->buf[i]->data;
+ }
+ for (; i < AV_NUM_DATA_POINTERS; i++) {
+ pic->data[i] = NULL;
+ pic->linesize[i] = 0;
+ }
+
+ if (s->debug & FF_DEBUG_BUFFERS)
+ av_log(s, AV_LOG_DEBUG, "default_get_buffer called on pic %p\n", pic);
+
+ return 0;
+fail:
+ av_frame_unref(pic);
+ return AVERROR(ENOMEM);
+}
+
+int avcodec_default_get_buffer2(AVCodecContext *avctx, AVFrame *frame, int flags)
+{
+ int ret;
+
+ if (avctx->hw_frames_ctx) {
+ ret = av_hwframe_get_buffer(avctx->hw_frames_ctx, frame, 0);
+ frame->width = avctx->coded_width;
+ frame->height = avctx->coded_height;
+ return ret;
+ }
+
+ if ((ret = update_frame_pool(avctx, frame)) < 0)
+ return ret;
+
+ switch (avctx->codec_type) {
+ case AVMEDIA_TYPE_VIDEO:
+ return video_get_buffer(avctx, frame);
+ case AVMEDIA_TYPE_AUDIO:
+ return audio_get_buffer(avctx, frame);
+ default:
+ return -1;
+ }
+}
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 4/8] lavc/snow: only allocate mconly_picture for decoding
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 3/8] lavc: move default get_buffer2() to its own file Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-03-24 23:07 ` Michael Niedermayer
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 5/8] lavc/encode: add an encoder-specific get_buffer() variant Anton Khirnov
` (6 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
It is not used in the encoder.
---
libavcodec/snow.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/libavcodec/snow.c b/libavcodec/snow.c
index 0a500695ce..1224b95491 100644
--- a/libavcodec/snow.c
+++ b/libavcodec/snow.c
@@ -513,20 +513,23 @@ int ff_snow_common_init_after_header(AVCodecContext *avctx) {
int ret, emu_buf_size;
if(!s->scratchbuf) {
- if ((ret = ff_get_buffer(s->avctx, s->mconly_picture,
- AV_GET_BUFFER_FLAG_REF)) < 0)
- return ret;
+ if (av_codec_is_decoder(avctx->codec)) {
+ if ((ret = ff_get_buffer(s->avctx, s->mconly_picture,
+ AV_GET_BUFFER_FLAG_REF)) < 0)
+ return ret;
+
+ if (s->mconly_picture->format != avctx->pix_fmt) {
+ av_log(avctx, AV_LOG_ERROR, "pixel format changed\n");
+ return AVERROR_INVALIDDATA;
+ }
+ }
+
emu_buf_size = FFMAX(s->mconly_picture->linesize[0], 2*avctx->width+256) * (2 * MB_SIZE + HTAPS_MAX - 1);
if (!FF_ALLOCZ_TYPED_ARRAY(s->scratchbuf, FFMAX(s->mconly_picture->linesize[0], 2*avctx->width+256) * 7 * MB_SIZE) ||
!FF_ALLOCZ_TYPED_ARRAY(s->emu_edge_buffer, emu_buf_size))
return AVERROR(ENOMEM);
}
- if(s->mconly_picture->format != avctx->pix_fmt) {
- av_log(avctx, AV_LOG_ERROR, "pixel format changed\n");
- return AVERROR_INVALIDDATA;
- }
-
for(plane_index=0; plane_index < s->nb_planes; plane_index++){
int w= s->avctx->width;
int h= s->avctx->height;
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 5/8] lavc/encode: add an encoder-specific get_buffer() variant
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (2 preceding siblings ...)
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 4/8] lavc/snow: only allocate mconly_picture for decoding Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-03-23 16:26 ` James Almer
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 6/8] lavc/avcodec: only allocate decoding packets for decoders Anton Khirnov
` (5 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
Several encoders (roqvideo, svq1, snow, and the mpegvideo family)
currently call ff_get_buffer(). However this function is written
assuming it is called by a decoder. Though nothing has been obviously
broken by this until now, this may change in the future.
To avoid potential future issues, introduce a simple encode-specific
wrapper around avcodec_default_get_buffer2() and enforce its use in
encoders.
---
libavcodec/decode.c | 2 ++
libavcodec/encode.c | 34 ++++++++++++++++++++++++++++++++++
libavcodec/encode.h | 5 +++++
libavcodec/mpegpicture.c | 16 +++++++++-------
libavcodec/roqvideoenc.c | 4 ++--
libavcodec/snow.c | 8 ++++++--
libavcodec/svq1enc.c | 4 ++--
7 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index fffad5f700..3733e6d4b8 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1405,6 +1405,8 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
int override_dimensions = 1;
int ret;
+ av_assert0(av_codec_is_decoder(avctx->codec));
+
if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
if ((unsigned)avctx->width > INT_MAX - STRIDE_ALIGN ||
(ret = av_image_check_size2(FFALIGN(avctx->width, STRIDE_ALIGN), avctx->height, avctx->max_pixels, AV_PIX_FMT_NONE, 0, avctx)) < 0 || avctx->pix_fmt<0) {
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 837ffaa40d..ec8d06d068 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -572,3 +572,37 @@ FF_ENABLE_DEPRECATION_WARNINGS
return 0;
}
+
+int ff_encode_alloc_frame(AVCodecContext *avctx, AVFrame *frame)
+{
+ int ret;
+
+ switch (avctx->codec->type) {
+ case AVMEDIA_TYPE_VIDEO:
+ frame->format = avctx->pix_fmt;
+ if (frame->width <= 0 || frame->height <= 0) {
+ frame->width = FFMAX(avctx->width, avctx->coded_width);
+ frame->height = FFMAX(avctx->height, avctx->coded_height);
+ }
+
+ break;
+ case AVMEDIA_TYPE_AUDIO:
+ frame->sample_rate = avctx->sample_rate;
+ frame->format = avctx->sample_fmt;
+ if (!frame->ch_layout.nb_channels) {
+ int ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
+ if (ret < 0)
+ return ret;
+ }
+ break;
+ }
+
+ ret = avcodec_default_get_buffer2(avctx, frame, 0);
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
+ av_frame_unref(frame);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/libavcodec/encode.h b/libavcodec/encode.h
index 97b3acf9df..b2536bf0f3 100644
--- a/libavcodec/encode.h
+++ b/libavcodec/encode.h
@@ -44,6 +44,11 @@ int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame);
*/
int ff_get_encode_buffer(AVCodecContext *avctx, AVPacket *avpkt, int64_t size, int flags);
+/**
+ * Allocate buffers for a frame. Encoder equivalent to ff_get_buffer().
+ */
+int ff_encode_alloc_frame(AVCodecContext *avctx, AVFrame *frame);
+
/**
* Check AVPacket size and allocate data.
*
diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index 681ccc2b82..aaa1df0bd8 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -26,6 +26,7 @@
#include "libavutil/imgutils.h"
#include "avcodec.h"
+#include "encode.h"
#include "motion_est.h"
#include "mpegpicture.h"
#include "mpegutils.h"
@@ -123,14 +124,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx, Picture *pic,
int r, ret;
pic->tf.f = pic->f;
- if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
- avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
- avctx->codec_id != AV_CODEC_ID_MSS2) {
- if (edges_needed) {
- pic->f->width = avctx->width + 2 * EDGE_WIDTH;
- pic->f->height = avctx->height + 2 * EDGE_WIDTH;
- }
+ if (edges_needed) {
+ pic->f->width = avctx->width + 2 * EDGE_WIDTH;
+ pic->f->height = avctx->height + 2 * EDGE_WIDTH;
+
+ r = ff_encode_alloc_frame(avctx, pic->f);
+ } else if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
+ avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
+ avctx->codec_id != AV_CODEC_ID_MSS2) {
r = ff_thread_get_ext_buffer(avctx, &pic->tf,
pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
} else {
diff --git a/libavcodec/roqvideoenc.c b/libavcodec/roqvideoenc.c
index ef7861088d..95db514140 100644
--- a/libavcodec/roqvideoenc.c
+++ b/libavcodec/roqvideoenc.c
@@ -1081,8 +1081,8 @@ static int roq_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
if (enc->first_frame) {
/* Alloc memory for the reconstruction data (we must know the stride
for that) */
- if ((ret = ff_get_buffer(avctx, roq->current_frame, 0)) < 0 ||
- (ret = ff_get_buffer(avctx, roq->last_frame, 0)) < 0)
+ if ((ret = ff_encode_alloc_frame(avctx, roq->current_frame)) < 0 ||
+ (ret = ff_encode_alloc_frame(avctx, roq->last_frame )) < 0)
return ret;
/* Before the first video frame, write a "video info" chunk */
diff --git a/libavcodec/snow.c b/libavcodec/snow.c
index 1224b95491..bc0d9a8983 100644
--- a/libavcodec/snow.c
+++ b/libavcodec/snow.c
@@ -23,6 +23,7 @@
#include "libavutil/opt.h"
#include "libavutil/thread.h"
#include "avcodec.h"
+#include "encode.h"
#include "me_cmp.h"
#include "snow_dwt.h"
#include "internal.h"
@@ -76,8 +77,11 @@ int ff_snow_get_buffer(SnowContext *s, AVFrame *frame)
if (edges_needed) {
frame->width += 2 * EDGE_WIDTH;
frame->height += 2 * EDGE_WIDTH;
- }
- if ((ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
+
+ ret = ff_encode_alloc_frame(s->avctx, frame);
+ } else
+ ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF);
+ if (ret < 0)
return ret;
if (edges_needed) {
for (i = 0; frame->data[i]; i++) {
diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 706bc2e42e..cb0d0308fc 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -595,12 +595,12 @@ static int svq1_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
}
if (!s->current_picture->data[0]) {
- if ((ret = ff_get_buffer(avctx, s->current_picture, 0)) < 0) {
+ if ((ret = ff_encode_alloc_frame(avctx, s->current_picture)) < 0) {
return ret;
}
}
if (!s->last_picture->data[0]) {
- ret = ff_get_buffer(avctx, s->last_picture, 0);
+ ret = ff_encode_alloc_frame(avctx, s->last_picture);
if (ret < 0)
return ret;
}
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 6/8] lavc/avcodec: only allocate decoding packets for decoders
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (3 preceding siblings ...)
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 5/8] lavc/encode: add an encoder-specific get_buffer() variant Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-04-13 14:51 ` Andreas Rheinhardt
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 7/8] lavc/pthread_frame: do not copy AVCodecInternal contents Anton Khirnov
` (4 subsequent siblings)
9 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
---
libavcodec/avcodec.c | 7 +------
libavcodec/decode.c | 8 ++++++++
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index c7daa385e7..5fd988a41c 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -180,12 +180,7 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
avci->buffer_frame = av_frame_alloc();
avci->buffer_pkt = av_packet_alloc();
- avci->in_pkt = av_packet_alloc();
- avci->last_pkt_props = av_packet_alloc();
- avci->pkt_props = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
- AV_FIFO_FLAG_AUTO_GROW);
- if (!avci->buffer_frame || !avci->buffer_pkt ||
- !avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props) {
+ if (!avci->buffer_frame || !avci->buffer_pkt) {
ret = AVERROR(ENOMEM);
goto free_and_end;
}
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 3733e6d4b8..bb3857afd9 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1527,6 +1527,7 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
int ff_decode_preinit(AVCodecContext *avctx)
{
+ AVCodecInternal *avci = avctx->internal;
int ret = 0;
/* if the decoder init function was already called previously,
@@ -1605,6 +1606,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
}
+ avci->in_pkt = av_packet_alloc();
+ avci->last_pkt_props = av_packet_alloc();
+ avci->pkt_props = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
+ AV_FIFO_FLAG_AUTO_GROW);
+ if (!avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props)
+ return AVERROR(ENOMEM);
+
ret = decode_bsfs_init(avctx);
if (ret < 0)
return ret;
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 7/8] lavc/pthread_frame: do not copy AVCodecInternal contents
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (4 preceding siblings ...)
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 6/8] lavc/avcodec: only allocate decoding packets for decoders Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 8/8] lavc: drop a confusing message about "thread emulation" Anton Khirnov
` (3 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
None of its fields have meaningful values at that point that would need
to be copied to frame thread workers.
---
libavcodec/pthread_frame.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index aa971bd74d..4ae1c02f62 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -791,7 +791,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
p->parent = fctx;
p->avctx = copy;
- copy->internal = av_memdup(avctx->internal, sizeof(*avctx->internal));
+ copy->internal = av_mallocz(sizeof(*copy->internal));
if (!copy->internal)
return AVERROR(ENOMEM);
copy->internal->thread_ctx = p;
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH 8/8] lavc: drop a confusing message about "thread emulation"
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (5 preceding siblings ...)
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 7/8] lavc/pthread_frame: do not copy AVCodecInternal contents Anton Khirnov
@ 2022-03-23 15:57 ` Anton Khirnov
2022-04-13 10:23 ` [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (2 subsequent siblings)
9 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-03-23 15:57 UTC (permalink / raw)
To: ffmpeg-devel
There is no such thing.
---
libavcodec/avcodec.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 5fd988a41c..dec09476cc 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -309,9 +309,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
if (ret < 0)
goto free_and_end;
- if (!HAVE_THREADS)
- av_log(avctx, AV_LOG_WARNING, "Warning: not compiled with thread support, using thread emulation\n");
-
if (CONFIG_FRAME_THREAD_ENCODER && av_codec_is_encoder(avctx->codec)) {
ret = ff_frame_thread_encoder_init(avctx);
if (ret < 0)
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 5/8] lavc/encode: add an encoder-specific get_buffer() variant
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 5/8] lavc/encode: add an encoder-specific get_buffer() variant Anton Khirnov
@ 2022-03-23 16:26 ` James Almer
2022-04-11 9:05 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
0 siblings, 1 reply; 33+ messages in thread
From: James Almer @ 2022-03-23 16:26 UTC (permalink / raw)
To: ffmpeg-devel
On 3/23/2022 12:57 PM, Anton Khirnov wrote:
> Several encoders (roqvideo, svq1, snow, and the mpegvideo family)
> currently call ff_get_buffer(). However this function is written
> assuming it is called by a decoder. Though nothing has been obviously
> broken by this until now, this may change in the future.
>
> To avoid potential future issues, introduce a simple encode-specific
> wrapper around avcodec_default_get_buffer2() and enforce its use in
> encoders.
> ---
> libavcodec/decode.c | 2 ++
> libavcodec/encode.c | 34 ++++++++++++++++++++++++++++++++++
> libavcodec/encode.h | 5 +++++
> libavcodec/mpegpicture.c | 16 +++++++++-------
> libavcodec/roqvideoenc.c | 4 ++--
> libavcodec/snow.c | 8 ++++++--
> libavcodec/svq1enc.c | 4 ++--
> 7 files changed, 60 insertions(+), 13 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index fffad5f700..3733e6d4b8 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1405,6 +1405,8 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
> int override_dimensions = 1;
> int ret;
>
> + av_assert0(av_codec_is_decoder(avctx->codec));
> +
> if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
> if ((unsigned)avctx->width > INT_MAX - STRIDE_ALIGN ||
> (ret = av_image_check_size2(FFALIGN(avctx->width, STRIDE_ALIGN), avctx->height, avctx->max_pixels, AV_PIX_FMT_NONE, 0, avctx)) < 0 || avctx->pix_fmt<0) {
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 837ffaa40d..ec8d06d068 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -572,3 +572,37 @@ FF_ENABLE_DEPRECATION_WARNINGS
>
> return 0;
> }
> +
> +int ff_encode_alloc_frame(AVCodecContext *avctx, AVFrame *frame)
> +{
> + int ret;
> +
> + switch (avctx->codec->type) {
> + case AVMEDIA_TYPE_VIDEO:
> + frame->format = avctx->pix_fmt;
> + if (frame->width <= 0 || frame->height <= 0) {
> + frame->width = FFMAX(avctx->width, avctx->coded_width);
> + frame->height = FFMAX(avctx->height, avctx->coded_height);
> + }
> +
> + break;
> + case AVMEDIA_TYPE_AUDIO:
> + frame->sample_rate = avctx->sample_rate;
> + frame->format = avctx->sample_fmt;
> + if (!frame->ch_layout.nb_channels) {
> + int ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
Unnecessarily shadowing ret.
> + if (ret < 0)
> + return ret;
> + }
> + break;
> + }
> +
> + ret = avcodec_default_get_buffer2(avctx, frame, 0);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> + av_frame_unref(frame);
> + return ret;
> + }
> +
> + return 0;
> +}
> diff --git a/libavcodec/encode.h b/libavcodec/encode.h
> index 97b3acf9df..b2536bf0f3 100644
> --- a/libavcodec/encode.h
> +++ b/libavcodec/encode.h
> @@ -44,6 +44,11 @@ int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame);
> */
> int ff_get_encode_buffer(AVCodecContext *avctx, AVPacket *avpkt, int64_t size, int flags);
>
> +/**
> + * Allocate buffers for a frame. Encoder equivalent to ff_get_buffer().
> + */
> +int ff_encode_alloc_frame(AVCodecContext *avctx, AVFrame *frame);
> +
> /**
> * Check AVPacket size and allocate data.
> *
> diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> index 681ccc2b82..aaa1df0bd8 100644
> --- a/libavcodec/mpegpicture.c
> +++ b/libavcodec/mpegpicture.c
> @@ -26,6 +26,7 @@
> #include "libavutil/imgutils.h"
>
> #include "avcodec.h"
> +#include "encode.h"
> #include "motion_est.h"
> #include "mpegpicture.h"
> #include "mpegutils.h"
> @@ -123,14 +124,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx, Picture *pic,
> int r, ret;
>
> pic->tf.f = pic->f;
> - if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
> - avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
> - avctx->codec_id != AV_CODEC_ID_MSS2) {
> - if (edges_needed) {
> - pic->f->width = avctx->width + 2 * EDGE_WIDTH;
> - pic->f->height = avctx->height + 2 * EDGE_WIDTH;
> - }
>
> + if (edges_needed) {
> + pic->f->width = avctx->width + 2 * EDGE_WIDTH;
> + pic->f->height = avctx->height + 2 * EDGE_WIDTH;
> +
> + r = ff_encode_alloc_frame(avctx, pic->f);
> + } else if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
> + avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
> + avctx->codec_id != AV_CODEC_ID_MSS2) {
> r = ff_thread_get_ext_buffer(avctx, &pic->tf,
> pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
> } else {
> diff --git a/libavcodec/roqvideoenc.c b/libavcodec/roqvideoenc.c
> index ef7861088d..95db514140 100644
> --- a/libavcodec/roqvideoenc.c
> +++ b/libavcodec/roqvideoenc.c
> @@ -1081,8 +1081,8 @@ static int roq_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> if (enc->first_frame) {
> /* Alloc memory for the reconstruction data (we must know the stride
> for that) */
> - if ((ret = ff_get_buffer(avctx, roq->current_frame, 0)) < 0 ||
> - (ret = ff_get_buffer(avctx, roq->last_frame, 0)) < 0)
> + if ((ret = ff_encode_alloc_frame(avctx, roq->current_frame)) < 0 ||
> + (ret = ff_encode_alloc_frame(avctx, roq->last_frame )) < 0)
> return ret;
>
> /* Before the first video frame, write a "video info" chunk */
> diff --git a/libavcodec/snow.c b/libavcodec/snow.c
> index 1224b95491..bc0d9a8983 100644
> --- a/libavcodec/snow.c
> +++ b/libavcodec/snow.c
> @@ -23,6 +23,7 @@
> #include "libavutil/opt.h"
> #include "libavutil/thread.h"
> #include "avcodec.h"
> +#include "encode.h"
> #include "me_cmp.h"
> #include "snow_dwt.h"
> #include "internal.h"
> @@ -76,8 +77,11 @@ int ff_snow_get_buffer(SnowContext *s, AVFrame *frame)
> if (edges_needed) {
> frame->width += 2 * EDGE_WIDTH;
> frame->height += 2 * EDGE_WIDTH;
> - }
> - if ((ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
> +
> + ret = ff_encode_alloc_frame(s->avctx, frame);
> + } else
> + ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF);
> + if (ret < 0)
> return ret;
> if (edges_needed) {
> for (i = 0; frame->data[i]; i++) {
> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
> index 706bc2e42e..cb0d0308fc 100644
> --- a/libavcodec/svq1enc.c
> +++ b/libavcodec/svq1enc.c
> @@ -595,12 +595,12 @@ static int svq1_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> }
>
> if (!s->current_picture->data[0]) {
> - if ((ret = ff_get_buffer(avctx, s->current_picture, 0)) < 0) {
> + if ((ret = ff_encode_alloc_frame(avctx, s->current_picture)) < 0) {
> return ret;
> }
> }
> if (!s->last_picture->data[0]) {
> - ret = ff_get_buffer(avctx, s->last_picture, 0);
> + ret = ff_encode_alloc_frame(avctx, s->last_picture);
> if (ret < 0)
> return ret;
> }
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders Anton Khirnov
@ 2022-03-23 16:29 ` James Almer
2022-04-11 8:39 ` [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext Anton Khirnov
0 siblings, 1 reply; 33+ messages in thread
From: James Almer @ 2022-03-23 16:29 UTC (permalink / raw)
To: ffmpeg-devel
On 3/23/2022 12:57 PM, Anton Khirnov wrote:
> And only when needed, i.e. for encoders using the simple API.
> ---
> libavcodec/avcodec.c | 4 +---
> libavcodec/encode.c | 7 +++++++
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index dbaa9f78a2..c7daa385e7 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -180,14 +180,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>
> avci->buffer_frame = av_frame_alloc();
> avci->buffer_pkt = av_packet_alloc();
> - avci->es.in_frame = av_frame_alloc();
> avci->in_pkt = av_packet_alloc();
> avci->last_pkt_props = av_packet_alloc();
> avci->pkt_props = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
> AV_FIFO_FLAG_AUTO_GROW);
> if (!avci->buffer_frame || !avci->buffer_pkt ||
> - !avci->es.in_frame || !avci->in_pkt ||
> - !avci->last_pkt_props || !avci->pkt_props) {
> + !avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props) {
> ret = AVERROR(ENOMEM);
> goto free_and_end;
> }
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 70bd8da81f..837ffaa40d 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -412,6 +412,7 @@ int attribute_align_arg avcodec_receive_packet(AVCodecContext *avctx, AVPacket *
>
> int ff_encode_preinit(AVCodecContext *avctx)
> {
> + AVCodecInternal *avci = avctx->internal;
> int i;
>
> if (avctx->time_base.num <= 0 || avctx->time_base.den <= 0) {
> @@ -563,5 +564,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
> if (avctx->codec_descriptor->props & AV_CODEC_PROP_INTRA_ONLY)
> avctx->internal->intra_only_flag = AV_PKT_FLAG_KEY;
>
> + if (ffcodec(avctx->codec)->encode2) {
> + avci->es.in_frame = av_frame_alloc();
Could use the chance to remove the EncodeSimpleContext struct and just
have in_frame in AVCodecInternal.
DecodeSimpleContext was removed the same way some time ago when it
became a struct for a single field.
> + if (!avci->es.in_frame)
> + return AVERROR(ENOMEM);
> + }
> +
> return 0;
> }
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] lavc/snow: only allocate mconly_picture for decoding
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 4/8] lavc/snow: only allocate mconly_picture for decoding Anton Khirnov
@ 2022-03-24 23:07 ` Michael Niedermayer
2022-04-11 8:49 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
0 siblings, 1 reply; 33+ messages in thread
From: Michael Niedermayer @ 2022-03-24 23:07 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1585 bytes --]
On Wed, Mar 23, 2022 at 04:57:16PM +0100, Anton Khirnov wrote:
> It is not used in the encoder.
> ---
> libavcodec/snow.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
this is segfaulting with some fuzzed file
==30657== Invalid read of size 8
==30657== at 0x1157660: ??? (libavcodec/x86/videodsp.asm:340)
==30657== by 0xE18591: emulated_edge_mc_avx2 (videodsp_init.c:268)
==30657== by 0x10D8973: ff_snow_pred_block (snow.c:370)
==30657== by 0xC0E7DA: add_yblock (snow.h:345)
==30657== by 0xC0FE62: predict_slice_buffered (snowdec.c:78)
==30657== by 0xC12CAD: decode_frame (snowdec.c:602)
==30657== by 0x8BF99F: decode_simple_internal (decode.c:306)
==30657== by 0x8C0650: decode_simple_receive_frame (decode.c:514)
==30657== by 0x8C0756: decode_receive_frame_internal (decode.c:535)
==30657== by 0x8C0A17: avcodec_send_packet (decode.c:603)
==30657== by 0x25D6A5: decode (ffmpeg.c:2275)
==30657== by 0x25DE2C: decode_video (ffmpeg.c:2400)
==30657== by 0x25EEBC: process_input_packet (ffmpeg.c:2640)
==30657== by 0x266A1E: process_input (ffmpeg.c:4494)
==30657== by 0x266F12: transcode_step (ffmpeg.c:4634)
==30657== by 0x26707A: transcode (ffmpeg.c:4688)
==30657== by 0x267AEF: main (ffmpeg.c:4904)
==30657== Address 0x0 is not stack'd, malloc'd or (recently) free'd
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Elect your leaders based on what they did after the last election, not
based on what they say before an election.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext
2022-03-23 16:29 ` James Almer
@ 2022-04-11 8:39 ` Anton Khirnov
2022-04-11 9:16 ` Paul B Mahol
2022-04-11 16:32 ` James Almer
0 siblings, 2 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-04-11 8:39 UTC (permalink / raw)
To: ffmpeg-devel
It has only a single member.
---
libavcodec/avcodec.c | 4 ++--
libavcodec/encode.c | 7 +++----
libavcodec/internal.h | 12 +++++++-----
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index c7daa385e7..e0f38ac42a 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -432,7 +432,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
av_packet_unref(avci->last_pkt_props);
- av_frame_unref(avci->es.in_frame);
+ av_frame_unref(avci->in_frame);
av_packet_unref(avci->in_pkt);
if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
@@ -498,7 +498,7 @@ av_cold int avcodec_close(AVCodecContext *avctx)
av_packet_free(&avci->last_pkt_props);
av_packet_free(&avci->in_pkt);
- av_frame_free(&avci->es.in_frame);
+ av_frame_free(&avci->in_frame);
av_buffer_unref(&avci->pool);
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 837ffaa40d..8b0d4443cd 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -175,8 +175,7 @@ int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame)
static int encode_simple_internal(AVCodecContext *avctx, AVPacket *avpkt)
{
AVCodecInternal *avci = avctx->internal;
- EncodeSimpleContext *es = &avci->es;
- AVFrame *frame = es->in_frame;
+ AVFrame *frame = avci->in_frame;
const FFCodec *const codec = ffcodec(avctx->codec);
int got_packet;
int ret;
@@ -565,8 +564,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
avctx->internal->intra_only_flag = AV_PKT_FLAG_KEY;
if (ffcodec(avctx->codec)->encode2) {
- avci->es.in_frame = av_frame_alloc();
- if (!avci->es.in_frame)
+ avci->in_frame = av_frame_alloc();
+ if (!avci->in_frame)
return AVERROR(ENOMEM);
}
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index f9d08fcb60..2fa56d3a59 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -47,10 +47,6 @@
# define STRIDE_ALIGN 8
#endif
-typedef struct EncodeSimpleContext {
- AVFrame *in_frame;
-} EncodeSimpleContext;
-
typedef struct AVCodecInternal {
/**
* When using frame-threaded decoding, this field is set for the first
@@ -101,7 +97,13 @@ typedef struct AVCodecInternal {
void *frame_thread_encoder;
- EncodeSimpleContext es;
+ /**
+ * The input frame is stored here for encoders implementing the simple
+ * encode API.
+ *
+ * Not allocated in other cases.
+ */
+ AVFrame *in_frame;
/**
* If this is set, then FFCodec->close (if existing) needs to be called
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH] lavc/snow: only allocate mconly_picture for decoding
2022-03-24 23:07 ` Michael Niedermayer
@ 2022-04-11 8:49 ` Anton Khirnov
2022-04-11 19:28 ` Michael Niedermayer
0 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-04-11 8:49 UTC (permalink / raw)
To: ffmpeg-devel
It is not used in the encoder.
---
Does this fix the crash?
---
libavcodec/snow.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libavcodec/snow.c b/libavcodec/snow.c
index 0a500695ce..97b0448dbf 100644
--- a/libavcodec/snow.c
+++ b/libavcodec/snow.c
@@ -513,16 +513,20 @@ int ff_snow_common_init_after_header(AVCodecContext *avctx) {
int ret, emu_buf_size;
if(!s->scratchbuf) {
- if ((ret = ff_get_buffer(s->avctx, s->mconly_picture,
- AV_GET_BUFFER_FLAG_REF)) < 0)
- return ret;
+ if (av_codec_is_decoder(avctx->codec)) {
+ if ((ret = ff_get_buffer(s->avctx, s->mconly_picture,
+ AV_GET_BUFFER_FLAG_REF)) < 0)
+ return ret;
+ }
+
emu_buf_size = FFMAX(s->mconly_picture->linesize[0], 2*avctx->width+256) * (2 * MB_SIZE + HTAPS_MAX - 1);
if (!FF_ALLOCZ_TYPED_ARRAY(s->scratchbuf, FFMAX(s->mconly_picture->linesize[0], 2*avctx->width+256) * 7 * MB_SIZE) ||
!FF_ALLOCZ_TYPED_ARRAY(s->emu_edge_buffer, emu_buf_size))
return AVERROR(ENOMEM);
}
- if(s->mconly_picture->format != avctx->pix_fmt) {
+ if (av_codec_is_decoder(avctx->codec) &&
+ s->mconly_picture->format != avctx->pix_fmt) {
av_log(avctx, AV_LOG_ERROR, "pixel format changed\n");
return AVERROR_INVALIDDATA;
}
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* [FFmpeg-devel] [PATCH] lavc/encode: add an encoder-specific get_buffer() variant
2022-03-23 16:26 ` James Almer
@ 2022-04-11 9:05 ` Anton Khirnov
0 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-04-11 9:05 UTC (permalink / raw)
To: ffmpeg-devel
Several encoders (roqvideo, svq1, snow, and the mpegvideo family)
currently call ff_get_buffer(). However this function is written
assuming it is called by a decoder. Though nothing has been obviously
broken by this until now, this may change in the future.
To avoid potential future issues, introduce a simple encode-specific
wrapper around avcodec_default_get_buffer2() and enforce its use in
encoders.
---
libavcodec/decode.c | 2 ++
libavcodec/encode.c | 34 ++++++++++++++++++++++++++++++++++
libavcodec/encode.h | 5 +++++
libavcodec/mpegpicture.c | 16 +++++++++-------
libavcodec/roqvideoenc.c | 4 ++--
libavcodec/snow.c | 8 ++++++--
libavcodec/svq1enc.c | 4 ++--
7 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index fffad5f700..3733e6d4b8 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1405,6 +1405,8 @@ int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
int override_dimensions = 1;
int ret;
+ av_assert0(av_codec_is_decoder(avctx->codec));
+
if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
if ((unsigned)avctx->width > INT_MAX - STRIDE_ALIGN ||
(ret = av_image_check_size2(FFALIGN(avctx->width, STRIDE_ALIGN), avctx->height, avctx->max_pixels, AV_PIX_FMT_NONE, 0, avctx)) < 0 || avctx->pix_fmt<0) {
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 8b0d4443cd..adeb28f0d3 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -571,3 +571,37 @@ FF_ENABLE_DEPRECATION_WARNINGS
return 0;
}
+
+int ff_encode_alloc_frame(AVCodecContext *avctx, AVFrame *frame)
+{
+ int ret;
+
+ switch (avctx->codec->type) {
+ case AVMEDIA_TYPE_VIDEO:
+ frame->format = avctx->pix_fmt;
+ if (frame->width <= 0 || frame->height <= 0) {
+ frame->width = FFMAX(avctx->width, avctx->coded_width);
+ frame->height = FFMAX(avctx->height, avctx->coded_height);
+ }
+
+ break;
+ case AVMEDIA_TYPE_AUDIO:
+ frame->sample_rate = avctx->sample_rate;
+ frame->format = avctx->sample_fmt;
+ if (!frame->ch_layout.nb_channels) {
+ ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
+ if (ret < 0)
+ return ret;
+ }
+ break;
+ }
+
+ ret = avcodec_default_get_buffer2(avctx, frame, 0);
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
+ av_frame_unref(frame);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/libavcodec/encode.h b/libavcodec/encode.h
index 97b3acf9df..b2536bf0f3 100644
--- a/libavcodec/encode.h
+++ b/libavcodec/encode.h
@@ -44,6 +44,11 @@ int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame);
*/
int ff_get_encode_buffer(AVCodecContext *avctx, AVPacket *avpkt, int64_t size, int flags);
+/**
+ * Allocate buffers for a frame. Encoder equivalent to ff_get_buffer().
+ */
+int ff_encode_alloc_frame(AVCodecContext *avctx, AVFrame *frame);
+
/**
* Check AVPacket size and allocate data.
*
diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index 681ccc2b82..aaa1df0bd8 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -26,6 +26,7 @@
#include "libavutil/imgutils.h"
#include "avcodec.h"
+#include "encode.h"
#include "motion_est.h"
#include "mpegpicture.h"
#include "mpegutils.h"
@@ -123,14 +124,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx, Picture *pic,
int r, ret;
pic->tf.f = pic->f;
- if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
- avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
- avctx->codec_id != AV_CODEC_ID_MSS2) {
- if (edges_needed) {
- pic->f->width = avctx->width + 2 * EDGE_WIDTH;
- pic->f->height = avctx->height + 2 * EDGE_WIDTH;
- }
+ if (edges_needed) {
+ pic->f->width = avctx->width + 2 * EDGE_WIDTH;
+ pic->f->height = avctx->height + 2 * EDGE_WIDTH;
+
+ r = ff_encode_alloc_frame(avctx, pic->f);
+ } else if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
+ avctx->codec_id != AV_CODEC_ID_VC1IMAGE &&
+ avctx->codec_id != AV_CODEC_ID_MSS2) {
r = ff_thread_get_ext_buffer(avctx, &pic->tf,
pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
} else {
diff --git a/libavcodec/roqvideoenc.c b/libavcodec/roqvideoenc.c
index ef7861088d..95db514140 100644
--- a/libavcodec/roqvideoenc.c
+++ b/libavcodec/roqvideoenc.c
@@ -1081,8 +1081,8 @@ static int roq_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
if (enc->first_frame) {
/* Alloc memory for the reconstruction data (we must know the stride
for that) */
- if ((ret = ff_get_buffer(avctx, roq->current_frame, 0)) < 0 ||
- (ret = ff_get_buffer(avctx, roq->last_frame, 0)) < 0)
+ if ((ret = ff_encode_alloc_frame(avctx, roq->current_frame)) < 0 ||
+ (ret = ff_encode_alloc_frame(avctx, roq->last_frame )) < 0)
return ret;
/* Before the first video frame, write a "video info" chunk */
diff --git a/libavcodec/snow.c b/libavcodec/snow.c
index 97b0448dbf..293a0eb7d9 100644
--- a/libavcodec/snow.c
+++ b/libavcodec/snow.c
@@ -23,6 +23,7 @@
#include "libavutil/opt.h"
#include "libavutil/thread.h"
#include "avcodec.h"
+#include "encode.h"
#include "me_cmp.h"
#include "snow_dwt.h"
#include "internal.h"
@@ -76,8 +77,11 @@ int ff_snow_get_buffer(SnowContext *s, AVFrame *frame)
if (edges_needed) {
frame->width += 2 * EDGE_WIDTH;
frame->height += 2 * EDGE_WIDTH;
- }
- if ((ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
+
+ ret = ff_encode_alloc_frame(s->avctx, frame);
+ } else
+ ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF);
+ if (ret < 0)
return ret;
if (edges_needed) {
for (i = 0; frame->data[i]; i++) {
diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index d934ef4646..999f974bcf 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -595,12 +595,12 @@ static int svq1_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
}
if (!s->current_picture->data[0]) {
- if ((ret = ff_get_buffer(avctx, s->current_picture, 0)) < 0) {
+ if ((ret = ff_encode_alloc_frame(avctx, s->current_picture)) < 0) {
return ret;
}
}
if (!s->last_picture->data[0]) {
- ret = ff_get_buffer(avctx, s->last_picture, 0);
+ ret = ff_encode_alloc_frame(avctx, s->last_picture);
if (ret < 0)
return ret;
}
--
2.34.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext
2022-04-11 8:39 ` [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext Anton Khirnov
@ 2022-04-11 9:16 ` Paul B Mahol
2022-04-11 16:32 ` James Almer
1 sibling, 0 replies; 33+ messages in thread
From: Paul B Mahol @ 2022-04-11 9:16 UTC (permalink / raw)
To: FFmpeg development discussions and patches
lgtm
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext
2022-04-11 8:39 ` [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext Anton Khirnov
2022-04-11 9:16 ` Paul B Mahol
@ 2022-04-11 16:32 ` James Almer
1 sibling, 0 replies; 33+ messages in thread
From: James Almer @ 2022-04-11 16:32 UTC (permalink / raw)
To: ffmpeg-devel
On 4/11/2022 5:39 AM, Anton Khirnov wrote:
> It has only a single member.
> ---
> libavcodec/avcodec.c | 4 ++--
> libavcodec/encode.c | 7 +++----
> libavcodec/internal.h | 12 +++++++-----
> 3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index c7daa385e7..e0f38ac42a 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -432,7 +432,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
> while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
> av_packet_unref(avci->last_pkt_props);
>
> - av_frame_unref(avci->es.in_frame);
> + av_frame_unref(avci->in_frame);
> av_packet_unref(avci->in_pkt);
>
> if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
> @@ -498,7 +498,7 @@ av_cold int avcodec_close(AVCodecContext *avctx)
> av_packet_free(&avci->last_pkt_props);
>
> av_packet_free(&avci->in_pkt);
> - av_frame_free(&avci->es.in_frame);
> + av_frame_free(&avci->in_frame);
>
> av_buffer_unref(&avci->pool);
>
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 837ffaa40d..8b0d4443cd 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -175,8 +175,7 @@ int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame)
> static int encode_simple_internal(AVCodecContext *avctx, AVPacket *avpkt)
> {
> AVCodecInternal *avci = avctx->internal;
> - EncodeSimpleContext *es = &avci->es;
> - AVFrame *frame = es->in_frame;
> + AVFrame *frame = avci->in_frame;
> const FFCodec *const codec = ffcodec(avctx->codec);
> int got_packet;
> int ret;
> @@ -565,8 +564,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
> avctx->internal->intra_only_flag = AV_PKT_FLAG_KEY;
>
> if (ffcodec(avctx->codec)->encode2) {
> - avci->es.in_frame = av_frame_alloc();
> - if (!avci->es.in_frame)
> + avci->in_frame = av_frame_alloc();
> + if (!avci->in_frame)
> return AVERROR(ENOMEM);
> }
>
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index f9d08fcb60..2fa56d3a59 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -47,10 +47,6 @@
> # define STRIDE_ALIGN 8
> #endif
>
> -typedef struct EncodeSimpleContext {
> - AVFrame *in_frame;
> -} EncodeSimpleContext;
> -
> typedef struct AVCodecInternal {
> /**
> * When using frame-threaded decoding, this field is set for the first
> @@ -101,7 +97,13 @@ typedef struct AVCodecInternal {
>
> void *frame_thread_encoder;
>
> - EncodeSimpleContext es;
> + /**
> + * The input frame is stored here for encoders implementing the simple
> + * encode API.
> + *
> + * Not allocated in other cases.
> + */
> + AVFrame *in_frame;
>
> /**
> * If this is set, then FFCodec->close (if existing) needs to be called
LGTM.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/snow: only allocate mconly_picture for decoding
2022-04-11 8:49 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
@ 2022-04-11 19:28 ` Michael Niedermayer
2022-04-13 10:21 ` Anton Khirnov
0 siblings, 1 reply; 33+ messages in thread
From: Michael Niedermayer @ 2022-04-11 19:28 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 611 bytes --]
On Mon, Apr 11, 2022 at 10:49:34AM +0200, Anton Khirnov wrote:
> It is not used in the encoder.
> ---
> Does this fix the crash?
> ---
> libavcodec/snow.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
This patch alone works fine
i havnt tested it in conjunction with other patches as i slightly
lost track but 1-3 + this seem not to build or i did something stupid
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
"I am not trying to be anyone's saviour, I'm trying to think about the
future and not be sad" - Elon Musk
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavc/snow: only allocate mconly_picture for decoding
2022-04-11 19:28 ` Michael Niedermayer
@ 2022-04-13 10:21 ` Anton Khirnov
0 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-04-13 10:21 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Michael Niedermayer (2022-04-11 21:28:10)
> On Mon, Apr 11, 2022 at 10:49:34AM +0200, Anton Khirnov wrote:
> > It is not used in the encoder.
> > ---
> > Does this fix the crash?
> > ---
> > libavcodec/snow.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
>
> This patch alone works fine
>
> i havnt tested it in conjunction with other patches as i slightly
> lost track but 1-3 + this seem not to build or i did something stupid
Probably conflicts with the AVCodec callback restructuring by Andreas.
This patch should not interact with the previous ones in any way, so it
should be fine if it works on its own
I also pushed the rebased versions of patches before this one, so you
can test it again if you prefer.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (6 preceding siblings ...)
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 8/8] lavc: drop a confusing message about "thread emulation" Anton Khirnov
@ 2022-04-13 10:23 ` Anton Khirnov
2022-06-05 5:23 ` Soft Works
2022-06-05 7:01 ` Anton Khirnov
9 siblings, 0 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-04-13 10:23 UTC (permalink / raw)
To: FFmpeg development discussions and patches
pushed patches 1-3 + new patch dropping EncodeSimpleContext.
Will push the rest soonish if nobody objects.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 6/8] lavc/avcodec: only allocate decoding packets for decoders
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 6/8] lavc/avcodec: only allocate decoding packets for decoders Anton Khirnov
@ 2022-04-13 14:51 ` Andreas Rheinhardt
0 siblings, 0 replies; 33+ messages in thread
From: Andreas Rheinhardt @ 2022-04-13 14:51 UTC (permalink / raw)
To: ffmpeg-devel
Anton Khirnov:
> ---
> libavcodec/avcodec.c | 7 +------
> libavcodec/decode.c | 8 ++++++++
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index c7daa385e7..5fd988a41c 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -180,12 +180,7 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>
> avci->buffer_frame = av_frame_alloc();
> avci->buffer_pkt = av_packet_alloc();
> - avci->in_pkt = av_packet_alloc();
> - avci->last_pkt_props = av_packet_alloc();
> - avci->pkt_props = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
> - AV_FIFO_FLAG_AUTO_GROW);
> - if (!avci->buffer_frame || !avci->buffer_pkt ||
> - !avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props) {
> + if (!avci->buffer_frame || !avci->buffer_pkt) {
> ret = AVERROR(ENOMEM);
> goto free_and_end;
> }
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 3733e6d4b8..bb3857afd9 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1527,6 +1527,7 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
>
> int ff_decode_preinit(AVCodecContext *avctx)
> {
> + AVCodecInternal *avci = avctx->internal;
> int ret = 0;
>
> /* if the decoder init function was already called previously,
> @@ -1605,6 +1606,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
> avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS;
> }
>
> + avci->in_pkt = av_packet_alloc();
> + avci->last_pkt_props = av_packet_alloc();
> + avci->pkt_props = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
> + AV_FIFO_FLAG_AUTO_GROW);
> + if (!avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props)
> + return AVERROR(ENOMEM);
> +
> ret = decode_bsfs_init(avctx);
> if (ret < 0)
> return ret;
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295326.html needs
to be applied before this patch or flushing an encoder will segfault.
- 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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (7 preceding siblings ...)
2022-04-13 10:23 ` [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
@ 2022-06-05 5:23 ` Soft Works
2022-06-05 7:01 ` Anton Khirnov
9 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 5:23 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Wednesday, March 23, 2022 4:57 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> On entry to avcodec_open2(), the type and id either have to be
> UNKNOWN/NONE or have to match the codec to be used.
> ---
> libavcodec/avcodec.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index fbe4a5e413..dbaa9f78a2 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -158,17 +158,15 @@ int attribute_align_arg avcodec_open2(AVCodecContext
> *avctx, const AVCodec *code
> codec = avctx->codec;
> codec2 = ffcodec(codec);
>
> - if ((avctx->codec_type == AVMEDIA_TYPE_UNKNOWN || avctx->codec_type ==
> codec->type) &&
> - avctx->codec_id == AV_CODEC_ID_NONE) {
> - avctx->codec_type = codec->type;
> - avctx->codec_id = codec->id;
> - }
> - if (avctx->codec_id != codec->id || (avctx->codec_type != codec->type &&
> - avctx->codec_type !=
> AVMEDIA_TYPE_ATTACHMENT)) {
> + if ((avctx->codec_type != AVMEDIA_TYPE_UNKNOWN && avctx->codec_type !=
> codec->type) ||
> + (avctx->codec_id != AV_CODEC_ID_NONE && avctx->codec_id !=
> codec->id)) {
> av_log(avctx, AV_LOG_ERROR, "Codec type or id mismatches\n");
> return AVERROR(EINVAL);
> }
> - avctx->codec = codec;
> +
> + avctx->codec_type = codec->type;
> + avctx->codec_id = codec->id;
> + avctx->codec = codec;
>
> if (avctx->extradata_size < 0 || avctx->extradata_size >=
> FF_MAX_EXTRADATA_SIZE)
> return AVERROR(EINVAL);
> --
This is causing a regression in ffprobe.
The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT which
was required for ffprobe and had been added with e83c716e16c52fa56a78274408f7628e5dc719da.
The demand from the commit message is not yet guaranteed to be fulfilled:
> On entry to avcodec_open2(), the type and id either have to be
> UNKNOWN/NONE or have to match the codec to be used.
I have one verified example (maybe a second will follow), which is an MKV with
an attachment "stream" of type "text".
The found codec will be textdec of type 'subtitle' even though the stream type
is attachment. Without the special condition for attachment streams, this
is now causing ffprobe to error out with non-zero exit code and incomplete
output.
------------------------------------------------------------------------
Example:
[...]
Stream #0:9: Attachment: text
Metadata:
filename : textfile.text
mimetype : text/plain
[text @ 000001AC32310340] Codec type or id mismatches
Could not open codec for input stream 9
------------------------------------------------------------------------
Regards,
softworkz
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
` (8 preceding siblings ...)
2022-06-05 5:23 ` Soft Works
@ 2022-06-05 7:01 ` Anton Khirnov
2022-06-05 7:54 ` Soft Works
2022-06-05 8:20 ` Anton Khirnov
9 siblings, 2 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-06-05 7:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Soft Works (2022-06-05 07:23:18)
> This is causing a regression in ffprobe.
>
> The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT which
> was required for ffprobe and had been added with e83c716e16c52fa56a78274408f7628e5dc719da.
>
> The demand from the commit message is not yet guaranteed to be fulfilled:
>
> > On entry to avcodec_open2(), the type and id either have to be
> > UNKNOWN/NONE or have to match the codec to be used.
>
> I have one verified example (maybe a second will follow), which is an MKV with
> an attachment "stream" of type "text".
> The found codec will be textdec of type 'subtitle' even though the stream type
> is attachment. Without the special condition for attachment streams, this
> is now causing ffprobe to error out with non-zero exit code and incomplete
> output.
>
>
> ------------------------------------------------------------------------
> Example:
>
> [...]
> Stream #0:9: Attachment: text
> Metadata:
> filename : textfile.text
> mimetype : text/plain
> [text @ 000001AC32310340] Codec type or id mismatches
> Could not open codec for input stream 9
> ------------------------------------------------------------------------
This sounds very much like a bug in ffprobe. It makes no sense to call
avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 7:01 ` Anton Khirnov
@ 2022-06-05 7:54 ` Soft Works
2022-06-05 7:59 ` Soft Works
2022-06-05 8:20 ` Anton Khirnov
1 sibling, 1 reply; 33+ messages in thread
From: Soft Works @ 2022-06-05 7:54 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Sunday, June 5, 2022 9:01 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> Quoting Soft Works (2022-06-05 07:23:18)
> > This is causing a regression in ffprobe.
> >
> > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT which
> > was required for ffprobe and had been added with
> e83c716e16c52fa56a78274408f7628e5dc719da.
> >
> > The demand from the commit message is not yet guaranteed to be fulfilled:
> >
> > > On entry to avcodec_open2(), the type and id either have to be
> > > UNKNOWN/NONE or have to match the codec to be used.
> >
> > I have one verified example (maybe a second will follow), which is an MKV
> with
> > an attachment "stream" of type "text".
> > The found codec will be textdec of type 'subtitle' even though the stream
> type
> > is attachment. Without the special condition for attachment streams, this
> > is now causing ffprobe to error out with non-zero exit code and incomplete
> > output.
> >
> >
> > ------------------------------------------------------------------------
> > Example:
> >
> > [...]
> > Stream #0:9: Attachment: text
> > Metadata:
> > filename : textfile.text
> > mimetype : text/plain
> > [text @ 000001AC32310340] Codec type or id mismatches
> > Could not open codec for input stream 9
> > ------------------------------------------------------------------------
>
> This sounds very much like a bug in ffprobe. It makes no sense to call
> avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
You make a behavioral change to an API function that had this behavior
established and constant over more than 10 years, and when that change
breaks functionality, it's the callers' fault?
How does this go together with all that peanut counting of major, minor
and micro version numbers per library? What is this versioning good for,
when you can make breaking changes and declare the breakage as bugs?
Though, I don't want to say that your change is wrong or shouldn't be made.
Yet, the change requires ffprobe to be adjusted (I just wouldn't call
it a "bug fix"..)
Thanks,
softworkz
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 7:54 ` Soft Works
@ 2022-06-05 7:59 ` Soft Works
0 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 7:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Soft Works
> Sent: Sunday, June 5, 2022 9:55 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > Khirnov
> > Sent: Sunday, June 5, 2022 9:01 AM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec
> id/type
> > validity checking
> >
> > Quoting Soft Works (2022-06-05 07:23:18)
> > > This is causing a regression in ffprobe.
> > >
> > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT
> which
> > > was required for ffprobe and had been added with
> > e83c716e16c52fa56a78274408f7628e5dc719da.
> > >
> > > The demand from the commit message is not yet guaranteed to be fulfilled:
> > >
> > > > On entry to avcodec_open2(), the type and id either have to be
> > > > UNKNOWN/NONE or have to match the codec to be used.
> > >
> > > I have one verified example (maybe a second will follow), which is an MKV
> > with
> > > an attachment "stream" of type "text".
> > > The found codec will be textdec of type 'subtitle' even though the stream
> > type
> > > is attachment. Without the special condition for attachment streams, this
> > > is now causing ffprobe to error out with non-zero exit code and
> incomplete
> > > output.
> > >
> > >
> > > ------------------------------------------------------------------------
> > > Example:
> > >
> > > [...]
> > > Stream #0:9: Attachment: text
> > > Metadata:
> > > filename : textfile.text
> > > mimetype : text/plain
> > > [text @ 000001AC32310340] Codec type or id mismatches
> > > Could not open codec for input stream 9
> > > ------------------------------------------------------------------------
> >
> > This sounds very much like a bug in ffprobe. It makes no sense to call
> > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
>
> You make a behavioral change to an API function that had this behavior
> established and constant over more than 10 years, and when that change
> breaks functionality, it's the callers' fault?
> How does this go together with all that peanut counting of major, minor
> and micro version numbers per library? What is this versioning good for,
> when you can make breaking changes and declare the breakage as bugs?
As per your logic - wouldn't this change require a major version bump?
Or even an avcodec_open3() to ensure backward-compatibility?
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 7:01 ` Anton Khirnov
2022-06-05 7:54 ` Soft Works
@ 2022-06-05 8:20 ` Anton Khirnov
2022-06-05 8:55 ` Paul B Mahol
` (3 more replies)
1 sibling, 4 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-06-05 8:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Soft Works (2022-06-05 09:54:51)
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > Khirnov
> > Sent: Sunday, June 5, 2022 9:01 AM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> > validity checking
> >
> > Quoting Soft Works (2022-06-05 07:23:18)
> > > This is causing a regression in ffprobe.
> > >
> > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT which
> > > was required for ffprobe and had been added with
> > e83c716e16c52fa56a78274408f7628e5dc719da.
> > >
> > > The demand from the commit message is not yet guaranteed to be fulfilled:
> > >
> > > > On entry to avcodec_open2(), the type and id either have to be
> > > > UNKNOWN/NONE or have to match the codec to be used.
> > >
> > > I have one verified example (maybe a second will follow), which is an MKV
> > with
> > > an attachment "stream" of type "text".
> > > The found codec will be textdec of type 'subtitle' even though the stream
> > type
> > > is attachment. Without the special condition for attachment streams, this
> > > is now causing ffprobe to error out with non-zero exit code and incomplete
> > > output.
> > >
> > >
> > > ------------------------------------------------------------------------
> > > Example:
> > >
> > > [...]
> > > Stream #0:9: Attachment: text
> > > Metadata:
> > > filename : textfile.text
> > > mimetype : text/plain
> > > [text @ 000001AC32310340] Codec type or id mismatches
> > > Could not open codec for input stream 9
> > > ------------------------------------------------------------------------
> >
> > This sounds very much like a bug in ffprobe. It makes no sense to call
> > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
>
> You make a behavioral change to an API function that had this behavior
> established and constant over more than 10 years, and when that change
> breaks functionality, it's the callers' fault?
> How does this go together with all that peanut counting of major, minor
> and micro version numbers per library? What is this versioning good for,
> when you can make breaking changes and declare the breakage as bugs?
We maintain compatibility for valid API usage. We do not maintain bug
compatibility.
I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
is valid API usage. What do you expect it to do? There are no
AVMEDIA_TYPE_ATTACHMENT decoders.
More generally, arguments along the line of "change <X> is needed to
keep program <Y> working>" on their own sound very shady to me and
suggest that perhaps program <Y> should not be doing whatever it is
doing.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 8:20 ` Anton Khirnov
@ 2022-06-05 8:55 ` Paul B Mahol
2022-06-05 8:55 ` Soft Works
` (2 subsequent siblings)
3 siblings, 0 replies; 33+ messages in thread
From: Paul B Mahol @ 2022-06-05 8:55 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sun, Jun 5, 2022 at 10:20 AM Anton Khirnov <anton@khirnov.net> wrote:
> Quoting Soft Works (2022-06-05 09:54:51)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Anton
> > > Khirnov
> > > Sent: Sunday, June 5, 2022 9:01 AM
> > > To: FFmpeg development discussions and patches <
> ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec
> id/type
> > > validity checking
> > >
> > > Quoting Soft Works (2022-06-05 07:23:18)
> > > > This is causing a regression in ffprobe.
> > > >
> > > > The commit removes the special-case check for
> AVMEDIA_TYPE_ATTACHMENT which
> > > > was required for ffprobe and had been added with
> > > e83c716e16c52fa56a78274408f7628e5dc719da.
> > > >
> > > > The demand from the commit message is not yet guaranteed to be
> fulfilled:
> > > >
> > > > > On entry to avcodec_open2(), the type and id either have to be
> > > > > UNKNOWN/NONE or have to match the codec to be used.
> > > >
> > > > I have one verified example (maybe a second will follow), which is
> an MKV
> > > with
> > > > an attachment "stream" of type "text".
> > > > The found codec will be textdec of type 'subtitle' even though the
> stream
> > > type
> > > > is attachment. Without the special condition for attachment streams,
> this
> > > > is now causing ffprobe to error out with non-zero exit code and
> incomplete
> > > > output.
> > > >
> > > >
> > > >
> ------------------------------------------------------------------------
> > > > Example:
> > > >
> > > > [...]
> > > > Stream #0:9: Attachment: text
> > > > Metadata:
> > > > filename : textfile.text
> > > > mimetype : text/plain
> > > > [text @ 000001AC32310340] Codec type or id mismatches
> > > > Could not open codec for input stream 9
> > > >
> ------------------------------------------------------------------------
> > >
> > > This sounds very much like a bug in ffprobe. It makes no sense to call
> > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
> >
> > You make a behavioral change to an API function that had this behavior
> > established and constant over more than 10 years, and when that change
> > breaks functionality, it's the callers' fault?
> > How does this go together with all that peanut counting of major, minor
> > and micro version numbers per library? What is this versioning good for,
> > when you can make breaking changes and declare the breakage as bugs?
>
> We maintain compatibility for valid API usage. We do not maintain bug
> compatibility.
>
> I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> is valid API usage. What do you expect it to do? There are no
> AVMEDIA_TYPE_ATTACHMENT decoders.
>
> More generally, arguments along the line of "change <X> is needed to
> keep program <Y> working>" on their own sound very shady to me and
> suggest that perhaps program <Y> should not be doing whatever it is
> doing.
>
You broke existing functionality, fix it ASAP or revert those changes!
> --
> 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-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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 8:20 ` Anton Khirnov
2022-06-05 8:55 ` Paul B Mahol
@ 2022-06-05 8:55 ` Soft Works
2022-06-05 9:15 ` Soft Works
2022-06-05 10:42 ` Anton Khirnov
3 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 8:55 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Sunday, June 5, 2022 10:20 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> Quoting Soft Works (2022-06-05 09:54:51)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > > Khirnov
> > > Sent: Sunday, June 5, 2022 9:01 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec
> id/type
> > > validity checking
> > >
> > > Quoting Soft Works (2022-06-05 07:23:18)
> > > > This is causing a regression in ffprobe.
> > > >
> > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT
> which
> > > > was required for ffprobe and had been added with
> > > e83c716e16c52fa56a78274408f7628e5dc719da.
> > > >
> > > > The demand from the commit message is not yet guaranteed to be
> fulfilled:
> > > >
> > > > > On entry to avcodec_open2(), the type and id either have to be
> > > > > UNKNOWN/NONE or have to match the codec to be used.
> > > >
> > > > I have one verified example (maybe a second will follow), which is an
> MKV
> > > with
> > > > an attachment "stream" of type "text".
> > > > The found codec will be textdec of type 'subtitle' even though the
> stream
> > > type
> > > > is attachment. Without the special condition for attachment streams,
> this
> > > > is now causing ffprobe to error out with non-zero exit code and
> incomplete
> > > > output.
> > > >
> > > >
> > > > -----------------------------------------------------------------------
> -
> > > > Example:
> > > >
> > > > [...]
> > > > Stream #0:9: Attachment: text
> > > > Metadata:
> > > > filename : textfile.text
> > > > mimetype : text/plain
> > > > [text @ 000001AC32310340] Codec type or id mismatches
> > > > Could not open codec for input stream 9
> > > > -----------------------------------------------------------------------
> -
> > >
> > > This sounds very much like a bug in ffprobe. It makes no sense to call
> > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
> >
> > You make a behavioral change to an API function that had this behavior
> > established and constant over more than 10 years, and when that change
> > breaks functionality, it's the callers' fault?
> > How does this go together with all that peanut counting of major, minor
> > and micro version numbers per library? What is this versioning good for,
> > when you can make breaking changes and declare the breakage as bugs?
>
> We maintain compatibility for valid API usage. We do not maintain bug
> compatibility.
> I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> is valid API usage. What do you expect it to do? There are no
> AVMEDIA_TYPE_ATTACHMENT decoders.
>
> More generally, arguments along the line of "change <X> is needed to
> keep program <Y> working>" on their own sound very shady to me and
> suggest that perhaps program <Y> should not be doing whatever it is
> doing.
I might agree to that when:
- the function documentation would have been clear about it
- it wouldn't be ffprobe code getting invalidated by the change
When looking at all the APIs that you are so carefully protecting with
those version numbers, there is a small number of APIs, with good to
great documentation, but a large number where you can't get the
slightest clue about how it is supposed to be used and called and
which conditions need to be met, which prerequisites are required
before calling, how to interpret and what to do with the results, etc.
...without looking at the ffmpeg/ffprobe code for how these tools
are using the APIs.
I mean - almost everybody does that. And when somebody is looking
at the code of ffprobe to understand how to use the ffmpeg API,
then one needs to be able to rely on the code he sees there
being correct. And even if it isn't from an ffmpeg internal perspective,
I would still consider it as an API break when a change would
cause such code fail.
sw
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 8:20 ` Anton Khirnov
2022-06-05 8:55 ` Paul B Mahol
2022-06-05 8:55 ` Soft Works
@ 2022-06-05 9:15 ` Soft Works
2022-06-05 10:42 ` Anton Khirnov
3 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 9:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Sunday, June 5, 2022 10:20 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> Quoting Soft Works (2022-06-05 09:54:51)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > > Khirnov
> > > Sent: Sunday, June 5, 2022 9:01 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec
> id/type
> > > validity checking
> > >
> > > Quoting Soft Works (2022-06-05 07:23:18)
> > > > This is causing a regression in ffprobe.
> > > >
> > > > The commit removes the special-case check for AVMEDIA_TYPE_ATTACHMENT
> which
> > > > was required for ffprobe and had been added with
> > > e83c716e16c52fa56a78274408f7628e5dc719da.
> > > >
> > > > The demand from the commit message is not yet guaranteed to be
> fulfilled:
> > > >
> > > > > On entry to avcodec_open2(), the type and id either have to be
> > > > > UNKNOWN/NONE or have to match the codec to be used.
> > > >
> > > > I have one verified example (maybe a second will follow), which is an
> MKV
> > > with
> > > > an attachment "stream" of type "text".
> > > > The found codec will be textdec of type 'subtitle' even though the
> stream
> > > type
> > > > is attachment. Without the special condition for attachment streams,
> this
> > > > is now causing ffprobe to error out with non-zero exit code and
> incomplete
> > > > output.
> > > >
> > > >
> > > > -----------------------------------------------------------------------
> -
> > > > Example:
> > > >
> > > > [...]
> > > > Stream #0:9: Attachment: text
> > > > Metadata:
> > > > filename : textfile.text
> > > > mimetype : text/plain
> > > > [text @ 000001AC32310340] Codec type or id mismatches
> > > > Could not open codec for input stream 9
> > > > -----------------------------------------------------------------------
> -
> > >
> > > This sounds very much like a bug in ffprobe. It makes no sense to call
> > > avcodec_open2() with the AVMEDIA_TYPE_ATTACHMENT type.
> >
> > You make a behavioral change to an API function that had this behavior
> > established and constant over more than 10 years, and when that change
> > breaks functionality, it's the callers' fault?
> > How does this go together with all that peanut counting of major, minor
> > and micro version numbers per library? What is this versioning good for,
> > when you can make breaking changes and declare the breakage as bugs?
>
> We maintain compatibility for valid API usage. We do not maintain bug
> compatibility.
>
> I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> is valid API usage. What do you expect it to do? There are no
> AVMEDIA_TYPE_ATTACHMENT decoders.
>
> More generally, arguments along the line of "change <X> is needed to
> keep program <Y> working>" on their own sound very shady to me and
> suggest that perhaps program <Y> should not be doing whatever it is
> doing.
Shady? I don't understand how you can say that.
First of all:
You broke ffprobe.
It took me one and a half working days to trace this back from the
symptom that the user was experiencing to that commit of yours.
Now, you could fix that in ffprobe - but then it would be still
broken for:
1. Applications which have copied the code from ffprobe
2. Users running (an unchanged) ffprobe with a newer libavcodec
version; (you said it would be so important that the libraries
are exchangeable and multiple versions can be mixed)
Adjusting ffprobe would still break these two cases.
Thanks,
sw
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 8:20 ` Anton Khirnov
` (2 preceding siblings ...)
2022-06-05 9:15 ` Soft Works
@ 2022-06-05 10:42 ` Anton Khirnov
2022-06-05 10:55 ` Soft Works
` (2 more replies)
3 siblings, 3 replies; 33+ messages in thread
From: Anton Khirnov @ 2022-06-05 10:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
So much text, but no actual answer. Again:
> I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> is valid API usage. What do you expect it to do? There are no
> AVMEDIA_TYPE_ATTACHMENT decoders.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 10:42 ` Anton Khirnov
@ 2022-06-05 10:55 ` Soft Works
2022-06-05 11:10 ` Soft Works
2022-06-05 13:20 ` Anton Khirnov
2 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 10:55 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Sunday, June 5, 2022 12:42 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> So much text, but no actual answer. Again:
> > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> > is valid API usage. What do you expect it to do? There are no
> > AVMEDIA_TYPE_ATTACHMENT decoders.
Yes. That's perfectly right. But it doesn't matter at this point.
And I used "so much text" to precisely explain why it doesn't matter.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 10:42 ` Anton Khirnov
2022-06-05 10:55 ` Soft Works
@ 2022-06-05 11:10 ` Soft Works
2022-06-05 13:20 ` Anton Khirnov
2 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 11:10 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Sunday, June 5, 2022 12:42 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> So much text, but no actual answer. Again:
> > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> > is valid API usage. What do you expect it to do? There are no
> > AVMEDIA_TYPE_ATTACHMENT decoders.
As you didn't mention anything about how you want to address it, does it
mean that your intention is to leave it as is and declare all other code
being wrong?
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 10:42 ` Anton Khirnov
2022-06-05 10:55 ` Soft Works
2022-06-05 11:10 ` Soft Works
@ 2022-06-05 13:20 ` Anton Khirnov
2022-06-05 14:06 ` Soft Works
2 siblings, 1 reply; 33+ messages in thread
From: Anton Khirnov @ 2022-06-05 13:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Soft Works (2022-06-05 13:10:49)
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > Khirnov
> > Sent: Sunday, June 5, 2022 12:42 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> > validity checking
> >
> > So much text, but no actual answer. Again:
> > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> > > is valid API usage. What do you expect it to do? There are no
> > > AVMEDIA_TYPE_ATTACHMENT decoders.
>
> As you didn't mention anything about how you want to address it, does it
> mean that your intention is to leave it as is and declare all other code
> being wrong?
Frankly, your attitude of breathlessly repeating "ffprobe is BROKEN, and
this is HORRIBLE" is unhelpful.
I am open to various kinds of solutions, which include (temporarily?)
reintroducing previous behavior, but first we must determine what the
actual issue is. I.e. whether it is libavcodec or ffprobe that is
broken. You seem uninterested in this question, which makes me not very
interested in spending time on this.
--
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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking
2022-06-05 13:20 ` Anton Khirnov
@ 2022-06-05 14:06 ` Soft Works
0 siblings, 0 replies; 33+ messages in thread
From: Soft Works @ 2022-06-05 14:06 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Sunday, June 5, 2022 3:21 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type
> validity checking
>
> Quoting Soft Works (2022-06-05 13:10:49)
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> > > Khirnov
> > > Sent: Sunday, June 5, 2022 12:42 PM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec
> id/type
> > > validity checking
> > >
> > > So much text, but no actual answer. Again:
> > > > I fail to see how calling avcodec_open2() with AVMEDIA_TYPE_ATTACHMENT
> > > > is valid API usage. What do you expect it to do? There are no
> > > > AVMEDIA_TYPE_ATTACHMENT decoders.
> >
> > As you didn't mention anything about how you want to address it, does it
> > mean that your intention is to leave it as is and declare all other code
> > being wrong?
>
> Frankly, your attitude of breathlessly repeating "ffprobe is BROKEN, and
> this is HORRIBLE" is unhelpful.
> I am open to various kinds of solutions, which include (temporarily?)
> reintroducing previous behavior, but first we must determine what the
> actual issue is. I.e. whether it is libavcodec or ffprobe that is
> broken. You seem uninterested in this question, which makes me not very
> interested in spending time on this.
I need to fight about every single character of submitted code, and you
are trying to justify your commit that clearly breaks behavior instead of
either reverting or offering a solution.
Instead I need to go through stupid discussions with you. I don't understand
that behavior. For most others it would be totally clear that such commit
would need to be reverted until a new solution is found.
I have reported the issue nicely and well explained. But you start to find
some justifications instead of suggesting any solution.
I don't like that. I wish I wouldn't have been required to write that much
text, and you would have just responded something like, OK, I'll see how I
can resolve the regression that my commit has caused.
That would be a normal reaction IMO.
Regards,
softworkz
_______________________________________________
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".
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2022-06-05 14:06 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-23 15:57 [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 2/8] lavc/avcodec: only allocate the encoding frame for encoders Anton Khirnov
2022-03-23 16:29 ` James Almer
2022-04-11 8:39 ` [FFmpeg-devel] [PATCH] lavc/encode: drop EncodeSimpleContext Anton Khirnov
2022-04-11 9:16 ` Paul B Mahol
2022-04-11 16:32 ` James Almer
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 3/8] lavc: move default get_buffer2() to its own file Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 4/8] lavc/snow: only allocate mconly_picture for decoding Anton Khirnov
2022-03-24 23:07 ` Michael Niedermayer
2022-04-11 8:49 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
2022-04-11 19:28 ` Michael Niedermayer
2022-04-13 10:21 ` Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 5/8] lavc/encode: add an encoder-specific get_buffer() variant Anton Khirnov
2022-03-23 16:26 ` James Almer
2022-04-11 9:05 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 6/8] lavc/avcodec: only allocate decoding packets for decoders Anton Khirnov
2022-04-13 14:51 ` Andreas Rheinhardt
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 7/8] lavc/pthread_frame: do not copy AVCodecInternal contents Anton Khirnov
2022-03-23 15:57 ` [FFmpeg-devel] [PATCH 8/8] lavc: drop a confusing message about "thread emulation" Anton Khirnov
2022-04-13 10:23 ` [FFmpeg-devel] [PATCH 1/8] lavc/avcodec: simplify codec id/type validity checking Anton Khirnov
2022-06-05 5:23 ` Soft Works
2022-06-05 7:01 ` Anton Khirnov
2022-06-05 7:54 ` Soft Works
2022-06-05 7:59 ` Soft Works
2022-06-05 8:20 ` Anton Khirnov
2022-06-05 8:55 ` Paul B Mahol
2022-06-05 8:55 ` Soft Works
2022-06-05 9:15 ` Soft Works
2022-06-05 10:42 ` Anton Khirnov
2022-06-05 10:55 ` Soft Works
2022-06-05 11:10 ` Soft Works
2022-06-05 13:20 ` Anton Khirnov
2022-06-05 14:06 ` Soft Works
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git