Skip to content

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Aug 25, 2025

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

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

  • Update FFmpeg from version 7.1.1 to 8.0

  • Update rclone from version 1.70.3 to 1.71.0

  • Update Makefile configuration for new FFmpeg version


Diagram Walkthrough

flowchart LR
  A["FFmpeg 7.1.1"] --> B["FFmpeg 8.0"]
  C["rclone 1.70.3"] --> D["rclone 1.71.0"]
  E["Makefile config"] --> F["Updated FFmpeg tags"]
Loading

File Walkthrough

Relevant files
Dependencies
Dockerfile
Update FFmpeg and rclone versions                                               

.ffmpeg/Dockerfile

  • Update FFMPEG_VERSION from "7.1.1" to "8.0"
  • Update RCLONE_VER from "v1.70.3" to "v1.71.0"
+2/-2     
Configuration changes
Makefile
Update FFmpeg configuration variables                                       

Makefile

  • Update FFMPEG_VERSION default from 7.1 to 8.0
  • Update FFMPEG_TAG_VERSION from ffmpeg-7.1 to ffmpeg-8.0
+2/-2     

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 merged commit e517467 into trunk Aug 25, 2025
25 of 29 checks passed
@VietND96 VietND96 deleted the ffmpeg-8.0 branch August 25, 2025 07:17
Copy link
Contributor

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

Default Version Logic

The use of $(or $(FFMPEG_VERSION),$(FFMPEG_VERSION),8.0) redundantly references the same variable twice and can mask empty values coming from the environment/shell. Confirm that the intent is to fall back to 8.0 only when both Make variable and environment are unset; consider simplifying to a clearer pattern to avoid confusion.

FFMPEG_VERSION := $(or $(FFMPEG_VERSION),$(FFMPEG_VERSION),8.0)
FFMPEG_TAG_PREV_VERSION := $(or $(FFMPEG_TAG_PREV_VERSION),$(FFMPEG_TAG_PREV_VERSION),ffmpeg-7.1)
Tag Consistency

FFMPEG_TAG_VERSION default updated to ffmpeg-8.0 but FFMPEG_TAG_PREV_VERSION remains ffmpeg-7.1. Verify downstream logic expects this pairing and that any consumers are prepared for the new tag naming to avoid pulling/building mismatched images.

FFMPEG_TAG_VERSION := $(or $(FFMPEG_TAG_VERSION),$(FFMPEG_TAG_VERSION),ffmpeg-8.0)
FFMPEG_BASED_NAME := $(or $(FFMPEG_BASED_NAME),$(FFMPEG_BASED_NAME),selenium)
Version Bump Validation

Upgrading to FFmpeg 8.0 and rclone v1.71.0 may require additional build flags or dependency updates. Ensure the rest of the Dockerfile and build scripts are compatible and that CI builds multi-arch images successfully with these versions.

ARG FFMPEG_VERSION="8.0"
ARG RCLONE_VER="v1.71.0"

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Simplify default variable fallback

Avoid redundant self-references in defaults which can cause confusing evaluation
and masking of unset values. Use a single environment override with a clear
hardcoded fallback.

Makefile [22]

-FFMPEG_VERSION := $(or $(FFMPEG_VERSION),$(FFMPEG_VERSION),8.0)
+FFMPEG_VERSION := $(or $(FFMPEG_VERSION),8.0)
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: This is a valid code simplification that improves readability by removing the redundant self-reference in the $(or ...) function without changing the logic.

Low
  • More

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