feat: support raw telephony codecs and additional container formats#31
Merged
Conversation
Bumps org.jetbrains.intellij.platform from 2.14.0 to 2.16.0. --- updated-dependencies: - dependency-name: org.jetbrains.intellij.platform dependency-version: 2.16.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Adds PCMU/PCMA/G.722/GSM/SLN raw audio (with explicit demuxer hints for headerless streams) plus AMR, AU, and MPEG-TS/3GP video extensions.
📝 WalkthroughWalkthroughAdds RawAudioHint and RAW_AUDIO_HINTS to configure FFmpegFrameGrabber for headerless/raw audio; refactors MediaTranscoder frame-loop and progress; expands plugin.xml media extensions and adds a test; bumps intelliJPlatform in the Gradle catalog; makes plugin verification run per-IDE via discovery and Gradle pinning. ChangesRaw audio codec support
Plugin verification per-IDE
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt`:
- Around line 41-49: RAW_AUDIO_HINTS in MediaTranscoder.kt duplicates extension
keys that should be maintained in plugin.xml; instead of hardcoding the map,
remove the literal RAW_AUDIO_HINTS map and load the audio hint data from the
central extension registry (or parse plugin.xml) at runtime—use the existing
RawAudioHint data class and a helper method (e.g., in MediaTranscoder or a new
provider) to look up hints by extension name so the code references the
canonical source of supported extensions rather than duplicating entries.
- Around line 56-63: The FFmpeg native resources can leak because
FFmpegFrameGrabber.start() and recorder.start() may throw before
stop()/release() are called; update MediaTranscoder to allocate grabber and
recorder, then call grabber.start() and recorder.start() inside try blocks with
corresponding finally blocks that always call grabber.stop()/grabber.release()
and recorder.stop()/recorder.release() (guarded by null and/or "started"
booleans) so cleanup runs even on startup exceptions; apply the same safe
lifecycle pattern around the code paths that currently use FFmpegFrameGrabber
(grabber) and the recorder (recorder) in the method to ensure native resources
are freed on error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b10d3fa-e93e-45a3-87cd-6876b3b775ae
📒 Files selected for processing (2)
src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktsrc/main/resources/META-INF/plugin.xml
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Wrap grabber/recorder lifecycle in a single try/finally with started flags so stop()/release() always run even if start() throws. Extract helpers (applyRawHints, buildRecorder, runFrameLoop, reportProgress, safely) to remove nested try/finally and keep transcode() at two indent levels.
Contributor
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2026.1.0
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
Reads the plugin.xml resource at test time and fails the build if any raw-audio hint extension is missing from the registered media file type, catching drift between the two files before it ships.
….0 (#29) Merged to pick up newer IntelliJ Plugin Verifier, which addresses ClosedFileSystemException races seen in verifyPlugin on 2.14.0.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt (1)
53-56: ⚡ Quick winMake the plugin.xml assertion tolerant of formatting-only changes.
This regex only matches
extensions="..."with no surrounding whitespace, and the parsed tokens are not trimmed. A simple XML reformat can fail the test even when the registered extensions are unchanged.Suggested fix
- val match = Regex("""extensions="([^"]+)"""").find(xml) + val match = Regex("""extensions\s*=\s*"([^"]+)"""").find(xml) ?: error("Could not find extensions attribute in plugin.xml") - val registered = match.groupValues[1].split(";").map { it.lowercase() }.toSet() + val registered = match.groupValues[1] + .split(";") + .map { it.trim().lowercase() } + .filter { it.isNotEmpty() } + .toSet()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt` around lines 53 - 56, The test in MediaTranscoderTest reads the plugin.xml into variable xml and uses Regex("""extensions="([^"]+)"""") which fails on formatting changes and leaves tokens untrimmed; update the pattern to allow optional whitespace around the equals and quotes (e.g. use Regex("""extensions\s*=\s*"([^"]*)" """) or parse the attribute with a lightweight XML/HTML parser), then split the captured group, trim each token, filter out empty strings and lowercase them before building the registered set so the assertion is robust to formatting-only changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt`:
- Around line 130-146: The indeterminate-duration branch in reportProgress never
emits the initial onProgress(-1.0) because lastReportedTenth is initialized to
INDETERMINATE_TENTH and the branch checks the opposite; change the
totalMicroseconds <= 0 branch in reportProgress to emit onProgress(-1.0) when
lastReportedTenth == INDETERMINATE_TENTH (i.e., first time seeing an unknown
duration) and then return a different sentinel (e.g.,
REPORTED_INDETERMINATE_TENTH or any non-INDETERMINATE value) so subsequent calls
don't repeat the emission; update or add the sentinel constant and use it in the
caller loop that stores the returned value from reportProgress
(lastReportedTenth) to prevent duplicate indeterminate notifications.
---
Nitpick comments:
In `@src/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt`:
- Around line 53-56: The test in MediaTranscoderTest reads the plugin.xml into
variable xml and uses Regex("""extensions="([^"]+)"""") which fails on
formatting changes and leaves tokens untrimmed; update the pattern to allow
optional whitespace around the equals and quotes (e.g. use
Regex("""extensions\s*=\s*"([^"]*)" """) or parse the attribute with a
lightweight XML/HTML parser), then split the captured group, trim each token,
filter out empty strings and lowercase them before building the registered set
so the assertion is robust to formatting-only changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d01ebb8c-76c8-4634-9d00-8073c233a002
📒 Files selected for processing (3)
gradle/libs.versions.tomlsrc/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktsrc/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt
✅ Files skipped from review due to trivial changes (1)
- gradle/libs.versions.toml
reportProgress never emitted onProgress(-1.0) for unknown-duration inputs because lastReportedTenth started at INDETERMINATE_TENTH and the branch only emitted when it wasn't. Flip the check and return a distinct REPORTED_INDETERMINATE_TENTH sentinel so the notification fires once on first sight and never repeats. Also widen the plugin.xml extensions regex to tolerate whitespace around '=' and trim/filter token splits, so the registration test isn't sensitive to formatting-only edits.
The single-runner verifyPlugin job spawned all recommended() IDEs as parallel workers under one verifier process, and a transient failure in any worker would interrupt the others mid-JAR-read, surfacing as ClosedByInterruptException cascades that obscure the real cause. Split it into a discover-ides job that calls printProductsReleases (via .github/scripts/list-verifier-ides.sh, which emits the same recommended() set as a JSON array) and a matrix verify job that pins each runner to a single IDE through -PverifierIde. With fail-fast: false, one IDE's failure no longer hides the others, and the shared JAR-filesystem cache can't cascade across runners.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/build.yml (2)
322-322: 💤 Low valueArtifact names must comply with GitHub Actions restrictions.
The artifact name includes
${{ matrix.ide }}(e.g.,pluginVerifier-IU-2025.2.6.2). GitHub Actions artifact names cannot contain:,<,>,|,*,?,\,/, or control characters. The TYPE-VERSION format with dots and dashes should be safe, but if the discovery script ever returns IDEs with different formats, uploads could fail.🛡️ Sanitize artifact names
with: - name: pluginVerifier-${{ matrix.ide }} + name: pluginVerifier-${{ matrix.ide }}-report path: ${{ github.workspace }}/build/reports/pluginVerifierOr explicitly sanitize:
- name: Sanitize IDE name for artifact id: sanitize run: echo "ide_safe=$(echo '${{ matrix.ide }}' | tr -cd '[:alnum:].-')" >> "$GITHUB_OUTPUT" - name: Collect Plugin Verifier Result uses: actions/upload-artifact@v6 with: name: pluginVerifier-${{ steps.sanitize.outputs.ide_safe }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml at line 322, The artifact name uses pluginVerifier-${{ matrix.ide }} which can include illegal characters; add a sanitization step before the upload to produce a safe variable (e.g., step id "sanitize" producing outputs.ide_safe) that strips disallowed chars (allow only alphanumerics, dots, dashes) and then change the upload-artifact name to pluginVerifier-${{ steps.sanitize.outputs.ide_safe }} so all artifact uploads use the sanitized IDE name.
271-274: ⚡ Quick winConsider guarding against empty IDE arrays.
If the discovery script returns an empty JSON array
[], the matrix job will skip all verification runs silently. While this might be intentional, it could mask configuration issues.🛡️ Add validation for empty IDE list
You could add a validation step to the
discover-idesjob after the list step:- name: List verifier IDEs id: list run: echo "ides=$(.github/scripts/list-verifier-ides.sh)" >> "$GITHUB_OUTPUT" - name: Validate IDE list run: | IDES='${{ steps.list.outputs.ides }}' if [ "$(echo "$IDES" | jq '. | length')" -eq 0 ]; then echo "::error::No IDEs discovered for verification" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 271 - 274, The matrix uses needs.discover-ides.outputs.ides to populate the ide matrix but doesn't guard against an empty array; update the discover-ides job (the step that sets ides, e.g., step id "list" / the "List verifier IDEs" step) to add a validation step after producing the ides output that parses steps.list.outputs.ides (with jq or similar) and fails the job with a clear error if the array length is 0 so the workflow does not silently skip all matrix runs.build.gradle.kts (1)
102-109: ⚡ Quick winValidate that type and version are non-empty after splitting.
The current validation checks that the split produces exactly two parts but doesn't verify that neither part is blank. If a user provides
verifierIde=IU-orverifierIde=-2025.2, the validation passes butcreate(type, version)will likely fail with a less clear error message.♻️ Enhanced validation
val pinned = providers.gradleProperty("verifierIde").orNull if (pinned.isNullOrBlank()) { recommended() } else { val (type, version) = pinned.split("-", limit = 2) - .also { require(it.size == 2) { "verifierIde must be 'TYPE-VERSION' (e.g. IU-2025.2.6.2), got '$pinned'" } } + .also { + require(it.size == 2) { "verifierIde must be 'TYPE-VERSION' (e.g. IU-2025.2.6.2), got '$pinned'" } + require(it[0].isNotBlank() && it[1].isNotBlank()) { + "verifierIde type and version must be non-empty, got '$pinned'" + } + } create(type, version) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build.gradle.kts` around lines 102 - 109, After splitting pinned from providers.gradleProperty("verifierIde") into (type, version), validate that both type and version are non-empty and not blank before calling create(type, version) (currently in the branch that handles pinned). If either is blank, throw a clear IllegalArgumentException (or use require) with a message like "verifierIde must be 'TYPE-VERSION' with non-empty TYPE and VERSION, got '<pinned>'"; otherwise call create(type, version). Ensure recommended() behavior remains unchanged when pinned is blank.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/build.yml:
- Line 322: The artifact name uses pluginVerifier-${{ matrix.ide }} which can
include illegal characters; add a sanitization step before the upload to produce
a safe variable (e.g., step id "sanitize" producing outputs.ide_safe) that
strips disallowed chars (allow only alphanumerics, dots, dashes) and then change
the upload-artifact name to pluginVerifier-${{ steps.sanitize.outputs.ide_safe
}} so all artifact uploads use the sanitized IDE name.
- Around line 271-274: The matrix uses needs.discover-ides.outputs.ides to
populate the ide matrix but doesn't guard against an empty array; update the
discover-ides job (the step that sets ides, e.g., step id "list" / the "List
verifier IDEs" step) to add a validation step after producing the ides output
that parses steps.list.outputs.ides (with jq or similar) and fails the job with
a clear error if the array length is 0 so the workflow does not silently skip
all matrix runs.
In `@build.gradle.kts`:
- Around line 102-109: After splitting pinned from
providers.gradleProperty("verifierIde") into (type, version), validate that both
type and version are non-empty and not blank before calling create(type,
version) (currently in the branch that handles pinned). If either is blank,
throw a clear IllegalArgumentException (or use require) with a message like
"verifierIde must be 'TYPE-VERSION' with non-empty TYPE and VERSION, got
'<pinned>'"; otherwise call create(type, version). Ensure recommended() behavior
remains unchanged when pinned is blank.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff352a2f-5dd3-43ac-b054-82ebd014b199
📒 Files selected for processing (3)
.github/scripts/list-verifier-ides.sh.github/workflows/build.ymlbuild.gradle.kts
✅ Files skipped from review due to trivial changes (1)
- .github/scripts/list-verifier-ides.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds PCMU/PCMA/G.722/GSM/SLN raw audio (with explicit demuxer hints for headerless streams) plus AMR, AU, and MPEG-TS/3GP video extensions.
Summary by CodeRabbit