[FFmpeg-devel] [PATCH 6/6] avformat/av1: Improve filtering AV1 OBUs

[James Almer]

On 1/23/2020 1:08 PM, Andreas Rheinhardt wrote:
> Both ISOBMFF as well as Matroska require certain OBUs to be stripped
> before muxing them. There are two functions for this purpose; one writes
> directly into an AVIOContext, the other returns a freshly allocated
> buffer with the undesired units stripped away.
> 
> The latter one actually relies on the former by means of a dynamic
> buffer. This has several drawbacks: The underlying buffer might have to
> be reallocated multiple times; the buffer will eventually be
> overallocated; the data will not be directly copied into the final
> buffer, but rather first in the write buffer (in chunks of 1024 byte)
> and then written in these chunks. Moreover, the API for dynamic buffers
> is defective wrt error checking and as a consequence, the earlier code
> would indicate a length of -AV_INPUT_BUFFER_PADDING_SIZE on allocation
> failure, but it would not return an error; there would also be no error
> in case the arbitrary limit of INT_MAX/2 that is currently imposed on
> dynamic buffers is hit.
> 
> This commit changes this: The buffer is now parsed twice, once to get
> the precise length which will then be allocated; and once to actually
> write the data.
> 
> For a 22.7mb/s file with average framesize 113 kB this improved the time
> for the calls to ff_av1_filter_obus_buf() when writing Matroska from
> 753662 decicycles to 313319 decicycles (based upon 50 runs a 2048 frames
> each); for another 1.5mb/s file (with average framesize of 7.3 kB) it
> improved from 79270 decicycles to 34539 decicycles (based upon 50 runs a
> 4096 frames).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavformat/av1.c | 32 +++++++++++++++++++++-----------
>  libavformat/av1.h |  4 ++--
>  2 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/libavformat/av1.c b/libavformat/av1.c
> index 4ff4bffddf..80c049f62f 100644
> --- a/libavformat/av1.c
> +++ b/libavformat/av1.c
> @@ -19,6 +19,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +#include "libavutil/avassert.h"
>  #include "libavutil/mem.h"
>  #include "libavcodec/av1.h"
>  #include "libavcodec/av1_parse.h"
> @@ -48,7 +49,8 @@ int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size)
>          case AV1_OBU_PADDING:
>              break;
>          default:
> -            avio_write(pb, buf, len);
> +            if (pb)
> +                avio_write(pb, buf, len);
>              size += len;
>              break;
>          }
> @@ -58,23 +60,31 @@ int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size)
>      return size;
>  }
>  
> -int ff_av1_filter_obus_buf(const uint8_t *buf, uint8_t **out, int *size)
> +int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out, int *size)
>  {
> -    AVIOContext *pb;
> -    int ret;
> +    AVIOContext pb;
> +    uint8_t *buf;
> +    int len, ret;
>  
> -    ret = avio_open_dyn_buf(&pb);
> -    if (ret < 0)
> -        return ret;
> -
> -    ret = ff_av1_filter_obus(pb, buf, *size);
> +    len = ret = ff_av1_filter_obus(NULL, in, *size);
>      if (ret < 0) {
> -        ffio_free_dyn_buf(&pb);
>          return ret;
>      }
>  
> +    buf = av_malloc((size_t)len + AV_INPUT_BUFFER_PADDING_SIZE);

We never cast the arguments to av_malloc() to size_t, and len here is
guaranteed to be <= size by parse_obu_header(), so it's unnecessary.

Removed it and pushed the patch. Thanks.

> +    if (!buf)
> +        return AVERROR(ENOMEM);
> +
> +    ffio_init_context(&pb, buf, len, 1, NULL, NULL, NULL, NULL);
> +
> +    ret = ff_av1_filter_obus(&pb, in, *size);
> +    av_assert1(ret == len);
> +
> +    memset(buf + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> +
>      av_freep(out);
> -    *size = avio_close_dyn_buf(pb, out);
> +    *out  = buf;
> +    *size = len;
>  
>      return 0;
>  }
> diff --git a/libavformat/av1.h b/libavformat/av1.h
> index 0578435376..acba12612c 100644
> --- a/libavformat/av1.h
> +++ b/libavformat/av1.h
> @@ -61,7 +61,7 @@ int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size);
>   *
>   * @param pb pointer to the AVIOContext where the filtered bitstream shall be
>   *           written
> - * @param buf input data buffer
> + * @param in input data buffer
>   * @param out pointer to pointer that will hold the allocated data buffer
>   * @param size size of the input data buffer. The size of the resulting output
>                 data buffer will be written here
> @@ -69,7 +69,7 @@ int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size);
>   * @return 0 in case of success, a negative AVERROR code in case of failure.
>   *         On failure, out and size are unchanged
>   */
> -int ff_av1_filter_obus_buf(const uint8_t *buf, uint8_t **out, int *size);
> +int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out, int *size);
>  
>  /**
>   * Parses a Sequence Header from the the provided buffer.
>