[FFmpeg-devel] [PATCH 12/12] avformat/av1: Avoid allocation + copying when filtering OBUs

James Almer jamrial at gmail.com
Mon Jan 27 03:58:08 EET 2020


On 1/26/2020 10:45 PM, Andreas Rheinhardt wrote:
> On Sun, Jan 26, 2020 at 5:28 PM James Almer <jamrial at gmail.com> wrote:
> 
>> On 1/24/2020 7:48 PM, Andreas Rheinhardt wrote:
>>> 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.
>>
> 
> I think you missed the point of the comment above: It is not about
> performance. Unless movenc uses a hint track for the av1 stream, it did not
> need to copy the data to a freshly allocated buffer and so it did not have
> to suffer the performance penalty for it. Ergo there is nothing to be
> gained for it. And if a hint track is used, it already benefits from this
> patch as-is (and as it has been applied).

Right, for some reason i thought the ff_av1_filter_obus() call from
movenc was using a dynamically allocated AVIOContext, where it's simply
writing directly to the output muxer's AVIOContext instead.

Nevermind then, sorry for the confusion.

> 
> 
>>>
>>>  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.
> 
> 
> Good. I have tested it with redundant frame headers in the middle of
> packets (you were right that libaom can create such files; thanks for the
> suggestion) as well as temporal delimiters (of course) and padding at the
> end (your sample).
> Thanks for your efforts.
> 
> - Andreas
> 
> PS: I'll update my patch
> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-January/255149.html after
> you have added the second new test.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 



More information about the ffmpeg-devel mailing list