Skip to content

fix(export): make a GIF with looping off play only once - #1197

Merged
EtienneLescot merged 1 commit into
mainfrom
fix/gif-play-once
Oct 14, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
fix/gif-play-once

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

A GIF exported with looping off played twice instead of once.

Cause

  • The compositor always wrote the Netscape 2.0 loop block. With looping off it wrote a count of 1, since both the Export dialog and the CLI map gifLoop: false to loopCount: 1.
  • Viewers (Chromium, Firefox, ffmpeg's -loop convention) read that count as repeats after the first play, so 1 meant two plays.

Fix

  • loop_count now means how many times the GIF plays: 0 or absent forever, 1 once, n n times.
  • One function, netscape_repeats, maps it to the block: a count of 0 for forever, no block at all for once, n - 1 for n plays.
  • The TS side already sends 1 for "once", so the dialog, the CLI and MCP are all fixed with no change there.
  • The loopCount doc comments in contracts.ts, addon.d.ts and the napi input now say plays, not loops.
  • The docs (cli.md, the e2e checklist) already promised "plays once"; this makes them true.

Evidence

  • An MCP GIF export with gifLoop: false, on build run 38553956428, carried NETSCAPE2.0 03 01 01 00, a count of 1.

Related issue

Found while verifying the MCP GIF export (#1192) in the e2e pass (#1191).

Type of change

  • Bug fix

Release impact

  • Patch

Desktop impact

  • Not platform-specific

Testing

  • cargo test -p openscreen-compositor --lib gif_export: 21 pass, including a new a_gif_that_plays_once_carries_no_loop_block.
  • Biome passes on the touched TS files.
  • The next e2e pass re-checks an exported GIF has no NETSCAPE block.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • GIF exports now interpret the loop setting as the total number of plays: omission or 0 plays indefinitely, 1 plays once, and higher values play that many times.
  • Documentation
    • Clarified the GIF loop-setting descriptions across the app and its export interfaces to match these playback rules.

The export wrote the Netscape 2.0 loop block with a count of 1 for a GIF with
looping off. Chromium, Firefox and ffmpeg's -loop read that count as repeats
after the first play, so the GIF played twice. loop_count now means how many
times the GIF plays: 0 forever, 1 once (no block), n writes n - 1 repeats.
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0390c141-c9b8-4eda-a3b1-6db46d40189c
📥 Commits

Reviewing files that changed from the base of the PR and between d628682 and 518cabe.

📒 Files selected for processing: 4
  • crates/compositor-view-napi/src/lib.rs
  • crates/compositor/src/gif_export.rs
  • electron/native/compositor-view/addon.d.ts
  • src/native/contracts.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The GIF exporter now converts requested total plays into Netscape repeat counts and omits the loop extension for one play. Rust and TypeScript comments describe the playback-count semantics, and a test covers the mapping.

Changes: GIF playback count handling

Layer / File(s) Summary
Document and encode playback counts
crates/compositor/src/gif_export.rs, crates/compositor-view-napi/src/lib.rs, electron/native/compositor-view/addon.d.ts, src/native/contracts.ts
Loop-count comments define values as total plays. The exporter maps None and 0 to infinite looping, omits the extension for one play, and encodes larger play counts as one fewer repeat. A test checks these mappings.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: my-denia

Merge Risk: ⚪ Minimal · up to 518ca

GIF exports now map requested total plays to the corresponding loop behavior, including playing once without a loop block. No actionable merge risk remains.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary bug fix: GIF exports with looping disabled now play only once.
Description check Passed The description includes a clear summary, cause, fix, affected integrations, bug-fix and patch classifications, platform impact, and testing details. The related issue section does not use the request…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.1)

Clippy execution failed

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant