Skip to content

[Depends][Bento4] Cleanups to remove some patches #1654

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

Draft
wants to merge 4 commits into
base: Piers
Choose a base branch
from

Conversation

CastagnaIT
Copy link
Collaborator

@CastagnaIT CastagnaIT commented Aug 24, 2024

Description

Cleanups to remove some patches

i try waiting an answer of the maintainer on axiomatic-systems/Bento4#977
to understand what changes to be included or not on our bento4 repo

Motivation and context

A bit less commits to maintains on our custom bento4

How has this been tested?

Already tested on android also to check DolbyDigital+ bento4 regression (recently fixed)

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

Most rilevant change,
add our implementation of ReadGolomb
in order to remove bento4 custom patch
This allow to remove custom bento4 changes
@CastagnaIT CastagnaIT added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: Depends v22 Piers labels Aug 24, 2024
@CastagnaIT CastagnaIT added the On hold PR that is not currently being worked on (e.g. waiting for a 3rd-party release or feedback) label Aug 24, 2024
@garbear garbear requested a review from Copilot May 25, 2025 06:51
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 cleans up custom patches to reduce commit volume by removing obsolete modifications from the custom Bento4 fork. Key changes include:

  • Adding a new constant (MP4_SAMPLE_FORMAT_WVTT) and a helper class (MP4UnknownUuidAtom) to expose atom data.
  • Refactoring type casting in ParseTrafTfrf from a dynamic_cast to a static_cast.
  • Removing unused member variables from CodecHandler and updating dependency references in the Bento4 configuration files.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

File Description
src/samplereader/FragmentedSampleReader.cpp Introduces MP4_SAMPLE_FORMAT_WVTT and refactors type casting in ParseTrafTfrf.
src/codechandler/CodecHandler.h Cleans up member variables and modernizes the destructor.
src/codechandler/AVCCodecHandler.h & .cpp Adjusts picture ID handling and streamlines slice header parsing logic.
depends/common/bento4/* Updates dependency URLs and checksums for Bento4.

@@ -505,7 +515,7 @@ void CFragmentedSampleReader::UpdateSampleDescription()

void CFragmentedSampleReader::ParseTrafTfrf(AP4_UuidAtom* uuidAtom)
{
const AP4_DataBuffer& buf{AP4_DYNAMIC_CAST(AP4_UnknownUuidAtom, uuidAtom)->GetData()};
const AP4_DataBuffer& buf{static_cast<MP4UnknownUuidAtom*>(uuidAtom)->GetData()};
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.

Consider adding a runtime type check or a static_assert to ensure that 'uuidAtom' is indeed an instance of MP4UnknownUuidAtom before using static_cast. This will help prevent potential issues if the input object is not of the expected type.

Suggested change
const AP4_DataBuffer& buf{static_cast<MP4UnknownUuidAtom*>(uuidAtom)->GetData()};
auto* unknownUuidAtom = dynamic_cast<MP4UnknownUuidAtom*>(uuidAtom);
if (!unknownUuidAtom)
{
LOG::LogF(LOGERROR, "Invalid atom type passed to ParseTrafTfrf. Expected MP4UnknownUuidAtom.");
return;
}
const AP4_DataBuffer& buf{unknownUuidAtom->GetData()};

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
Component: Depends On hold PR that is not currently being worked on (e.g. waiting for a 3rd-party release or feedback) Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v22 Piers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant