diff mbox series

[FFmpeg-devel,2/3] avcodec/aacdec_template: add more checks to make sure only 22.2 gets to 22.2

Message ID 20200818192538.35023-3-jeebjp@gmail.com
State Superseded
Headers show
Series avcodec/aacdec_template: improvements to 22.2 layout logic | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Jan Ekström Aug. 18, 2020, 7:25 p.m. UTC
This way we can check that we have exactly the required things for 22.2.

Fixes #8845
---
 libavcodec/aacdec_template.c | 40 ++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

Comments

Jan Ekström Aug. 21, 2020, 8:54 p.m. UTC | #1
On Tue, Aug 18, 2020 at 10:25 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> This way we can check that we have exactly the required things for 22.2.
>
> Fixes #8845
> ---
>  libavcodec/aacdec_template.c | 40 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
> index 9eef1c0158..1e112ccf76 100644
> --- a/libavcodec/aacdec_template.c
> +++ b/libavcodec/aacdec_template.c
> @@ -263,6 +263,7 @@ static int count_paired_channels(uint8_t (*layout_map)[3], int tags, int pos,
>      return num_pos_channels;
>  }
>
> +#define PREFIX_FOR_22POINT2 (AV_CH_LAYOUT_7POINT1_WIDE_BACK|AV_CH_BACK_CENTER|AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT|AV_CH_LOW_FREQUENCY_2)
>  static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
>  {
>      int i, n, total_non_cc_elements;
> @@ -399,46 +400,78 @@ static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
>      }
>
>      // The previous checks would end up at 8 at this point for 22.2
> -    if (tags == 16 && i == 8) {
> +    if (layout == PREFIX_FOR_22POINT2 && tags == 16 && i == 8) {
> +        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
> +            goto end_of_layout_definition;
> +

After giving this another look, will add syntax element check here as
well (SCE or CPE).

This will then make sure that when assign_pair is called, only
TYPE_CPE elements are passed to it.

>          e2c_vec[i] = (struct elem_to_channel) {
>              .av_position  = AV_CH_TOP_FRONT_CENTER,
>              .syn_ele      = layout_map[i][0],
>              .elem_id      = layout_map[i][1],
>              .aac_position = layout_map[i][2]
>          }; layout |= e2c_vec[i].av_position; i++;
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
> +            goto end_of_layout_definition;
> +
>          i += assign_pair(e2c_vec, layout_map, i,
>                           AV_CH_TOP_FRONT_LEFT,
>                           AV_CH_TOP_FRONT_RIGHT,
>                           AAC_CHANNEL_FRONT,
>                           &layout);
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_SIDE)
> +            goto end_of_layout_definition;
> +
>          i += assign_pair(e2c_vec, layout_map, i,
>                           AV_CH_TOP_SIDE_LEFT,
>                           AV_CH_TOP_SIDE_RIGHT,
>                           AAC_CHANNEL_SIDE,
>                           &layout);
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
> +            goto end_of_layout_definition;
> +
>          e2c_vec[i] = (struct elem_to_channel) {
>              .av_position  = AV_CH_TOP_CENTER,
>              .syn_ele      = layout_map[i][0],
>              .elem_id      = layout_map[i][1],
>              .aac_position = layout_map[i][2]
>          }; layout |= e2c_vec[i].av_position; i++;
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_BACK)
> +            goto end_of_layout_definition;
> +
>          i += assign_pair(e2c_vec, layout_map, i,
>                           AV_CH_TOP_BACK_LEFT,
>                           AV_CH_TOP_BACK_RIGHT,
>                           AAC_CHANNEL_BACK,
>                           &layout);
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_BACK)
> +            goto end_of_layout_definition;
> +
>          e2c_vec[i] = (struct elem_to_channel) {
>              .av_position  = AV_CH_TOP_BACK_CENTER,
>              .syn_ele      = layout_map[i][0],
>              .elem_id      = layout_map[i][1],
>              .aac_position = layout_map[i][2]
>          }; layout |= e2c_vec[i].av_position; i++;
> +
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
> +            goto end_of_layout_definition;
> +
>          e2c_vec[i] = (struct elem_to_channel) {
>              .av_position  = AV_CH_BOTTOM_FRONT_CENTER,
>              .syn_ele      = layout_map[i][0],
>              .elem_id      = layout_map[i][1],
>              .aac_position = layout_map[i][2]
>          }; layout |= e2c_vec[i].av_position; i++;
> +
> +        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
> +            goto end_of_layout_definition;
> +
>          i += assign_pair(e2c_vec, layout_map, i,
>                           AV_CH_BOTTOM_FRONT_LEFT,
>                           AV_CH_BOTTOM_FRONT_RIGHT,
> @@ -446,9 +479,12 @@ static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
>                           &layout);
>      }
>
> +end_of_layout_definition:
> +
>      total_non_cc_elements = n = i;
>
> -    if (tags == 16 && total_non_cc_elements == 16) {
> +    if (layout == AV_CH_LAYOUT_22POINT2 &&
> +        tags == 16 && total_non_cc_elements == 16) {

Will simplify this check to just the layout. The numeric checks are
already taken care of earlier, and this eases reading of the check
since it just checks if the layout has ended up 22.2 (which is what it
ends up at if all the checks of that if statement succeed).

>          // For 22.2 reorder the result as needed
>          FFSWAP(struct elem_to_channel, e2c_vec[2], e2c_vec[0]);   // FL & FR first (final), FC third
>          FFSWAP(struct elem_to_channel, e2c_vec[2], e2c_vec[1]);   // FC second (final), FLc & FRc third
> --
> 2.26.2
>

Jan
diff mbox series

Patch

diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index 9eef1c0158..1e112ccf76 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -263,6 +263,7 @@  static int count_paired_channels(uint8_t (*layout_map)[3], int tags, int pos,
     return num_pos_channels;
 }
 
+#define PREFIX_FOR_22POINT2 (AV_CH_LAYOUT_7POINT1_WIDE_BACK|AV_CH_BACK_CENTER|AV_CH_SIDE_LEFT|AV_CH_SIDE_RIGHT|AV_CH_LOW_FREQUENCY_2)
 static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
 {
     int i, n, total_non_cc_elements;
@@ -399,46 +400,78 @@  static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
     }
 
     // The previous checks would end up at 8 at this point for 22.2
-    if (tags == 16 && i == 8) {
+    if (layout == PREFIX_FOR_22POINT2 && tags == 16 && i == 8) {
+        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
+            goto end_of_layout_definition;
+
         e2c_vec[i] = (struct elem_to_channel) {
             .av_position  = AV_CH_TOP_FRONT_CENTER,
             .syn_ele      = layout_map[i][0],
             .elem_id      = layout_map[i][1],
             .aac_position = layout_map[i][2]
         }; layout |= e2c_vec[i].av_position; i++;
+
+        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
+            goto end_of_layout_definition;
+
         i += assign_pair(e2c_vec, layout_map, i,
                          AV_CH_TOP_FRONT_LEFT,
                          AV_CH_TOP_FRONT_RIGHT,
                          AAC_CHANNEL_FRONT,
                          &layout);
+
+        if (layout_map[i][2] != AAC_CHANNEL_SIDE)
+            goto end_of_layout_definition;
+
         i += assign_pair(e2c_vec, layout_map, i,
                          AV_CH_TOP_SIDE_LEFT,
                          AV_CH_TOP_SIDE_RIGHT,
                          AAC_CHANNEL_SIDE,
                          &layout);
+
+        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
+            goto end_of_layout_definition;
+
         e2c_vec[i] = (struct elem_to_channel) {
             .av_position  = AV_CH_TOP_CENTER,
             .syn_ele      = layout_map[i][0],
             .elem_id      = layout_map[i][1],
             .aac_position = layout_map[i][2]
         }; layout |= e2c_vec[i].av_position; i++;
+
+        if (layout_map[i][2] != AAC_CHANNEL_BACK)
+            goto end_of_layout_definition;
+
         i += assign_pair(e2c_vec, layout_map, i,
                          AV_CH_TOP_BACK_LEFT,
                          AV_CH_TOP_BACK_RIGHT,
                          AAC_CHANNEL_BACK,
                          &layout);
+
+        if (layout_map[i][2] != AAC_CHANNEL_BACK)
+            goto end_of_layout_definition;
+
         e2c_vec[i] = (struct elem_to_channel) {
             .av_position  = AV_CH_TOP_BACK_CENTER,
             .syn_ele      = layout_map[i][0],
             .elem_id      = layout_map[i][1],
             .aac_position = layout_map[i][2]
         }; layout |= e2c_vec[i].av_position; i++;
+
+
+        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
+            goto end_of_layout_definition;
+
         e2c_vec[i] = (struct elem_to_channel) {
             .av_position  = AV_CH_BOTTOM_FRONT_CENTER,
             .syn_ele      = layout_map[i][0],
             .elem_id      = layout_map[i][1],
             .aac_position = layout_map[i][2]
         }; layout |= e2c_vec[i].av_position; i++;
+
+        if (layout_map[i][2] != AAC_CHANNEL_FRONT)
+            goto end_of_layout_definition;
+
         i += assign_pair(e2c_vec, layout_map, i,
                          AV_CH_BOTTOM_FRONT_LEFT,
                          AV_CH_BOTTOM_FRONT_RIGHT,
@@ -446,9 +479,12 @@  static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
                          &layout);
     }
 
+end_of_layout_definition:
+
     total_non_cc_elements = n = i;
 
-    if (tags == 16 && total_non_cc_elements == 16) {
+    if (layout == AV_CH_LAYOUT_22POINT2 &&
+        tags == 16 && total_non_cc_elements == 16) {
         // For 22.2 reorder the result as needed
         FFSWAP(struct elem_to_channel, e2c_vec[2], e2c_vec[0]);   // FL & FR first (final), FC third
         FFSWAP(struct elem_to_channel, e2c_vec[2], e2c_vec[1]);   // FC second (final), FLc & FRc third