diff mbox series

[FFmpeg-devel,23/36] avcodec/utils: Add utility functions for bsf

Message ID 20200530160541.29517-23-andreas.rheinhardt@gmail.com
State New
Headers show
Series [FFmpeg-devel,01/36] avcodec/vp9_superframe_bsf: Check for existence of data before reading it
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt May 30, 2020, 4:05 p.m. UTC
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@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.

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(-)

Comments

James Almer May 30, 2020, 4:48 p.m. UTC | #1
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@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 */
>
diff mbox series

Patch

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;
+}
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 */