[FFmpeg-devel,1/2] avcodec/cbs_av1: add a function to get a payload size without trailing zero bytes

Submitted by James Almer on April 13, 2019, 7:21 p.m.

Details

Message ID 20190413192143.4008-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer April 13, 2019, 7:21 p.m.
Factor it out from cbs_av1_read_metadata_itut_t35()

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs_av1.c                 | 11 +++++++++++
 libavcodec/cbs_av1_syntax_template.c | 10 +---------
 2 files changed, 12 insertions(+), 9 deletions(-)

Comments

Mark Thompson April 14, 2019, 5:17 p.m.
On 13/04/2019 20:21, James Almer wrote:
> Factor it out from cbs_av1_read_metadata_itut_t35()
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/cbs_av1.c                 | 11 +++++++++++
>  libavcodec/cbs_av1_syntax_template.c | 10 +---------
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index f02cca2dad..0c03ca28af 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -574,6 +574,17 @@ static int cbs_av1_get_relative_dist(const AV1RawSequenceHeader *seq,
>      return diff;
>  }
>  
> +static int cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
> +{
> +    GetBitContext tmp = *gbc;
> +    int size = 0;
> +    for (int i = 0; get_bits_left(&tmp) >= 8; i++) {
> +        if (get_bits(&tmp, 8))
> +            size = i;
> +    }
> +    return size;
> +}

I don't think it matters because of get_bits, but maybe this should be size_t?

> +
>  
>  #define HEADER(name) do { \
>          ff_cbs_trace_header(ctx, name); \
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index 76eb90b279..56009145e8 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -1674,15 +1674,7 @@ static int FUNC(metadata_itut_t35)(CodedBitstreamContext *ctx, RWContext *rw,
>  #ifdef READ
>      // The payload runs up to the start of the trailing bits, but there might
>      // be arbitrarily many trailing zeroes so we need to read through twice.
> -    {
> -        GetBitContext tmp = *rw;
> -        current->payload_size = 0;
> -        for (i = 0; get_bits_left(rw) >= 8; i++) {
> -            if (get_bits(rw, 8))
> -                current->payload_size = i;
> -        }
> -        *rw = tmp;
> -    }
> +    current->payload_size = cbs_av1_get_payload_bytes_left(rw);
>  
>      current->payload_ref = av_buffer_alloc(current->payload_size);
>      if (!current->payload_ref)
> 

LGTM in any case.

Thanks,

- Mark
James Almer April 14, 2019, 6:56 p.m.
On 4/14/2019 2:17 PM, Mark Thompson wrote:
> On 13/04/2019 20:21, James Almer wrote:
>> Factor it out from cbs_av1_read_metadata_itut_t35()
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/cbs_av1.c                 | 11 +++++++++++
>>  libavcodec/cbs_av1_syntax_template.c | 10 +---------
>>  2 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
>> index f02cca2dad..0c03ca28af 100644
>> --- a/libavcodec/cbs_av1.c
>> +++ b/libavcodec/cbs_av1.c
>> @@ -574,6 +574,17 @@ static int cbs_av1_get_relative_dist(const AV1RawSequenceHeader *seq,
>>      return diff;
>>  }
>>  
>> +static int cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>> +{
>> +    GetBitContext tmp = *gbc;
>> +    int size = 0;
>> +    for (int i = 0; get_bits_left(&tmp) >= 8; i++) {
>> +        if (get_bits(&tmp, 8))
>> +            size = i;
>> +    }
>> +    return size;
>> +}
> 
> I don't think it matters because of get_bits, but maybe this should be size_t?
> 
>> +
>>  
>>  #define HEADER(name) do { \
>>          ff_cbs_trace_header(ctx, name); \
>> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
>> index 76eb90b279..56009145e8 100644
>> --- a/libavcodec/cbs_av1_syntax_template.c
>> +++ b/libavcodec/cbs_av1_syntax_template.c
>> @@ -1674,15 +1674,7 @@ static int FUNC(metadata_itut_t35)(CodedBitstreamContext *ctx, RWContext *rw,
>>  #ifdef READ
>>      // The payload runs up to the start of the trailing bits, but there might
>>      // be arbitrarily many trailing zeroes so we need to read through twice.
>> -    {
>> -        GetBitContext tmp = *rw;
>> -        current->payload_size = 0;
>> -        for (i = 0; get_bits_left(rw) >= 8; i++) {
>> -            if (get_bits(rw, 8))
>> -                current->payload_size = i;
>> -        }
>> -        *rw = tmp;
>> -    }
>> +    current->payload_size = cbs_av1_get_payload_bytes_left(rw);
>>  
>>      current->payload_ref = av_buffer_alloc(current->payload_size);
>>      if (!current->payload_ref)
>>
> 
> LGTM in any case.
> 
> Thanks,

Changed to size_t and pushed.

Patch hide | download patch | download mbox

diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index f02cca2dad..0c03ca28af 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -574,6 +574,17 @@  static int cbs_av1_get_relative_dist(const AV1RawSequenceHeader *seq,
     return diff;
 }
 
+static int cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
+{
+    GetBitContext tmp = *gbc;
+    int size = 0;
+    for (int i = 0; get_bits_left(&tmp) >= 8; i++) {
+        if (get_bits(&tmp, 8))
+            size = i;
+    }
+    return size;
+}
+
 
 #define HEADER(name) do { \
         ff_cbs_trace_header(ctx, name); \
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
index 76eb90b279..56009145e8 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1674,15 +1674,7 @@  static int FUNC(metadata_itut_t35)(CodedBitstreamContext *ctx, RWContext *rw,
 #ifdef READ
     // The payload runs up to the start of the trailing bits, but there might
     // be arbitrarily many trailing zeroes so we need to read through twice.
-    {
-        GetBitContext tmp = *rw;
-        current->payload_size = 0;
-        for (i = 0; get_bits_left(rw) >= 8; i++) {
-            if (get_bits(rw, 8))
-                current->payload_size = i;
-        }
-        *rw = tmp;
-    }
+    current->payload_size = cbs_av1_get_payload_bytes_left(rw);
 
     current->payload_ref = av_buffer_alloc(current->payload_size);
     if (!current->payload_ref)