[FFmpeg-devel] [PATCH] Use AVBufferPool in MOV

[James Almer]

On 11/22/2024 2:46 PM, Arnaud Masserann wrote:
> Most demuxers allocate a new buffer for each packet.
> For MOV, this can be problematic, because of some high-bitrate
> codecs like ProRes/HAPQ/NotchLC.
> 
> Use pools of buffer instead.
> 
> For some test media, demuxing goes from 20ms/frame to 11ms/frame,
> close to the perf of ReadFile (10ms/frame), making it possible
> to read the media at 60Hz.
> 
> Signed-off-by: Arnaud Masserann <Arnaud1602 at gmail.com>
> ---
>   libavformat/avformat.h | 12 ++++++++++++
>   libavformat/isom.h     |  2 ++
>   libavformat/mov.c      | 20 ++++++++++++++++++--
>   libavformat/utils.c    | 29 +++++++++++++++++++++++++++++
>   4 files changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 56c1c80289..25f2e59b22 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -440,6 +440,18 @@ int av_get_packet(AVIOContext *s, AVPacket *pkt, int size);
>    */
>   int av_append_packet(AVIOContext *s, AVPacket *pkt, int size);
>   
> +
> +/**
> + * Like av_get_packet, but allocates the AVPacket's buffer from a pool.
> + *
> + * @param s    associated IO context
> + * @param pool the pool of buffers; must be initialized with a sufficient size
> + * @param pkt packet
> + * @param size desired payload size
> + * @return >0 (read size) if OK, AVERROR_xxx otherwise
> + */
> +int av_get_pooled_packet(AVIOContext *s, AVBufferPool* pool, AVPacket *pkt, int size);

This doesn't need to be public. It can be internal to lavf just fine.

> +
>   /*************************************************/
>   /* input/output formats */
>   
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 4723397048..c608f30e04 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -273,6 +273,8 @@ typedef struct MOVStreamContext {
>       } cenc;
>   
>       struct IAMFDemuxContext *iamf;
> +
> +    AVBufferPool* pools[32];

Like you argued, this can be in MovContext instead, since the pools are 
fixed size and can be used by any stream.

>   } MOVStreamContext;
>   
>   typedef struct HEIFItem {
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index a2333ac1fd..2668bdf78f 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -9583,6 +9583,7 @@ static void mov_free_encryption_index(MOVEncryptionIndex **index) {
>   static void mov_free_stream_context(AVFormatContext *s, AVStream *st)
>   {
>       MOVStreamContext *sc = st->priv_data;
> +    int i;
>   
>       if (!sc || --sc->refcount) {
>           st->priv_data = NULL;
> @@ -9639,6 +9640,9 @@ static void mov_free_stream_context(AVFormatContext *s, AVStream *st)
>           ff_iamf_read_deinit(sc->iamf);
>   #endif
>       av_freep(&sc->iamf);
> +
> +    for (i = 0; i < FF_ARRAY_ELEMS(sc->pools); i++)
> +        av_buffer_pool_uninit(&sc->pools[i]);
>   }
>   
>   static int mov_read_close(AVFormatContext *s)
> @@ -10544,6 +10548,16 @@ static int mov_finalize_packet(AVFormatContext *s, AVStream *st, AVIndexEntry *s
>       return 0;
>   }
>   
> +static AVBufferPool *mov_pool_get(AVBufferPool* pools[32], int size)
> +{
> +    int index = av_log2(size + AV_INPUT_BUFFER_PADDING_SIZE);
> +    if (!pools[index]) {
> +        int pool_size = 2 << index;
> +        pools[index] = av_buffer_pool_init(pool_size, NULL);
> +    }
> +    return pools[index];
> +}
> +
>   static int mov_read_packet(AVFormatContext *s, AVPacket *pkt)
>   {
>       MOVContext *mov = s->priv_data;
> @@ -10617,8 +10631,10 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt)
>                   return FFERROR_REDO;
>           }
>   #endif
> -        else
> -            ret = av_get_packet(sc->pb, pkt, sample->size);
> +        else{
> +            AVBufferPool* pool = mov_pool_get(sc->pools, sample->size);
> +            ret = av_get_pooled_packet(sc->pb, pool, pkt, sample->size);
> +        }
>           if (ret < 0) {
>               if (should_retry(sc->pb, ret)) {
>                   mov_current_sample_dec(sc);
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index e9ded627ad..869e97c089 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -104,6 +104,35 @@ FF_ENABLE_DEPRECATION_WARNINGS
>       return append_packet_chunked(s, pkt, size);
>   }
>   
> +int av_get_pooled_packet(AVIOContext *s, AVBufferPool* pool, AVPacket *pkt, int size)
> +{
> +#if FF_API_INIT_PACKET
> +FF_DISABLE_DEPRECATION_WARNINGS
> +    av_init_packet(pkt);
> +    pkt->data = NULL;
> +    pkt->size = 0;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#else
> +    av_packet_unref(pkt);
> +#endif
> +    pkt->pos  = avio_tell(s);
> +
> +    if(!pool)
> +        return AVERROR(ENOMEM);
> +
> +    pkt->buf = av_buffer_pool_get(pool);;

Extra ;

> +    if(!pkt->buf)
> +        return AVERROR(ENOMEM);
> +
> +    if(pkt->buf->size < size){
> +        av_packet_unref(pkt); // TODO test this
> +        return AVERROR(ENOMEM);

Maybe EINVAL instead.

Also, you're not setting pkt->data to pkt->buf->data.

> +    }
> +
> +    return append_packet_chunked(s, pkt, size);
> +}
> +
> +
>   int av_append_packet(AVIOContext *s, AVPacket *pkt, int size)
>   {
>       if (!pkt->size)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241124/f94cae75/attachment.sig>