| Message ID | 20190401233940.5941-1-sw@jkqxz.net |
|---|---|
| State | New |
| Headers | show |
On 4/1/2019 8:39 PM, Mark Thompson wrote: > This simplifies the parser and improves performance by reducing the number > of allocations and eliminating redundant copies. > --- > libavcodec/av1_parser.c | 63 +++++++++-------------------------------- > 1 file changed, 13 insertions(+), 50 deletions(-) > > diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c > index b916608d65..4a743d92d4 100644 > --- a/libavcodec/av1_parser.c > +++ b/libavcodec/av1_parser.c > @@ -27,7 +27,7 @@ > > typedef struct AV1ParseContext { > CodedBitstreamContext *cbc; > - CodedBitstreamFragment temporal_unit; > + AV1RawFrameHeader frame_header; > int parsed_extradata; > } AV1ParseContext; > > @@ -50,8 +50,10 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > const uint8_t *data, int size) > { > AV1ParseContext *s = ctx->priv_data; > - CodedBitstreamFragment *td = &s->temporal_unit; > CodedBitstreamAV1Context *av1 = s->cbc->priv_data; > + AV1RawSequenceHeader *seq; > + AV1RawColorConfig *color; > + AV1RawFrameHeader *frame; > int ret; > > *out_data = data; > @@ -66,67 +68,35 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > 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) { > + ret = ff_cbs_parse_headers(s->cbc, NULL, > + avctx->extradata, avctx->extradata_size); > + if (ret < 0) > av_log(avctx, AV_LOG_WARNING, "Failed to parse extradata.\n"); > - } > - > - ff_cbs_fragment_reset(s->cbc, td); > } > > - ret = ff_cbs_read(s->cbc, td, data, size); > + ret = ff_cbs_parse_headers(s->cbc, &s->frame_header, data, size); > if (ret < 0) { > av_log(avctx, AV_LOG_ERROR, "Failed to parse temporal unit.\n"); > goto end; > } > + frame = &s->frame_header; > > if (!av1->sequence_header) { > av_log(avctx, AV_LOG_ERROR, "No sequence header available\n"); > goto end; > } > + seq = av1->sequence_header; > + color = &seq->color_config; > > - 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; These should take the value from frame instead. As i said in another reply, i don't think anything in the spec enforces the last frame in a TU to be the visible one, only that there must be exactly one. These two lines are in the parser in its current form, so either it's fixed before this patch, or as part of it (The former would make it easier to backport to release branches). > - frame_type = frame->frame_type; > > - ctx->key_frame = frame_type == AV1_FRAME_KEY; > - } > + ctx->key_frame = frame->frame_type == AV1_FRAME_KEY; > > avctx->profile = seq->seq_profile; > avctx->level = seq->seq_level_idx[0]; > > - switch (frame_type) { > + switch (frame->frame_type) { > case AV1_FRAME_KEY: > case AV1_FRAME_INTRA_ONLY: > ctx->pict_type = AV_PICTURE_TYPE_I; > @@ -155,11 +125,8 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > break; > } > av_assert2(ctx->format != AV_PIX_FMT_NONE); > - } > > end: > - ff_cbs_fragment_reset(s->cbc, td); > - > s->cbc->log_ctx = NULL; > > return size; > @@ -182,9 +149,6 @@ static av_cold int av1_parser_init(AVCodecParserContext *ctx) > if (ret < 0) > return ret; > > - s->cbc->decompose_unit_types = (CodedBitstreamUnitType *)decompose_unit_types; > - s->cbc->nb_decompose_unit_types = FF_ARRAY_ELEMS(decompose_unit_types); You should also delete the decompose_unit_types array. > - > return 0; > } > > @@ -192,7 +156,6 @@ static void av1_parser_close(AVCodecParserContext *ctx) > { > AV1ParseContext *s = ctx->priv_data; > > - ff_cbs_fragment_free(s->cbc, &s->temporal_unit); > ff_cbs_close(&s->cbc); > } > >
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index c388be896b..ff98a1e8f4 100644 --- a/libavcodec/cbs.c +++ b/libavcodec/cbs.c @@ -280,6 +280,15 @@ int ff_cbs_read(CodedBitstreamContext *ctx, return cbs_read_fragment_content(ctx, frag); } +int ff_cbs_parse_headers(CodedBitstreamContext *ctx, void *header, + const uint8_t *data, size_t size) +{ + if (!ctx->codec->parse_headers) + return AVERROR(ENOSYS); + + return ctx->codec->parse_headers(ctx, header, data, size); +} + int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx, CodedBitstreamFragment *frag) diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h index 967dcd1468..be965ae258 100644 --- a/libavcodec/cbs.h +++ b/libavcodec/cbs.h @@ -278,6 +278,20 @@ int ff_cbs_read(CodedBitstreamContext *ctx, CodedBitstreamFragment *frag, const uint8_t *data, size_t size); +/** + * Parse headers from a bitstream from a memory region, updating state + * but not storing intermediate results. + * + * If header is not NULL, the decomposition of the header of the last + * displayed unit in the stream is written there. The type and size of + * this is codec-dependent. + * + * This is intended for use in parsers where only header parsing is + * required and the input is not refcounted. + */ +int ff_cbs_parse_headers(CodedBitstreamContext *ctx, void *header, + const uint8_t *data, size_t size); + /** * Write the content of the fragment to its own internal buffer. diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h index 53f2e5d187..9783555292 100644 --- a/libavcodec/cbs_internal.h +++ b/libavcodec/cbs_internal.h @@ -55,6 +55,10 @@ typedef struct CodedBitstreamType { // Free the codec internal state. void (*close)(CodedBitstreamContext *ctx); + + // Parse headers only. + int (*parse_headers)(CodedBitstreamContext *ctx, void *header, + const uint8_t *data, size_t size); } CodedBitstreamType;