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

Update FFmpeg v7.1 #2439

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Update FFmpeg v7.1 #2439

merged 1 commit into from
Oct 23, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 23, 2024

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Update FFmpeg to 7.1 from 7.0.2

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement


Description

  • Updated FFmpeg version from 7.0.2 to 7.1 to ensure the latest features and improvements are included.
  • Changed the FFmpeg based tag to version-7.1-cli for better version tracking.
  • Enhanced platform detection logic to correctly identify arm64 architecture.

Changes walkthrough 📝

Relevant files
Enhancement
Makefile
Update FFmpeg version and platform detection                         

Makefile

  • Updated FFmpeg version from 7.0.2 to 7.1.
  • Changed FFmpeg based tag to version-7.1-cli.
  • Modified platform detection to include arm64.
  • +3/-3     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Signed-off-by: Viet Nguyen Duc <[email protected]>
    @VietND96 VietND96 merged commit 3f3a3fe into trunk Oct 23, 2024
    45 of 46 checks passed
    @VietND96 VietND96 deleted the ffmpeg-update branch October 23, 2024 05:30
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Version Compatibility
    Ensure that the update to FFmpeg 7.1 is compatible with all dependent systems and doesn't introduce any breaking changes.

    Platform Detection
    Verify that the new platform detection logic correctly identifies all supported architectures, including edge cases.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Introduce a variable for the FFmpeg version to improve maintainability and consistency

    Consider using a variable for the FFmpeg version number (7.1) to make future updates
    easier and reduce the risk of inconsistencies.

    Makefile [21-23]

    -FFMPEG_TAG_VERSION := $(or $(FFMPEG_TAG_VERSION),$(FFMPEG_TAG_VERSION),ffmpeg-7.1)
    -FFMPEG_BASED_TAG := $(or $(FFMPEG_BASED_TAG),$(FFMPEG_BASED_TAG),version-7.1-cli)
    +FFMPEG_VERSION := 7.1
    +FFMPEG_TAG_VERSION := $(or $(FFMPEG_TAG_VERSION),$(FFMPEG_TAG_VERSION),ffmpeg-$(FFMPEG_VERSION))
    +FFMPEG_BASED_TAG := $(or $(FFMPEG_BASED_TAG),$(FFMPEG_BASED_TAG),version-$(FFMPEG_VERSION)-cli)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion improves maintainability by centralizing the FFmpeg version number, reducing the risk of inconsistencies and simplifying future updates. It is a practical enhancement that aligns with best practices for managing version numbers in configuration files.

    8
    Enhancement
    Improve architecture detection robustness by using a more widely supported command

    Consider using a more robust method to detect ARM64 architecture, such as using
    uname -m instead of arch, to ensure compatibility across different systems.

    Makefile [24]

    -CURRENT_PLATFORM := $(shell if [ `arch` = "aarch64" ] || [ `arch` = "arm64" ]; then echo "linux/arm64"; else echo "linux/amd64"; fi)
    +CURRENT_PLATFORM := $(shell if [ `uname -m` = "aarch64" ] || [ `uname -m` = "arm64" ]; then echo "linux/arm64"; else echo "linux/amd64"; fi)
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion enhances compatibility by using uname -m instead of arch for architecture detection, which is more widely supported across different systems. This change increases the robustness of platform detection, making it a valuable improvement.

    7

    💡 Need additional feedback ? start a PR chat

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant