[FFmpeg-devel] [PATCH 2/4] avcodec/cbs: allow cbs_read_fragment_content() to discard units

[Mark Thompson]

On 20/09/2020 18:24, James Almer wrote:
> The caller may not need all units in a fragment in reading only scenarios. They
> could in fact alter global state stored in the private CodedBitstreamType
> fields in an undesirable way.
> And unlike preventing decomposition of units, discarding can be done based on
> parsed values within the unit.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>   libavcodec/cbs.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index e73e18f398..363385b6f3 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -196,6 +196,11 @@ static int cbs_read_fragment_content(CodedBitstreamContext *ctx,
>               av_log(ctx->log_ctx, AV_LOG_VERBOSE,
>                      "Decomposition unimplemented for unit %d "
>                      "(type %"PRIu32").\n", i, unit->type);
> +        } else if (err  == AVERROR(EAGAIN)) {
> +            av_log(ctx->log_ctx, AV_LOG_VERBOSE,
> +                   "Discarding unit %d "
> +                   "(type %"PRIu32").\n", i, unit->type);
> +            ff_cbs_delete_unit(frag, i--);
>           } else if (err < 0) {
>               av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
>                      "(type %"PRIu32").\n", i, unit->type);
> 

I don't really like how it is modifying the units in the fragment internally in the read function.  EAGAIN as an "I didn't do anything with this" return from read_unit seems reasonable, but then deleting the unit here feels outside the intended meaning of the function.

Would it make sense to push that further out somehow?  E.g. av1dec.c ignoring undecomposed units, or maybe a separate function to do deletion under whatever conditions.

- Mark