[FFmpeg-devel] [PATCH 12/12] avformat/av1: Avoid allocation + copying when filtering OBUs
James Almer
jamrial at gmail.com
Sun Jan 26 17:57:44 EET 2020
On 1/24/2020 7:48 PM, Andreas Rheinhardt wrote:
> Certain types of OBUs are stripped away before muxing into Matroska and
> ISOBMFF; there are two functions to do this: One that outputs by
> directly writing in an AVIOContext and one that returns a freshly
> allocated buffer with the units not stripped away copied into it.
>
> The latter option is bad for performance, especially when the input
> does already not contain any of the units intended to be stripped away
> (this covers typical remuxing scenarios). Therefore this commit changes
> this by avoiding allocating and copying when possible; it is possible if
> the OBUs to be retained are consecutively in the input buffer (without
> an OBU to be discarded between them). In this case, the caller receives
> the offset as well as the length of the part of the buffer that contains
> the units to be kept. This also avoids copying when e.g. the only unit
> to be discarded is a temporal delimiter at the front.
>
> 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
> 313319 decicycles to 2368 decicycles; for another file with 1.5mb/s
> (average framesize 7.3 kB) it improved from 34539 decicycles to 1922
> decicyles. For these files the only units that needed to be stripped
> away were temporal unit delimiters at the front.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> If it is desired, I can add a commit to switch ff_mov_write_packet() to
> not use a pointer just for reformatted_data (that is of course
> initialized to NULL), but to replace it by a data buffer that gets
> initialized to pkt->data and modified so that data + offset always
> points to the current data. (This is possible now that the av_freep()
> have been removed from the reformatting functions.)
Yes, this would be ideal if the speed gains above can also be done for
movenc.
>
> libavformat/av1.c | 50 ++++++++++++++++++++++++++++++++-------
> libavformat/av1.h | 13 ++++++----
> libavformat/matroskaenc.c | 2 +-
> libavformat/movenc.c | 11 +++++----
> 4 files changed, 58 insertions(+), 18 deletions(-)
>
> diff --git a/libavformat/av1.c b/libavformat/av1.c
> index 07b399efcc..fef8e96f8d 100644
> --- a/libavformat/av1.c
> +++ b/libavformat/av1.c
> @@ -29,13 +29,20 @@
> #include "avio.h"
> #include "avio_internal.h"
>
> -int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size)
> +static int av1_filter_obus(AVIOContext *pb, const uint8_t *buf,
> + int size, int *offset)
> {
> - const uint8_t *end = buf + size;
> + const uint8_t *start = buf, *end = buf + size;
> int64_t obu_size;
> - int start_pos, type, temporal_id, spatial_id;
> -
> - size = 0;
> + int off, start_pos, type, temporal_id, spatial_id;
> + enum {
> + START_NOT_FOUND,
> + START_FOUND,
> + END_FOUND,
> + OFFSET_IMPOSSIBLE,
> + } state = START_NOT_FOUND;
> +
> + off = size = 0;
> while (buf < end) {
> int len = parse_obu_header(buf, end - buf, &obu_size, &start_pos,
> &type, &temporal_id, &spatial_id);
> @@ -47,8 +54,16 @@ int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size)
> case AV1_OBU_REDUNDANT_FRAME_HEADER:
> case AV1_OBU_TILE_LIST:
> case AV1_OBU_PADDING:
> + if (state == START_FOUND)
> + state = END_FOUND;
> break;
> default:
> + if (state == START_NOT_FOUND) {
> + off = buf - start;
> + state = START_FOUND;
> + } else if (state == END_FOUND) {
> + state = OFFSET_IMPOSSIBLE;
> + }
> if (pb)
> avio_write(pb, buf, len);
> size += len;
> @@ -57,19 +72,35 @@ int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size)
> buf += len;
> }
>
> + if (offset)
> + *offset = state != OFFSET_IMPOSSIBLE ? off : -1;
> +
> return size;
> }
>
> -int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out, int *size)
> +int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size)
> +{
> + return av1_filter_obus(pb, buf, size, NULL);
> +}
> +
> +int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out,
> + int *size, int *offset)
> {
> AVIOContext pb;
> uint8_t *buf;
> - int len, ret;
> + int len, off, ret;
>
> - len = ret = ff_av1_filter_obus(NULL, in, *size);
> + len = ret = av1_filter_obus(NULL, in, *size, &off);
> if (ret < 0) {
> return ret;
> }
> + if (off >= 0) {
> + *out = (uint8_t *)in;
> + *size = len;
> + *offset = off;
> +
> + return 0;
> + }
>
> buf = av_malloc((size_t)len + AV_INPUT_BUFFER_PADDING_SIZE);
> if (!buf)
> @@ -77,13 +108,14 @@ int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out, int *size)
>
> ffio_init_context(&pb, buf, len, 1, NULL, NULL, NULL, NULL);
>
> - ret = ff_av1_filter_obus(&pb, in, *size);
> + ret = av1_filter_obus(&pb, in, *size, NULL);
> av_assert1(ret == len);
>
> memset(buf + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>
> *out = buf;
> *size = len;
> + *offset = 0;
>
> return 0;
> }
> diff --git a/libavformat/av1.h b/libavformat/av1.h
> index 6cc3fcaad2..dd5b47dc25 100644
> --- a/libavformat/av1.h
> +++ b/libavformat/av1.h
> @@ -56,19 +56,24 @@ typedef struct AV1SequenceParameters {
> int ff_av1_filter_obus(AVIOContext *pb, const uint8_t *buf, int size);
>
> /**
> - * Filter out AV1 OBUs not meant to be present in ISOBMFF sample data and write
> - * the resulting bitstream to a newly allocated data buffer.
> + * Filter out AV1 OBUs not meant to be present in ISOBMFF sample data and return
> + * the result in a data buffer, avoiding allocations and copies if possible.
> *
> * @param in input data buffer
> - * @param out pointer to pointer that will hold the allocated data buffer
> + * @param out pointer to pointer for the returned buffer. In case of success,
> + * it is independently allocated if and only if `*out` differs from in.
> * @param size size of the input data buffer. The size of the resulting output
> * data buffer will be written here
> + * @param offset offset of the returned data inside `*out`: It runs from
> + * `*out + offset` (inclusive) to `*out + offset + size`
> + * (exclusive); is zero if `*out` is independently allocated.
> *
> * @return 0 in case of success, a negative AVERROR code in case of failure.
> * On failure, *out and *size are unchanged
> * @note *out will be treated as unintialized on input and will not be freed.
> */
> -int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out, int *size);
> +int ff_av1_filter_obus_buf(const uint8_t *in, uint8_t **out,
> + int *size, int *offset);
>
> /**
> * Parses a Sequence Header from the the provided buffer.
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 20bad95262..618f07c769 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2112,7 +2112,7 @@ static int mkv_write_block(AVFormatContext *s, AVIOContext *pb,
> /* extradata is Annex B, assume the bitstream is too and convert it */
> err = ff_hevc_annexb2mp4_buf(pkt->data, &data, &size, 0, NULL);
> } else if (par->codec_id == AV_CODEC_ID_AV1) {
> - err = ff_av1_filter_obus_buf(pkt->data, &data, &size);
> + err = ff_av1_filter_obus_buf(pkt->data, &data, &size, &offset);
> } else if (par->codec_id == AV_CODEC_ID_WAVPACK) {
> err = mkv_strip_wavpack(pkt->data, &data, &size);
> } else
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index b5e06de3d5..f715f911f3 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5374,7 +5374,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> AVCodecParameters *par = trk->par;
> AVProducerReferenceTime *prft;
> unsigned int samples_in_chunk = 0;
> - int size = pkt->size, ret = 0;
> + int size = pkt->size, ret = 0, offset = 0;
> int prft_size;
> uint8_t *reformatted_data = NULL;
>
> @@ -5491,7 +5491,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> }
> } else if (par->codec_id == AV_CODEC_ID_AV1) {
> if (trk->hint_track >= 0 && trk->hint_track < mov->nb_streams) {
> - ret = ff_av1_filter_obus_buf(pkt->data, &reformatted_data, &size);
> + ret = ff_av1_filter_obus_buf(pkt->data, &reformatted_data,
> + &size, &offset);
> if (ret < 0)
> return ret;
> avio_write(pb, reformatted_data, size);
> @@ -5667,12 +5668,14 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>
> if (trk->hint_track >= 0 && trk->hint_track < mov->nb_streams)
> ff_mov_add_hinted_packet(s, pkt, trk->hint_track, trk->entry,
> - reformatted_data, size);
> + reformatted_data ? reformatted_data + offset
> + : NULL, size);
>
> end:
> err:
>
> - av_free(reformatted_data);
> + if (pkt->data != reformatted_data)
> + av_free(reformatted_data);
> return ret;
> }
Seems to work, so pushed alongside a new test to mux AV1 in Matroska
that tests the offset path.
I'll add another test with a sample using Padding OBUs in frames to test
the path allocating a new buffer.
More information about the ffmpeg-devel
mailing list