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

[Jan Ekström]

On Sat, Aug 22, 2020 at 2:17 PM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Sat, Aug 22, 2020 at 12:57:59AM +0300, Jan Ekström wrote:
> > This way we can check that we have exactly the required things for 22.2.
> >
> > Fixes #8845
> > ---
> >  libavcodec/aacdec_template.c | 47 ++++++++++++++++++++++++++++++++++--
> >  1 file changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
> > index 9f7016790e..63604d39fd 100644
> > --- a/libavcodec/aacdec_template.c
> > +++ b/libavcodec/aacdec_template.c
> > @@ -266,6 +266,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;
> > @@ -402,46 +403,86 @@ 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][0] != TYPE_SCE ||
> > +            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][0] != TYPE_CPE ||
> > +            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][0] != TYPE_CPE ||
> > +            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][0] != TYPE_SCE ||
> > +            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][0] != TYPE_CPE ||
> > +            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][0] != TYPE_SCE ||
> > +            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][0] != TYPE_SCE ||
> > +            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][0] != TYPE_CPE ||
> > +            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,
> > @@ -449,9 +490,11 @@ static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
> >                           &layout);
> >      }
> >
> > +end_of_layout_definition:
> > +
> >      total_non_cc_elements = n = i;
>
> Probably ok if you intend for partial matches to be accepted partially,
> because i think this would accept whatever comes before a mismatch but
> not after, but maybe iam missing something
>

Yes, it will partially match things that get this far - although to
get to this point to begin with you need to get 12/12 right and have
the exact correct amount of tags and matches before that. Not to
mention that a partial match will never end up in the 22.2 specific
re-ordering logic.

I can switch this to a really long if statement that goes through the
next X channel coding elements if that's really required. For the
original reasoning on this, I was just following the "check before
assigning" style that was utilized earlier in the function, trying to
mimic the same same style :P .

Jan