[FFmpeg-devel] [PATCH 5/5] avformat/matroskaenc: update AV1 support

[Steve Lhomme]

On 30/07/2018 17:13, James Almer wrote:
> On 7/30/2018 2:03 AM, Steve Lhomme wrote:
>> On 26/07/2018 03:11, James Almer wrote:
>>> Make sure to not write forbidden OBUs to CodecPrivate, and do the same
>>> with
>>> unnecessary OBUs for packets.
>> Does this include reordering the OBUs ? The first one must be the
>> Sequence Header OBU. Also it must be the only one of that kind.
> No, it doesn't. I can make that change, but why does it matter if it's
> the first? Why can't a metadata OBU be before a Seq Header? isobmff

Because if the spec say it's the first and its enforced, the OBU parser 
needed at the container level may be simpler. The muxing side can have 
more complexity if needed.

> doesn't care what's first, only what's in.

I requested that the same constraint be applied in ISOBMFF
https://github.com/AOMediaCodec/av1-isobmff/pull/47#issuecomment-408598719

> A parser is currently forced to know how to read OBUs anyway, and it
> could just skip any metadata obu before the sequence header.
>
> For that matter, i do agree with
> https://github.com/AOMediaCodec/av1-isobmff/issues/7#issuecomment-406257234
> and
> https://github.com/AOMediaCodec/av1-isobmff/issues/46#issuecomment-406516226
> that perhaps av1C should, much like hvcC, avcC and in it's own way also
> vpcC, contain a bunch of header bytes listing some basic information
> (and therefore the entire thing, sans atom size and fourcc, be included
> in CodecPrivate verbatim). Things like profile, level, and maybe number
> of OBUs, before the raw OBUs.
> As an extra benefit than simply removing the requirement for demuxers to
> parse OBUs that may even only be in sample data just to see if the file
> can be decoded (firing up a hardware decoder can be expensive), it would
> also give us a way to identify the source of the bitstream (mp4/mkv vs
> ivf/raw/annexb/etc), for example for the purpose of inserting the
> extradata Seq Header if not present on key frames (Matroska allows to
> remove it, but isobmff in section 2.5 seems to want a sync sample to be
> a fully compliant RAP without the need of extradata obus, despite the
> description in section 2.4.4), achieved by writing and using an
> av1_mp4tolobf bsf or similar, which would then be included where needed
> instead of the naive dump_extradata one from patch 2/5 of this set, for
> example.

I totally support this. In the end storing OBUs in extra data is 
becoming a PITA, especially if it needs to be prepended to some frames 
but not all. If we stick to a simpler structure with what's needed to 
identify decoder properties (mostly the profile) it would make things a 
lot easier.

>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>    libavformat/matroskaenc.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>> index b7ff1950d3..816ddd059a 100644
>>> --- a/libavformat/matroskaenc.c
>>> +++ b/libavformat/matroskaenc.c
>>> @@ -21,6 +21,7 @@
>>>      #include <stdint.h>
>>>    +#include "av1.h"
>>>    #include "avc.h"
>>>    #include "hevc.h"
>>>    #include "avformat.h"
>>> @@ -769,6 +770,9 @@ static int
>>> mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
>>>            ff_isom_write_hvcc(dyn_cp, par->extradata,
>>>                               par->extradata_size, 0);
>>>            return 0;
>>> +    case AV_CODEC_ID_AV1:
>>> +        return ff_isom_write_av1c(dyn_cp, par->extradata,
>>> +                                  par->extradata_size);
>>>        case AV_CODEC_ID_ALAC:
>>>            if (par->extradata_size < 36) {
>>>                av_log(s, AV_LOG_ERROR,
>>> @@ -2120,6 +2124,8 @@ static void mkv_write_block(AVFormatContext *s,
>>> AVIOContext *pb,
>>>                 (AV_RB24(par->extradata) == 1 ||
>>> AV_RB32(par->extradata) == 1))
>>>            /* extradata is Annex B, assume the bitstream is too and
>>> convert it */
>>>            ff_hevc_annexb2mp4_buf(pkt->data, &data, &size, 0, NULL);
>>> +    else if (par->codec_id == AV_CODEC_ID_AV1)
>>> +        ff_av1_filter_obus_buf(pkt->data, &data, &size);
>>>        else if (par->codec_id == AV_CODEC_ID_WAVPACK) {
>>>            int ret = mkv_strip_wavpack(pkt->data, &data, &size);
>>>            if (ret < 0) {
>>> -- 
>>> 2.18.0
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel