Skip to content

[VIDEO-3086] Configure gst-plugins-rs source, ref#18

Merged
jawilson merged 6 commits into1.24-lldcfrom
video-3086-configurable-gst-plugins-rs-source
Oct 28, 2024
Merged

[VIDEO-3086] Configure gst-plugins-rs source, ref#18
jawilson merged 6 commits into1.24-lldcfrom
video-3086-configurable-gst-plugins-rs-source

Conversation

@btgoodwin
Copy link
Copy Markdown

@btgoodwin btgoodwin commented Oct 24, 2024

Updated the github action to add recipes_remotes and recipes_commits into the configuration file, pointing gst-plugins-rs at our repository. The target including the following two changes is 0.12.9-lldc.1, here.

  1. [VIDEO-3085] Add extra event forwarding to intersink element gst-plugins-rs#2
  2. [VIDEO-3092] (Resurrected) Backport awss3sink softseek from upstream PR gst-plugins-rs#5

Signed-off-by: Thomas Goodwin <thomas.goodwin@laerdal.com>
@btgoodwin btgoodwin force-pushed the video-3086-configurable-gst-plugins-rs-source branch from a504b93 to 3acc266 Compare October 25, 2024 12:37
This patch release includes our aws3sink w/ cache and intersink changes
@btgoodwin btgoodwin marked this pull request as ready for review October 25, 2024 15:07
@btgoodwin btgoodwin added enhancement New feature or request github-actions Pull requests that update GitHub Actions code needs-code-review Code review needed from developer(s) labels Oct 25, 2024
@btgoodwin btgoodwin requested a review from jawilson October 25, 2024 15:09
@btgoodwin
Copy link
Copy Markdown
Author

I'm stumped. Earlier I let it run with a different existing tag and it was fine. Now it's not with the new tag. Their readme says "branch"...which won't be great; the variable is called commits after all.

The spec for git reset --hard is <commit>, which does accept a remote name but goes to the head of the branch (`remote/branch` format).  If we need to target a tag, our tag must be unique.
@btgoodwin
Copy link
Copy Markdown
Author

Look'n good. It made it past the fetch step @jawilson. Please let me know if this is implemented how you would have preferred. I used a pair of input variables to tie this back. The one for the ref can either be the lldc/<branch name> or a tag that's unique to our repository, as is currently set in this PR.

Copy link
Copy Markdown
Member

@jawilson jawilson left a comment

Choose a reason for hiding this comment

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

We'll need to also update the config-hash calculation. I'd suggest something simple like:

config_hash=$(echo "${CERBERO_ARGS}${{ inputs.config }}${GST_PLUGINS_RS_SOURCE}${GST_PLUGINS_RS_REF}" | sha256sum | cut -d' ' -f1)

@btgoodwin btgoodwin requested a review from jawilson October 28, 2024 16:09
Copy link
Copy Markdown
Member

@jawilson jawilson left a comment

Choose a reason for hiding this comment

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

LGTM assuming the build passes.

@btgoodwin btgoodwin removed the needs-code-review Code review needed from developer(s) label Oct 28, 2024
@btgoodwin
Copy link
Copy Markdown
Author

@jawilson Well, it's through the wickets but I can't merge to the branch because it's protected. I assume you have permission?

@jawilson
Copy link
Copy Markdown
Member

@jawilson Well, it's through the wickets but I can't merge to the branch because it's protected. I assume you have permission?

Got a little carried away with the branch protection rulesets. I couldn't merge either. Updated and good to merge now.

@jawilson jawilson merged commit 4361ec5 into 1.24-lldc Oct 28, 2024
@jawilson jawilson deleted the video-3086-configurable-gst-plugins-rs-source branch October 28, 2024 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request github-actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants