[FFmpeg-devel] [PATCH 23/36] avcodec/utils: Add utility functions for bsf
James Almer
jamrial at gmail.com
Sat May 30 19:48:30 EEST 2020
On 5/30/2020 1:05 PM, Andreas Rheinhardt wrote:
> Several bitstream filters (e.g. dump_extradata, imxdump, mjpeg2jpeg,
> mjpegadump, mp3decomp, ...) don't buffer packets; instead, they just
> modify the buffer of one packet and don't change any other of the
> packet's non-buffer properties. The usual approach of these bitstream
> filters is to use separate packets for in- and output as follows:
>
> 1. Get the input packet via ff_bsf_get_packet() which entails an allocation.
> 2. Use av_new_packet() to allocate a big enough buffer in the output
> packet.
> 3. Perform the actual work of the bitstream filter, i.e. fill the output
> buffer.
> 4. Use av_packet_copy_props() to copy the non-buffer fields of the input
> packet to the output packet.
> 5. Free the input packet and return.
>
> This commit adds two utility functions that allow a different approach:
> A function to (re)allocate a refcounted buffer with zeroed padding and a
> function to replace a packet's buffer and the buffer-related fields with
> information from an AVBufferRef. This allows to modify the bitstream
> filters as follows:
>
> 1. Get the packet via ff_bsf_get_packet_ref().
> 2. Use ff_buffer_padded_realloc() to get a big enough refcounted buffer.
> 3. Perform the actual work of the bitstream filter.
> 4. Use ff_packet_replace_buffer() to replace the old data in the packet
> with the modified one and return.
>
> The first of these functions is just packet_alloc() from avpacket.c which
> has been made non-static.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> An earlier version put the declarations to internal.h, but James
> suggested putting them into bsf_internal.h (or actually, into bsf.h
> because bsf_internal.h didn't exist back then), so I went with this.
I don't quite recall this, but in any case, ff_buffer_padded_realloc()
should be in internal.h or some other header since it's not bsf specific
and the function moved to utils.c next to the likes of
av_fast_padded_malloc(), and ff_packet_replace_buffer() should, if
applied (see my comments below), be in a new packet_internal.h header
instead, for the same reason.
See the patch i just sent.
>
> There is currently no user that actually makes use of the reallocation
> feature; it was initially thought for hevc_mp4toannexb, but then I
> decided to make it no longer reallocate the output buffer at all any
> more. But I kept this functionality. Might be useful some day.
>
> libavcodec/avpacket.c | 21 ++++++++++++++++-----
> libavcodec/bsf_internal.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 033f2d8f26..c8f3b0cf7a 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -27,6 +27,7 @@
> #include "libavutil/mathematics.h"
> #include "libavutil/mem.h"
>
> +#include "bsf_internal.h"
> #include "bytestream.h"
> #include "internal.h"
> #include "packet.h"
> @@ -69,7 +70,7 @@ void av_packet_free(AVPacket **pkt)
> av_freep(pkt);
> }
>
> -static int packet_alloc(AVBufferRef **buf, int size)
> +int ff_buffer_padded_realloc(AVBufferRef **buf, int size)
> {
> int ret;
> if (size < 0 || size >= INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
> @@ -87,7 +88,7 @@ static int packet_alloc(AVBufferRef **buf, int size)
> int av_new_packet(AVPacket *pkt, int size)
> {
> AVBufferRef *buf = NULL;
> - int ret = packet_alloc(&buf, size);
> + int ret = ff_buffer_padded_realloc(&buf, size);
> if (ret < 0)
> return ret;
>
> @@ -621,7 +622,7 @@ int av_packet_ref(AVPacket *dst, const AVPacket *src)
> goto fail;
>
> if (!src->buf) {
> - ret = packet_alloc(&dst->buf, src->size);
> + ret = ff_buffer_padded_realloc(&dst->buf, src->size);
> if (ret < 0)
> goto fail;
> av_assert1(!src->size || src->data);
> @@ -674,7 +675,7 @@ int av_packet_make_refcounted(AVPacket *pkt)
> if (pkt->buf)
> return 0;
>
> - ret = packet_alloc(&pkt->buf, pkt->size);
> + ret = ff_buffer_padded_realloc(&pkt->buf, pkt->size);
> if (ret < 0)
> return ret;
> av_assert1(!pkt->size || pkt->data);
> @@ -694,7 +695,7 @@ int av_packet_make_writable(AVPacket *pkt)
> if (pkt->buf && av_buffer_is_writable(pkt->buf))
> return 0;
>
> - ret = packet_alloc(&buf, pkt->size);
> + ret = ff_buffer_padded_realloc(&buf, pkt->size);
> if (ret < 0)
> return ret;
> av_assert1(!pkt->size || pkt->data);
> @@ -770,3 +771,13 @@ int ff_side_data_set_prft(AVPacket *pkt, int64_t timestamp)
>
> return 0;
> }
> +
> +void ff_packet_replace_buffer(AVPacket *pkt, AVBufferRef *buf)
> +{
> + av_buffer_unref(&pkt->buf);
> +
> + pkt->buf = buf;
> + pkt->data = buf->data;
> + av_assert1(buf->size >= AV_INPUT_BUFFER_PADDING_SIZE);
> + pkt->size = buf->size - AV_INPUT_BUFFER_PADDING_SIZE;
> +}
This function as is doesn't really seem too useful to me. It for example
can't be used in an scenario where pkt->size is intended to be less than
buf->size (Think buffers that come from a pool that was created using a
max expected size).
Adding a size parameter might be a good idea, while ensuring it's <=
buf->size - AV_INPUT_BUFFER_PADDING_SIZE.
> diff --git a/libavcodec/bsf_internal.h b/libavcodec/bsf_internal.h
> index fefd5b8905..edaacaa2dd 100644
> --- a/libavcodec/bsf_internal.h
> +++ b/libavcodec/bsf_internal.h
> @@ -19,6 +19,7 @@
> #ifndef AVCODEC_BSF_INTERNAL_H
> #define AVCODEC_BSF_INTERNAL_H
>
> +#include "libavutil/buffer.h"
> #include "libavutil/log.h"
>
> #include "bsf.h"
> @@ -42,6 +43,32 @@ int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt);
> */
> int ff_bsf_get_packet_ref(AVBSFContext *ctx, AVPacket *pkt);
>
> +/**
> + * (Re)allocate an AVBufferRef to an effective size of size. In addition,
> + * the buffer will have AV_BUFFER_INPUT_PADDING_SIZE bytes of zeroed padding
> + * at the end.
> + *
> + * @param buf Pointer to pointer to an AVBufferRef. Must not be NULL;
> + * *buf will be reallocated to the desired size; if *buf is NULL,
> + * it will be allocated, otherwise it must be writable.
> + * @param size Desired usable size.
> + * @return Zero on success, negative error code on failure.
> + * *buf is unchanged on error.
> + */
> +int ff_buffer_padded_realloc(AVBufferRef **buf, int size);
> +
> +/**
> + * Unreference the packet's buf and replace it with the given buf.
> + * The packet's data and size fields will be updated with the information
> + * from buf, too.
> + *
> + * @param pkt Pointer to a packet to modify. Must not be NULL.
> + * @param buf Pointer to an AVBufferRef. Must not be NULL.
> + * buf will be owned by pkt afterwards. Its size must include
> + * AV_INPUT_BUFFER_PADDING_SIZE bytes of padding at the end.
> + */
> +void ff_packet_replace_buffer(AVPacket *pkt, AVBufferRef *buf);
> +
> const AVClass *ff_bsf_child_class_next(const AVClass *prev);
>
> #endif /* AVCODEC_BSF_INTERNAL_H */
>
More information about the ffmpeg-devel
mailing list