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 <jamr...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel