[FFmpeg-devel] [PATCH 5/5] avformat/movenc: add support for AV1 streams

[James Almer]

On 7/9/2018 6:08 PM, Michael Niedermayer wrote:
> On Mon, Jul 09, 2018 at 03:26:54PM -0300, James Almer wrote:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> ff_av1_filter_obus() could eventually be replaced by an autoinserted
>> filter_units bsf, assuming it doesn't slow down the muxing process
>> too much (CBS is fast reading packets, but not so much assembling and
>> writing packets).
>> ff_isom_write_av1c() however can't be replaced given filter_units
>> doesn't handle extradata (either codecpar or packet side data).
>>
> [...]
>> diff --git a/libavformat/av1.h b/libavformat/av1.h
>> new file mode 100644
>> index 0000000000..733034c12d
>> --- /dev/null
>> +++ b/libavformat/av1.h
>> @@ -0,0 +1,70 @@
>> +/*
>> + * AV1 helper functions for muxers
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#ifndef AVFORMAT_AV1_H
>> +#define AVFORMAT_AV1_H
>> +
>> +#include <stdint.h>
>> +
>> +#include "avio.h"
>> +
>> +/**
>> + * Filter out AV1 OBUs not meant to be present in ISOBMFF sample data and write
>> + * the resulting bitstream to the provided AVIOContext.
>> + *
>> + * @param pb pointer to the AVIOContext where the filtered bitstream shall be
>> + *           written
>> + * @param buf input data buffer
>> + * @param size size of the input data buffer
>> + *
>> + * @return the amount of bytes written in case of success, a negative AVERROR
>> + *         code in case of failure
>> + */
>> +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.
>> + *
>> + * @param pb pointer to the AVIOContext where the filtered bitstream shall be
>> + *           written
>> + * @param buf 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
>> + *
>> + * @return the amount of bytes written in case of success, a negative AVERROR
>> + *         code in case of failure
> 
> this leaves it unspecified what happens to out/size in case of errors
> are they 0/null are they undefined, left as before ?

Depending on when in the function an error happens, i will either be
untouched or *out will be set to NULL.
I can make it so the latter always happens in case of failure and
document it as such if that's preferred, but the current callers pass a
pointer to a pointer set to NULL, so that's already guaranteed.

This is based on ff_avc_parse_nal_units_buf() and
ff_hevc_annexb2mp4_buf(), for that matter, so those could also be fixed.

> 
> 
>> + */
>> +int ff_av1_filter_obus_buf(const uint8_t *buf, uint8_t **out, int *size);
>> +
>> +/**
>> + * Writes AV1 extradata (Sequence Header and Metadata OBUs) to the provided
>> + * AVIOContext.
>> + *
>> + * @param pb pointer to the AVIOContext where the hvcC shall be written
>> + * @param buf input data buffer
> 
>> + * @param size size of the input data buffer
> 
> very minor nitpick but you could add "in bytes"

Sure.

> 
> 
>> + *
>> + * @return 0 in case of success, a negative AVERROR code in case of failure
> 
> if >= 0 is defined as success then its possible to use this in the future
> for some additional information without the need to review all callers

I can make this change, but as you well mention below it doesn't really
matter since it's internal, so anything defined now can be changed later.

> 
> i guess most of this doesnt matter much as its not public API ...
> 
> 
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>