diff mbox series

[FFmpeg-devel,2/3] avcodec/cbs_av1: infer segmentation parameters from reference frames

Message ID 20201021001126.14044-2-jamrial@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,1/3] avcodec/cbs_av1: infer loop filter delta parameters from reference frames | expand

Checks

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

Commit Message

James Almer Oct. 21, 2020, 12:11 a.m. UTC
Partially implements of setup_past_independence() and load_previous().
These ensures they are always set, even if the values were not coded
in the input bitstream and will not be coded in the output bitstream.

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

Comments

Mark Thompson Oct. 27, 2020, 8:47 p.m. UTC | #1
On 21/10/2020 01:11, James Almer wrote:
> Partially implements of setup_past_independence() and load_previous().
> These ensures they are always set, even if the values were not coded
> in the input bitstream and will not be coded in the output bitstream.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavcodec/cbs_av1.h                 |  2 ++
>   libavcodec/cbs_av1_syntax_template.c | 43 +++++++++++++++++++++-------
>   2 files changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index 97aeee9795..a2d78e736f 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -416,6 +416,8 @@ typedef struct AV1ReferenceFrameState {
>   
>       int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
>       int8_t  loop_filter_mode_deltas[2];
> +    uint8_t feature_enabled[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
> +    int16_t feature_value[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
>   } AV1ReferenceFrameState;
>   
>   typedef struct CodedBitstreamAV1Context {
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index 4edf4fd47c..2df5619279 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -743,8 +743,11 @@ static int FUNC(quantization_params)(CodedBitstreamContext *ctx, RWContext *rw,
>   static int FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext *rw,
>                                        AV1RawFrameHeader *current)
>   {
> +    CodedBitstreamAV1Context  *priv = ctx->priv_data;
>       static const uint8_t bits[AV1_SEG_LVL_MAX] = { 8, 6, 6, 6, 6, 3, 0, 0 };
>       static const uint8_t sign[AV1_SEG_LVL_MAX] = { 1, 1, 1, 1, 1, 0, 0, 0 };
> +    static const uint8_t default_feature_enabled[AV1_SEG_LVL_MAX] = { 0 };
> +    static const int16_t default_feature_value[AV1_SEG_LVL_MAX] = { 0 };
>       int i, j, err;
>   
>       flag(segmentation_enabled);
> @@ -763,22 +766,38 @@ static int FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext *rw,
>               flag(segmentation_update_data);
>           }
>   
> -        if (current->segmentation_update_data) {
>               for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
> -                for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
> -                    flags(feature_enabled[i][j], 2, i, j);
> +                const uint8_t *feature_enabled;
> +                const int16_t *feature_value;

Same comment as previous about ref_feature_enabled etc.

> +
> +                if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
> +                    feature_enabled = default_feature_enabled;
> +                    feature_value = default_feature_value;
> +                } else {
> +                    feature_enabled =
> +                        priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_enabled[i];
> +                    feature_value =
> +                        priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_value[i];
> +                }
>   
> -                    if (current->feature_enabled[i][j] && bits[j] > 0) {
> -                        if (sign[j])
> -                            sus(1 + bits[j], feature_value[i][j], 2, i, j);
> -                        else
> -                            fbs(bits[j], feature_value[i][j], 2, i, j);
> +                for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
> +                    if (current->segmentation_update_data) {
> +                        flags(feature_enabled[i][j], 2, i, j);
> +
> +                        if (current->feature_enabled[i][j] && bits[j] > 0) {
> +                            if (sign[j])
> +                                sus(1 + bits[j], feature_value[i][j], 2, i, j);
> +                            else
> +                                fbs(bits[j], feature_value[i][j], 2, i, j);
> +                        } else {
> +                            infer(feature_value[i][j], 0);
> +                        }
>                       } else {
> -                        infer(feature_value[i][j], 0);
> +                        infer(feature_enabled[i][j], feature_enabled[j]);
> +                        infer(feature_value[i][j], feature_value[j]);
>                       }
>                   }
>               }
> -        }
>       } else {
>           for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
>               for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
> @@ -1645,6 +1664,10 @@ update_refs:
>                      sizeof(current->loop_filter_ref_deltas));
>               memcpy(priv->ref[i].loop_filter_mode_deltas, current->loop_filter_mode_deltas,
>                      sizeof(current->loop_filter_mode_deltas));
> +            memcpy(priv->ref[i].feature_enabled, current->feature_enabled,
> +                   sizeof(current->feature_enabled));
> +            memcpy(priv->ref[i].feature_value, current->feature_value,
> +                   sizeof(current->feature_value));
>           }
>       }
>   
> 

Also sensible.  (Needs the reindent as well, not much value in it being separate given that this diff has most lines changed anyway.)

Thanks,

- Mark
James Almer Oct. 28, 2020, 2:52 p.m. UTC | #2
On 10/27/2020 5:47 PM, Mark Thompson wrote:
> On 21/10/2020 01:11, James Almer wrote:
>> Partially implements of setup_past_independence() and load_previous().
>> These ensures they are always set, even if the values were not coded
>> in the input bitstream and will not be coded in the output bitstream.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/cbs_av1.h                 |  2 ++
>>   libavcodec/cbs_av1_syntax_template.c | 43 +++++++++++++++++++++-------
>>   2 files changed, 35 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
>> index 97aeee9795..a2d78e736f 100644
>> --- a/libavcodec/cbs_av1.h
>> +++ b/libavcodec/cbs_av1.h
>> @@ -416,6 +416,8 @@ typedef struct AV1ReferenceFrameState {
>>         int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
>>       int8_t  loop_filter_mode_deltas[2];
>> +    uint8_t feature_enabled[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
>> +    int16_t feature_value[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
>>   } AV1ReferenceFrameState;
>>     typedef struct CodedBitstreamAV1Context {
>> diff --git a/libavcodec/cbs_av1_syntax_template.c
>> b/libavcodec/cbs_av1_syntax_template.c
>> index 4edf4fd47c..2df5619279 100644
>> --- a/libavcodec/cbs_av1_syntax_template.c
>> +++ b/libavcodec/cbs_av1_syntax_template.c
>> @@ -743,8 +743,11 @@ static int
>> FUNC(quantization_params)(CodedBitstreamContext *ctx, RWContext *rw,
>>   static int FUNC(segmentation_params)(CodedBitstreamContext *ctx,
>> RWContext *rw,
>>                                        AV1RawFrameHeader *current)
>>   {
>> +    CodedBitstreamAV1Context  *priv = ctx->priv_data;
>>       static const uint8_t bits[AV1_SEG_LVL_MAX] = { 8, 6, 6, 6, 6, 3,
>> 0, 0 };
>>       static const uint8_t sign[AV1_SEG_LVL_MAX] = { 1, 1, 1, 1, 1, 0,
>> 0, 0 };
>> +    static const uint8_t default_feature_enabled[AV1_SEG_LVL_MAX] = {
>> 0 };
>> +    static const int16_t default_feature_value[AV1_SEG_LVL_MAX] = { 0 };
>>       int i, j, err;
>>         flag(segmentation_enabled);
>> @@ -763,22 +766,38 @@ static int
>> FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext *rw,
>>               flag(segmentation_update_data);
>>           }
>>   -        if (current->segmentation_update_data) {
>>               for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
>> -                for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
>> -                    flags(feature_enabled[i][j], 2, i, j);
>> +                const uint8_t *feature_enabled;
>> +                const int16_t *feature_value;
> 
> Same comment as previous about ref_feature_enabled etc.
> 
>> +
>> +                if (current->primary_ref_frame ==
>> AV1_PRIMARY_REF_NONE) {
>> +                    feature_enabled = default_feature_enabled;
>> +                    feature_value = default_feature_value;
>> +                } else {
>> +                    feature_enabled =
>> +                       
>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_enabled[i];
>>
>> +                    feature_value =
>> +                       
>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_value[i];
>>
>> +                }
>>   -                    if (current->feature_enabled[i][j] && bits[j] >
>> 0) {
>> -                        if (sign[j])
>> -                            sus(1 + bits[j], feature_value[i][j], 2,
>> i, j);
>> -                        else
>> -                            fbs(bits[j], feature_value[i][j], 2, i, j);
>> +                for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
>> +                    if (current->segmentation_update_data) {
>> +                        flags(feature_enabled[i][j], 2, i, j);
>> +
>> +                        if (current->feature_enabled[i][j] && bits[j]
>> > 0) {
>> +                            if (sign[j])
>> +                                sus(1 + bits[j], feature_value[i][j],
>> 2, i, j);
>> +                            else
>> +                                fbs(bits[j], feature_value[i][j], 2,
>> i, j);
>> +                        } else {
>> +                            infer(feature_value[i][j], 0);
>> +                        }
>>                       } else {
>> -                        infer(feature_value[i][j], 0);
>> +                        infer(feature_enabled[i][j],
>> feature_enabled[j]);
>> +                        infer(feature_value[i][j], feature_value[j]);
>>                       }
>>                   }
>>               }
>> -        }
>>       } else {
>>           for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
>>               for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
>> @@ -1645,6 +1664,10 @@ update_refs:
>>                      sizeof(current->loop_filter_ref_deltas));
>>               memcpy(priv->ref[i].loop_filter_mode_deltas,
>> current->loop_filter_mode_deltas,
>>                      sizeof(current->loop_filter_mode_deltas));
>> +            memcpy(priv->ref[i].feature_enabled,
>> current->feature_enabled,
>> +                   sizeof(current->feature_enabled));
>> +            memcpy(priv->ref[i].feature_value, current->feature_value,
>> +                   sizeof(current->feature_value));
>>           }
>>       }
>>  
> 
> Also sensible.  (Needs the reindent as well, not much value in it being
> separate given that this diff has most lines changed anyway.)
> 
> Thanks,
> 
> - Mark

Added ref_, reindented, and applied.

Thanks
diff mbox series

Patch

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 97aeee9795..a2d78e736f 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -416,6 +416,8 @@  typedef struct AV1ReferenceFrameState {
 
     int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
     int8_t  loop_filter_mode_deltas[2];
+    uint8_t feature_enabled[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
+    int16_t feature_value[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
 } AV1ReferenceFrameState;
 
 typedef struct CodedBitstreamAV1Context {
diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
index 4edf4fd47c..2df5619279 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -743,8 +743,11 @@  static int FUNC(quantization_params)(CodedBitstreamContext *ctx, RWContext *rw,
 static int FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext *rw,
                                      AV1RawFrameHeader *current)
 {
+    CodedBitstreamAV1Context  *priv = ctx->priv_data;
     static const uint8_t bits[AV1_SEG_LVL_MAX] = { 8, 6, 6, 6, 6, 3, 0, 0 };
     static const uint8_t sign[AV1_SEG_LVL_MAX] = { 1, 1, 1, 1, 1, 0, 0, 0 };
+    static const uint8_t default_feature_enabled[AV1_SEG_LVL_MAX] = { 0 };
+    static const int16_t default_feature_value[AV1_SEG_LVL_MAX] = { 0 };
     int i, j, err;
 
     flag(segmentation_enabled);
@@ -763,22 +766,38 @@  static int FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext *rw,
             flag(segmentation_update_data);
         }
 
-        if (current->segmentation_update_data) {
             for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
-                for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
-                    flags(feature_enabled[i][j], 2, i, j);
+                const uint8_t *feature_enabled;
+                const int16_t *feature_value;
+
+                if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
+                    feature_enabled = default_feature_enabled;
+                    feature_value = default_feature_value;
+                } else {
+                    feature_enabled =
+                        priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_enabled[i];
+                    feature_value =
+                        priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_value[i];
+                }
 
-                    if (current->feature_enabled[i][j] && bits[j] > 0) {
-                        if (sign[j])
-                            sus(1 + bits[j], feature_value[i][j], 2, i, j);
-                        else
-                            fbs(bits[j], feature_value[i][j], 2, i, j);
+                for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
+                    if (current->segmentation_update_data) {
+                        flags(feature_enabled[i][j], 2, i, j);
+
+                        if (current->feature_enabled[i][j] && bits[j] > 0) {
+                            if (sign[j])
+                                sus(1 + bits[j], feature_value[i][j], 2, i, j);
+                            else
+                                fbs(bits[j], feature_value[i][j], 2, i, j);
+                        } else {
+                            infer(feature_value[i][j], 0);
+                        }
                     } else {
-                        infer(feature_value[i][j], 0);
+                        infer(feature_enabled[i][j], feature_enabled[j]);
+                        infer(feature_value[i][j], feature_value[j]);
                     }
                 }
             }
-        }
     } else {
         for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
             for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
@@ -1645,6 +1664,10 @@  update_refs:
                    sizeof(current->loop_filter_ref_deltas));
             memcpy(priv->ref[i].loop_filter_mode_deltas, current->loop_filter_mode_deltas,
                    sizeof(current->loop_filter_mode_deltas));
+            memcpy(priv->ref[i].feature_enabled, current->feature_enabled,
+                   sizeof(current->feature_enabled));
+            memcpy(priv->ref[i].feature_value, current->feature_value,
+                   sizeof(current->feature_value));
         }
     }