Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process AAC-LC Specific Config according to the spec #119

Closed
jwcullen opened this issue Aug 9, 2024 · 3 comments
Closed

Process AAC-LC Specific Config according to the spec #119

jwcullen opened this issue Aug 9, 2024 · 3 comments

Comments

@jwcullen
Copy link
Collaborator

jwcullen commented Aug 9, 2024

I noticed two issues with AAC-LC specific config where it does not comply to the referenced ISO+IEC-14496-1:2010.

Issue A: Use the "expandable" fields for DecoderConfigDescriptor and DecooderSpecificInfo.

Unfortunately when investigating the difference between ISO+IEC-14496-1:2010 and ISO+IEC-14496-1-1998
(AOMediaCodec/iamf#338) no one noticed the "expandable" keyword and associated
syntax. This keyword applies to DecoderConfigDescriptor and DecoderSpecificInfo through their inheritance of BaseDescriptor.

§ 8.3.3 defines the use of the "expandable" keyword. It encodes a size in the bitstream using a scheme that appears to be like a big-endian version of leb128.

As of b60e719 this field does not seem to be handled by the decoder and it is missing in the test vectors.

  • Update the decoder to handle this field in both locations.
  • Update the test vectors to use this field.

Issue B: Set DecoderConfigDescriptor reserved field to 1.

This is more trivial because it does not shift any bits around. See #338 which notes DecoderConfigDescriptor has a reserved field fixed at 1.

As of b60e719 the test set has this set incorrectly to 0.

  • Check the decoder is OK when this field is 1.
  • Update the test set to have this field be set to 1.
@tdaede
Copy link
Collaborator

tdaede commented Aug 12, 2024

For both issues, we will regenerate the test vectors for both v1.0.0-errata and v1.1.0 to match the spec. For issue A we will also update the decoders to match.

We will also double check other implementations (e.g. the ffmpeg implementation) to make sure they match the spec and not the old reference implementation.

jwcullen added a commit to AOMediaCodec/iamf-tools that referenced this issue Aug 14, 2024
…use that as a default.

  - As per ISO 14496-1:2010 this field should be fixed to 1.
  - Default it to the correct value at proto level and the class level:
    - Several other fields which typically are fixed in IAMF are assigned sane default values.
  - Enforce that this field is the correct value when reading or writing it.
  - Improve compliance of IAMF spec regarding AOMediaCodec/libiamf#119.

PiperOrigin-RevId: 662918677
jwcullen added a commit to AOMediaCodec/iamf-tools that referenced this issue Aug 14, 2024
…ields.

  - As an analog of `obu_size` it is programmatically calculated and always automatically inserted.
  - Note this field occurs in both the `AacDecoderConfig` and `DecoderSpecificInfo`.
    - Reject bitstreams where this field is too small to parse the known syntax.
    - Read extra bytes into a vector which will forwarded and written out when applicable.
    - Should be future proof and seamlessly passthrough any extensions that do come in the future (based on proper handling described in ISO 14496-1).
  - The implementation writes the section described by the size, then based on exactly what was written it determines the size and copies it with the ISO 14496-1 expanded size prepended.
    - In practice this is a very small data structure (~ 20 bytes without extension), so the overhead should be small.
  - Fixes the encoder issues with AAC (AOMediaCodec/libiamf#119).

PiperOrigin-RevId: 662919133
jwcullen added a commit to jwcullen/libiamf that referenced this issue Aug 14, 2024
…10 spec.

  - Resolves both parts A and B in AOMediaCodec#119 for the test vectors.
    - Part A: Add missing "expandable" size field to both of the `DecoderConfigDescriptor` and `DecoderSpecificInfo`.
    - Part B: Flip `DecoderConfigDescriptor` `reserved` bit to 1.
    - Based on AOMediaCodec/iamf-tools@57324193.
BtbN pushed a commit to FFmpeg/FFmpeg that referenced this issue Aug 31, 2024
Use ff_mp4_read_descr() to read both the tags and the vlc value
that comes after it, which was not being taken into account.

Ref: AOMediaCodec/libiamf#119

Signed-off-by: James Almer <jamrial@gmail.com>
BtbN pushed a commit to FFmpeg/FFmpeg that referenced this issue Aug 31, 2024
Use ff_mp4_read_descr() to read both the tags and the vlc value
that comes after it, which was not being taken into account.

Ref: AOMediaCodec/libiamf#119

Signed-off-by: James Almer <jamrial@gmail.com>
(cherry picked from commit 38bcb3b)
richardpl pushed a commit to librempeg/librempeg that referenced this issue Sep 3, 2024
Use ff_mp4_read_descr() to read both the tags and the vlc value
that comes after it, which was not being taken into account.

Ref: AOMediaCodec/libiamf#119

Signed-off-by: James Almer <jamrial@gmail.com>
Signed-off-by: Paul B Mahol <onemda@gmail.com>
@sunghee-hwang
Copy link
Collaborator

It can be closed as PR #118 have adressed this

@tdaede
Copy link
Collaborator

tdaede commented Sep 3, 2024

It has also been addressed in ffmpeg.

@tdaede tdaede closed this as completed Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants