[FFmpeg-devel] [PATCH v2] avcodec: add an AV1 parser

[Mark Thompson]

On 03/10/18 02:44, James Almer wrote:
> Simple parser to set keyframes, frame type, structure, width, height, and pixel
> format, plus stream profile and level.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> Missing Changelog entry and version bump still.
> 
>  configure               |   1 +
>  libavcodec/Makefile     |   1 +
>  libavcodec/av1_parser.c | 226 ++++++++++++++++++++++++++++++++++++++++
>  libavcodec/parsers.c    |   1 +
>  4 files changed, 229 insertions(+)
>  create mode 100644 libavcodec/av1_parser.c

Looks good, and tested a bit (+1 for it removing the annoying need for -copyinkf when messing with BSFs in stream copy from raw IVF files).

One very minor nitpick below, then no more from me :)

Thanks,

- Mark


> ...
> diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c
> new file mode 100644
> index 0000000000..0dee3e9d2e
> --- /dev/null
> +++ b/libavcodec/av1_parser.c
> ...
> +
> +static int av1_parser_parse(AVCodecParserContext *ctx,
> +                            AVCodecContext *avctx,
> +                            const uint8_t **out_data, int *out_size,
> +                            const uint8_t *data, int size)
> +{
> +    AV1ParseContext *s = ctx->priv_data;
> +    CodedBitstreamFragment *td = &s->temporal_unit;
> +    CodedBitstreamAV1Context *av1 = s->cbc->priv_data;
> +    int ret;
> +
> +    *out_data = data;
> +    *out_size = size;
> +
> +    ctx->key_frame         = -1;
> +    ctx->pict_type         = AV_PICTURE_TYPE_NONE;
> +    ctx->picture_structure = AV_PICTURE_STRUCTURE_UNKNOWN;
> +
> +    s->cbc->log_ctx = avctx;

I think this should be unset at the end of the function as well.  While I don't see a way for it to do so currently, some later call like the ff_cbs_close() could in theory attempt to use the logging context and then invoke undefined behaviour because the AVCodecContext assigned here need not still be accessible.

> +
> +    if (avctx->extradata_size && !s->parsed_extradata) {
> +        s->parsed_extradata = 1;
> +
> +        ret = ff_cbs_read(s->cbc, td, avctx->extradata, avctx->extradata_size);
> +        if (ret < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to parse extradata.\n");
> +            return size;
> +        }
> +
> +        ff_cbs_fragment_uninit(s->cbc, td);
> +    }
> +
> +    ret = ff_cbs_read(s->cbc, td, data, size);
> +    if (ret < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to parse temporal unit.\n");
> +        return size;
> +    }
> +
> +    if (!av1->sequence_header) {
> +        av_log(avctx, AV_LOG_ERROR, "No sequence header available\n");
> +        goto end;
> +    }
> +
> +    for (int i = 0; i < td->nb_units; i++) {
> +        CodedBitstreamUnit *unit = &td->units[i];
> +        AV1RawOBU *obu = unit->content;
> +        AV1RawSequenceHeader *seq = av1->sequence_header;
> +        AV1RawColorConfig *color = &seq->color_config;
> +        AV1RawFrameHeader *frame;
> +        int frame_type;
> +
> +        if (unit->type == AV1_OBU_FRAME)
> +            frame = &obu->obu.frame.header;
> +        else if (unit->type == AV1_OBU_FRAME_HEADER)
> +            frame = &obu->obu.frame_header;
> +        else
> +            continue;
> +
> +        if (frame->show_existing_frame) {
> +            AV1ReferenceFrameState *ref = &av1->ref[frame->frame_to_show_map_idx];
> +
> +            if (!ref->valid) {
> +                av_log(avctx, AV_LOG_ERROR, "Invalid reference frame\n");
> +                goto end;
> +            }
> +
> +            ctx->width  = ref->frame_width;
> +            ctx->height = ref->frame_height;
> +            frame_type  = ref->frame_type;
> +
> +            ctx->key_frame = 0;
> +        } else if (!frame->show_frame) {
> +            continue;
> +        } else {
> +            ctx->width  = av1->frame_width;
> +            ctx->height = av1->frame_height;
> +            frame_type  = frame->frame_type;
> +
> +            ctx->key_frame = frame_type == AV1_FRAME_KEY;
> +        }
> +
> +        avctx->profile = seq->seq_profile;
> +        avctx->level   = seq->seq_level_idx[0];
> +
> +        switch (frame_type) {
> +        case AV1_FRAME_KEY:
> +        case AV1_FRAME_INTRA_ONLY:
> +            ctx->pict_type = AV_PICTURE_TYPE_I;
> +            break;
> +        case AV1_FRAME_INTER:
> +            ctx->pict_type = AV_PICTURE_TYPE_P;
> +            break;
> +        case AV1_FRAME_SWITCH:
> +            ctx->pict_type = AV_PICTURE_TYPE_SP;
> +            break;
> +        }
> +        ctx->picture_structure = AV_PICTURE_STRUCTURE_FRAME;
> +
> +        switch (av1->bit_depth) {
> +        case 8:
> +            ctx->format = color->mono_chrome ? AV_PIX_FMT_GRAY8
> +                                             : pix_fmts_8bit [color->subsampling_x][color->subsampling_y];
> +            break;
> +        case 10:
> +            ctx->format = color->mono_chrome ? AV_PIX_FMT_GRAY10
> +                                             : pix_fmts_10bit[color->subsampling_x][color->subsampling_y];
> +            break;
> +        case 12:
> +            ctx->format = color->mono_chrome ? AV_PIX_FMT_GRAY12
> +                                             : pix_fmts_12bit[color->subsampling_x][color->subsampling_y];
> +            break;
> +        }
> +        av_assert2(ctx->format != AV_PIX_FMT_NONE);
> +    }
> +
> +end:
> +    ff_cbs_fragment_uninit(s->cbc, td);
> +
> +    return size;
> +}
> ...