diff mbox series

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

Message ID 20201115215541.4526-2-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/4,v2] avcodec/cbs: add an AVClass to CodedBitstreamType for option handling | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

James Almer Nov. 15, 2020, 9:55 p.m. UTC
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@gmail.com>
---
 libavcodec/cbs.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Mark Thompson Dec. 9, 2020, 9:22 p.m. UTC | #1
On 15/11/2020 21:55, 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@gmail.com>
> ---
>   libavcodec/cbs.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index c7afccd6f5..f4312d199b 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -202,6 +202,12 @@ 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);
> +            av_buffer_unref(&unit->content_ref);
> +            unit->content = NULL;
>           } else if (err < 0) {
>               av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
>                      "(type %"PRIu32").\n", i, unit->type);
> 

The description and log message no longer match exactly what it does.

Code part LGTM.

- Mark
James Almer Dec. 9, 2020, 9:47 p.m. UTC | #2
On 12/9/2020 6:22 PM, Mark Thompson wrote:
> On 15/11/2020 21:55, 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@gmail.com>
>> ---
>>   libavcodec/cbs.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
>> index c7afccd6f5..f4312d199b 100644
>> --- a/libavcodec/cbs.c
>> +++ b/libavcodec/cbs.c
>> @@ -202,6 +202,12 @@ 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);
>> +            av_buffer_unref(&unit->content_ref);
>> +            unit->content = NULL;
>>           } else if (err < 0) {
>>               av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit 
>> %d "
>>                      "(type %"PRIu32").\n", i, unit->type);
>>
> 
> The description and log message no longer match exactly what it does.

Would "Discarding content for unit %d" be ok?

> 
> Code part LGTM.
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index c7afccd6f5..f4312d199b 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -202,6 +202,12 @@  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);
+            av_buffer_unref(&unit->content_ref);
+            unit->content = NULL;
         } else if (err < 0) {
             av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
                    "(type %"PRIu32").\n", i, unit->type);