On 2/21/2021 4:13 PM, Mark Thompson wrote:
On 21/02/2021 17:35, James Almer wrote:
This callback is functionally the same as get_buffer2() is for
decoders, and
implements for the new encode API the functionality of the old encode
API had
where the user could provide their own buffers.
Signed-off-by: James Almer <jamr...@gmail.com>
---
Used the names Lynne suggested this time, plus a line about how the
callback
must be thread safe.
libavcodec/avcodec.h | 45 ++++++++++++++++++++++++++++++++++++
libavcodec/codec.h | 8 ++++---
libavcodec/encode.c | 54 +++++++++++++++++++++++++++++++++++++++++++-
libavcodec/encode.h | 8 +++++++
libavcodec/options.c | 1 +
5 files changed, 112 insertions(+), 4 deletions(-)
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..e60eb16ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
*/
#define AV_GET_BUFFER_FLAG_REF (1 << 0)
+/**
+ * The encoder will keep a reference to the packet and may reuse it
later.
+ */
+#define AV_GET_ENCODER_BUFFER_FLAG_REF (1 << 0)
+
struct AVCodecInternal;
/**
@@ -2346,6 +2351,39 @@ typedef struct AVCodecContext {
* - encoding: set by user
*/
int export_side_data;
+
+ /**
+ * This callback is called at the beginning of each packet to get
a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet before this
callback is
+ * called:
+ * - size
+ * This callback must use the above value to calculate the
required buffer size,
+ * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data
Is the data pointer allowed to be in write-only memory?
I'm not sure what the use case for this would be, so probably no?
Does it have any alignment requirements?
No, just padding. AVPacket doesn't require alignment for the payload.
+ * - buf must contain a pointer to an AVBufferRef structure. The
packet's
+ * data pointer must be contained in it.
+ * See: av_buffer_create(), av_buffer_alloc(), and
av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then get_encoder_buffer() must
call
+ * avcodec_default_get_encoder_buffer() instead of providing a
buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENCODER_BUFFER_FLAG_REF is set in flags then the
packet may be reused
+ * (read and/or written to if it is writable) later by libavcodec.
+ *
+ * This callback must be thread-safe, as when frame
multithreading is used, it may
+ * be called from multiple threads simultaneously.
Allowing simulatenous calls feels unexpectedly tricky. Is it really
necessary?
This was a suggestion by Lynne, i personally don't know. We support
frame threading encoding (For intra-only codecs), but currently
ff_alloc_packet2() does not seem to be thread safe, seeing it calls
av_fast_padded_malloc(), yet it's called by frame threaded encoders.
Should i remove this?
+ *
+ * @see avcodec_default_get_encoder_buffer()
+ *
+ * - encoding: Set by libavcodec, user can override.
+ * - decoding: unused
+ */
+ int (*get_encoder_buffer)(struct AVCodecContext *s, AVPacket
*pkt, int flags);
Can the encoder ask for arbitrarily many packets?
Can the user return "not yet" somehow to this if they have a fixed
output buffer pool but no buffer is currently available?
No, as is it can't. Return values < 0 are considered errors.
I don't much like the idea of the user suspending the thread in the
callback until they have some available, which might work in some cases
but might also deadlock if an avcodec_receive_packet() call is blocked
by it.
Can we make what's in essence a malloc() call return something like
EAGAIN, and this in turn be propagated back to
encode_receive_packet_internal()? Couldn't this potentially end up in
the forbidden scenario of avcodec_send_frame() and
avcodec_receive_packet() both returning EAGAIN?
(For get_buffer2 we have a bit of consideration of this problem for
hardware frames which might have limited numbers via
avcodec_get_hw_frames_parameters(), though the general case does not
have a solution.)
} AVCodecContext;
#if FF_API_CODEC_GET_SET
@@ -2920,6 +2958,13 @@ void avsubtitle_free(AVSubtitle *sub);
*/
int avcodec_default_get_buffer2(AVCodecContext *s, AVFrame *frame,
int flags);
+/**
+ * The default callback for AVCodecContext.get_encoder_buffer(). It
is made public so
+ * it can be called by custom get_encoder_buffer() implementations
for encoders without
+ * AV_CODEC_CAP_DR1 set.
+ */
+int avcodec_default_get_encoder_buffer(AVCodecContext *s, AVPacket
*pkt, int flags);
+
/**
* Modify width and height values so that they will result in a memory
* buffer that is acceptable for the codec if you do not use any
horizontal
diff --git a/libavcodec/codec.h b/libavcodec/codec.h
index 0ccbf0eb19..a679fdc9e1 100644
--- a/libavcodec/codec.h
+++ b/libavcodec/codec.h
@@ -43,9 +43,11 @@
*/
#define AV_CODEC_CAP_DRAW_HORIZ_BAND (1 << 0)
/**
- * Codec uses get_buffer() for allocating buffers and supports custom
allocators.
- * If not set, it might not use get_buffer() at all or use operations
that
- * assume the buffer was allocated by avcodec_default_get_buffer.
+ * Codec uses get_buffer() or get_encoder_buffer() for allocating
buffers and
+ * supports custom allocators.
+ * If not set, it might not use get_buffer() or get_encoder_buffer()
at all, or
+ * use operations that assume the buffer was allocated by
+ * avcodec_default_get_buffer2 or avcodec_default_get_encoder_buffer.
*/
#define AV_CODEC_CAP_DR1 (1 << 1)
#define AV_CODEC_CAP_TRUNCATED (1 << 3)
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 282337e453..f39c8d38ce 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -56,6 +56,52 @@ int ff_alloc_packet2(AVCodecContext *avctx,
AVPacket *avpkt, int64_t size, int64
return 0;
}
+int avcodec_default_get_encoder_buffer(AVCodecContext *avctx,
AVPacket *avpkt, int flags)
+{
+ int ret;
+
+ if (avpkt->data || avpkt->buf) {
+ av_log(avctx, AV_LOG_ERROR, "avpkt->{data,buf} != NULL in
avcodec_default_get_encoder_buffer()\n");
+ return AVERROR(EINVAL);
+ }
+
+ ret = av_new_packet(avpkt, avpkt->size);
+ if (ret < 0)
+ av_log(avctx, AV_LOG_ERROR, "Failed to allocate packet of
size %d\n", avpkt->size);
+
+ return ret;
+}
+
+int ff_get_encoder_buffer(AVCodecContext *avctx, AVPacket *avpkt,
int64_t size, int flags)
+{
+ int ret;
+
+ if (size < 0 || size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
+ return AVERROR(EINVAL);
+
+ av_assert0(!avpkt->data && !avpkt->buf);
+
+ avpkt->size = size;
+ ret = avctx->get_encoder_buffer(avctx, avpkt, flags);
+ if (ret < 0)
+ goto fail;
+
+ if (!avpkt->data || !avpkt->buf) {
+ av_log(avctx, AV_LOG_ERROR, "No buffer returned by
get_encoder_buffer()\n");
+ ret = AVERROR(EINVAL);
+ goto fail;
+ }
+
+ ret = 0;
+fail:
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR, "get_encoder_buffer() failed\n");
+ av_packet_unref(avpkt);
+ }
+
+ return ret;
+}
+
/**
* Pad last frame with silence.
*/
@@ -169,7 +215,7 @@ static int encode_simple_internal(AVCodecContext
*avctx, AVPacket *avpkt)
emms_c();
if (!ret && got_packet) {
- if (avpkt->data) {
+ if (avpkt->data && !(avctx->codec->capabilities &
AV_CODEC_CAP_DR1)) {
ret = av_packet_make_refcounted(avpkt);
if (ret < 0)
goto end;
@@ -377,6 +423,12 @@ static int compat_encode(AVCodecContext *avctx,
AVPacket *avpkt,
av_log(avctx, AV_LOG_WARNING, "AVFrame.width or height
is not set\n");
}
+ if (avctx->codec->capabilities & AV_CODEC_CAP_DR1) {
+ av_log(avctx, AV_LOG_WARNING, "The deprecated
avcodec_encode_* API does not support "
+ "AV_CODEC_CAP_DR1 encoders\n");
+ return AVERROR(ENOSYS);
+ }
+
ret = avcodec_send_frame(avctx, frame);
if (ret == AVERROR_EOF)
ret = 0;
diff --git a/libavcodec/encode.h b/libavcodec/encode.h
index dfa9cb2d97..3192bd9e38 100644
--- a/libavcodec/encode.h
+++ b/libavcodec/encode.h
@@ -24,6 +24,7 @@
#include "libavutil/frame.h"
#include "avcodec.h"
+#include "packet.h"
/**
* Called by encoders to get the next frame for encoding.
@@ -36,4 +37,11 @@
*/
int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame);
+/**
+ * Get a buffer for a packet. This is a wrapper around
+ * AVCodecContext.get_encoder_buffer() and should be used instead
calling get_encoder_buffer()
+ * directly.
+ */
+int ff_get_encoder_buffer(AVCodecContext *avctx, AVPacket *avpkt,
int64_t size, int flags);
Why int64_t rather than size_t?
That's what ff_alloc_packet2() uses, so i figured it will make porting
existing users much easier (basically just a sed replace, vs trying to
find if any encoder is assuming int64_t, like exrenc).
+
#endif /* AVCODEC_ENCODE_H */
diff --git a/libavcodec/options.c b/libavcodec/options.c
index 4bbf74ec7f..cd5fa6eb14 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -130,6 +130,7 @@ static int init_context_defaults(AVCodecContext
*s, const AVCodec *codec)
s->pkt_timebase = (AVRational){ 0, 1 };
s->get_buffer2 = avcodec_default_get_buffer2;
s->get_format = avcodec_default_get_format;
+ s->get_encoder_buffer = avcodec_default_get_encoder_buffer;
s->execute = avcodec_default_execute;
s->execute2 = avcodec_default_execute2;
s->sample_aspect_ratio = (AVRational){0,1};
It might help to have an implementation for at least one codec and an
example user for review.
Sure, below is huffyuvenc adapted to use this API.
diff --git a/libavcodec/huffyuvenc.c b/libavcodec/huffyuvenc.c
index 28a5534535..709a5a9d0e 100644
--- a/libavcodec/huffyuvenc.c
+++ b/libavcodec/huffyuvenc.c
@@ -29,6 +29,7 @@
*/
#include "avcodec.h"
+#include "encode.h"
#include "huffyuv.h"
#include "huffman.h"
#include "huffyuvencdsp.h"
@@ -762,7 +763,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket
*pkt,
const AVFrame * const p = pict;
int i, j, size = 0, ret;
- if ((ret = ff_alloc_packet2(avctx, pkt, width * height * 3 * 4 +
AV_INPUT_BUFFER_MIN_SIZE, 0)) < 0)
+ if ((ret = ff_get_encoder_buffer(avctx, pkt, width * height * 3 * 4 +
AV_INPUT_BUFFER_MIN_SIZE, 0)) < 0)
return ret;
if (s->context) {
@@ -1111,7 +1112,7 @@ AVCodec ff_ffvhuff_encoder = {
.init = encode_init,
.encode2 = encode_frame,
.close = encode_end,
- .capabilities = AV_CODEC_CAP_FRAME_THREADS,
+ .capabilities = AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_DR1,
.priv_class = &ff_class,
.pix_fmts = (const enum AVPixelFormat[]){
AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P,
AV_PIX_FMT_YUV411P,
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
