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

feat: Add xHE-AAC support #1092

Merged
merged 2 commits into from
Oct 18, 2022
Merged

feat: Add xHE-AAC support #1092

merged 2 commits into from
Oct 18, 2022

Conversation

geoffjukes
Copy link
Contributor

Note:

  • An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
    value to the allowed grid where SAP/RAPs can occur.
    e.g.: -rapInterval 5000 (5s) may result in actual SAPs/RAPs every 4.984s.
  • To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
    "--segment_duration" is less than or equal to that adjusted value.
  • If every SAP/RAP should trigger a new segment, just set the segment
    length to a very low value e.g.: --segment_duration 0.1

Note:
* An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
  value to the allowed grid where SAP/RAPs can occur.
  e.g.: `-rapInterval 5000` (5s) may result in actual SAPs/RAPs every 4.984s.
* To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
  "--segment_duration" is less than or equal to that adjusted value.
* If every SAP/RAP should trigger a new segment, just set the segment
  length to a very low value e.g.: `--segment_duration 0.1`
@geoffjukes geoffjukes changed the title Add xHE-AAC support feat: Add xHE-AAC support Aug 31, 2022
@cosmin
Copy link
Contributor

cosmin commented Sep 7, 2022

I verified that this patch does package xhe-aac sample files from https://www2.iis.fraunhofer.de/AAC/xhe-aac-abr.html whereas without this patch the packaging errors.

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good otherwise. Thanks!

packager/media/codecs/aac_audio_specific_config.cc Outdated Show resolved Hide resolved
packager/media/codecs/aac_audio_specific_config.cc Outdated Show resolved Hide resolved
Note:
* An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
  value to the allowed grid where SAP/RAPs can occur.
  e.g.: `-rapInterval 5000` (5s) may result in actual SAPs/RAPs every 4.984s.
* To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
  "--segment_duration" is less than or equal to that adjusted value.
* If every SAP/RAP should trigger a new segment, just set the segment
  length to a very low value e.g.: `--segment_duration 0.1`
@geoffjukes
Copy link
Contributor Author

@joeyparrish Did the changes come through. TBH I've not often contributed to projects, and so I'm not sure I did it correctly.

@geoffjukes
Copy link
Contributor Author

@joeyparrish Apologies for bugging you. Is there a timeline for when this might get merged (assuming it's acceptable)? I have a downstream vendor that depends on the Shaka packager, and we can't use their service until it has xHE-AAC support.

@cosmin
Copy link
Contributor

cosmin commented Oct 6, 2022

It does look like the requested changes have been addressed. +1 for merging this.

@geoffjukes
Copy link
Contributor Author

@joeyparrish Feedback from a downstream application that uses Shaka Packager is that it works for them, but they are unable to merge the patch with their code until this PR is merged.

@joeyparrish
Copy link
Member

Apologies for dropping this. I'll review tomorrow (Tuesday, US/Pacific time).

@joeyparrish joeyparrish merged commit 5d998fc into shaka-project:main Oct 18, 2022
joeyparrish pushed a commit that referenced this pull request Oct 18, 2022
Note:
* An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
  value to the allowed grid where SAP/RAPs can occur.
e.g.: `-rapInterval 5000` (5s) may result in actual SAPs/RAPs every
4.984s.
* To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
  "--segment_duration" is less than or equal to that adjusted value.
* If every SAP/RAP should trigger a new segment, just set the segment
  length to a very low value e.g.: `--segment_duration 0.1`
@geoffjukes geoffjukes deleted the xHE-AAC-Support branch October 18, 2022 18:02
sr1990 pushed a commit to sr1990/shaka-packager that referenced this pull request Feb 18, 2023
Note:
* An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
  value to the allowed grid where SAP/RAPs can occur.
e.g.: `-rapInterval 5000` (5s) may result in actual SAPs/RAPs every
4.984s.
* To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
  "--segment_duration" is less than or equal to that adjusted value.
* If every SAP/RAP should trigger a new segment, just set the segment
  length to a very low value e.g.: `--segment_duration 0.1`
sr1990 pushed a commit to sr1990/shaka-packager that referenced this pull request Feb 18, 2023
Note:
* An xHE-AAC capable encoder will auto adjust the user-specified SAP/RAP
  value to the allowed grid where SAP/RAPs can occur.
e.g.: `-rapInterval 5000` (5s) may result in actual SAPs/RAPs every
4.984s.
* To ensure SAP/RAP starts a new segment, Shaka needs to executed with a
  "--segment_duration" is less than or equal to that adjusted value.
* If every SAP/RAP should trigger a new segment, just set the segment
  length to a very low value e.g.: `--segment_duration 0.1`
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants