Skip to content

Conversation

CastagnaIT
Copy link
Collaborator

@CastagnaIT CastagnaIT commented Apr 28, 2025

Description

Implement AC4 audio

TODO: check for possible required changes to AudioCodecHandler

Motivation and context

some months ago i seen some changes on kodi core about ac4
i made an attempt to check if it actually works
unfortunately at today kodi use ffmpeg 7.1.1 and looks like there is no AC4 decoder available
since avcodec_find_decoder_by_name dont return the decoder

I don't know if 7.2 will have it, seem also exists unofficial patched ffmpeg with ac4 support,
but i don't think it's worth it, better wait for the official ffmpeg implementation

How has this been tested?

https://ott.dolby.com/OnDelKits/AC-4/Dolby_AC-4_Online_Delivery_Kit_1.5/help_files/topics/kit_wrapper_HLS_multiplexed_streams.html

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • I have read the Contributing document
  • My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

@CastagnaIT CastagnaIT added On hold PR that is not currently being worked on (e.g. waiting for a 3rd-party release or feedback) Type: Feature non-breaking change which adds functionality labels Apr 28, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for AC4 audio by extending codec name/fourcc lists, introducing an AC4 parser, updating the ADTS demuxer, and ensuring sessions recognize AC4 streams.

  • Add NAME_AC4 and FOURCC_AC_4 constants to Utils.h
  • Introduce CAdaptiveAc4Parser and its FindFrameHeader implementation
  • Extend ADTSReader to parse AC4 frames and update Session to handle AC4

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

File Description
src/utils/Utils.h Added AC4 to codec name and fourcc lists
src/parser/CodecParser.h/.cpp Added CAdaptiveAc4Parser class and header parsing
src/demuxers/ADTSReader.h/.cpp Added AC4 parsing methods and case handling
src/Session.cpp Updated stream update logic to recognize AC4
Comments suppressed due to low confidence (1)

src/parser/CodecParser.h:48

  • No unit tests cover CAdaptiveAc4Parser or AC4 frame parsing; adding tests would help ensure correct behavior and prevent regressions.
class ATTR_DLL_LOCAL CAdaptiveAc4Parser : public AP4_Ac4Parser

if (!AP4_SUCCEEDED(stream->Read(buffer.UseData(), AP4_AC3_HEADER_SIZE)))
return false;

CAdaptiveAc3Parser parser;
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

The AC4 header parser is using CAdaptiveAc3Parser; this should be CAdaptiveAc4Parser to correctly parse AC4 frames.

Suggested change
CAdaptiveAc3Parser parser;
CAdaptiveAc4Parser parser;

Copilot uses AI. Check for mistakes.

Comment on lines +282 to +284
buffer.SetDataSize(AP4_AC3_HEADER_SIZE);

if (!AP4_SUCCEEDED(stream->Read(buffer.UseData(), AP4_AC3_HEADER_SIZE)))
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

Parsing AC4 headers should use AP4_AC4_HEADER_SIZE instead of AP4_AC3_HEADER_SIZE to allocate the correct buffer size.

Suggested change
buffer.SetDataSize(AP4_AC3_HEADER_SIZE);
if (!AP4_SUCCEEDED(stream->Read(buffer.UseData(), AP4_AC3_HEADER_SIZE)))
buffer.SetDataSize(AP4_AC4_HEADER_SIZE);
if (!AP4_SUCCEEDED(stream->Read(buffer.UseData(), AP4_AC4_HEADER_SIZE)))

Copilot uses AI. Check for mistakes.

AP4_Size sz = buffer.GetDataSize();
parser.Feed(buffer.GetData(), &sz);

AP4_Ac3Frame frame;
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

You’re instantiating an AP4_Ac3Frame for AC4 parsing; this should be an AP4_Ac4Frame to match the AC4 parser.

Copilot uses AI. Check for mistakes.


unsigned char* rawframe = new unsigned char[sync_frame_size];

// copy the whole frame becasue toc size is unknown
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

Fix typo: replace “becasue” with “because”.

Suggested change
// copy the whole frame becasue toc size is unknown
// copy the whole frame because toc size is unknown

Copilot uses AI. Check for mistakes.

frame.m_Info.m_Ac4Dsi.d.v1.b_uuid = ac4_header.m_BProgramUuidPresent;
AP4_CopyMemory(frame.m_Info.m_Ac4Dsi.d.v1.program_uuid, ac4_header.m_ProgramUuid, 16);

// Calcuate the bit rate mode according to ETSI TS 103 190-2 V1.2.1 Annex B
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

Fix typo: replace “Calcuate” with “Calculate”.

Suggested change
// Calcuate the bit rate mode according to ETSI TS 103 190-2 V1.2.1 Annex B
// Calculate the bit rate mode according to ETSI TS 103 190-2 V1.2.1 Annex B

Copilot uses AI. Check for mistakes.

{
public:
CAdaptiveAc4Parser() {}
AP4_Result FindFrameHeader(AP4_Ac4Frame& frame);
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] Mark this method as override to clarify that it overrides the base class implementation.

Suggested change
AP4_Result FindFrameHeader(AP4_Ac4Frame& frame);
AP4_Result FindFrameHeader(AP4_Ac4Frame& frame) override;

Copilot uses AI. Check for mistakes.

return AP4_ERROR_NOT_ENOUGH_DATA;
}

unsigned char* rawframe = new unsigned char[sync_frame_size];
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using a std::vector<unsigned char> or smart pointer instead of manual new[]/delete[] to manage frame buffers.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On hold PR that is not currently being worked on (e.g. waiting for a 3rd-party release or feedback) Type: Feature non-breaking change which adds functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant