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

[James Almer]

On 10/1/2020 8:57 PM, Mark Thompson wrote:
> 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.

Seems overtly complicated for what ultimately will be the same result.
The fragment will have only the units that were not discarded before
being returned to the caller.

And the caller (av1dec) can't really know if a unit was decomposed or
not, assuming they are not removed from the fragment. In the case of
Patch 3/4, it will return EAGAIN after having filled the OBU header bits.

> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".