Skip to content

Fix encapsulated frame index offsets#33

Merged
ThalesMMS merged 1 commit intocompression-supportfrom
codex/fix-comments-in-frameindex.swift
Sep 15, 2025
Merged

Fix encapsulated frame index offsets#33
ThalesMMS merged 1 commit intocompression-supportfrom
codex/fix-comments-in-frameindex.swift

Conversation

@ThalesMMS
Copy link
Owner

Summary

  • iterate the Basic Offset Table returned by PixelSequence.basicOffsetTable() as an array of byte offsets
  • derive frame lengths from the concatenated fragment size so the final frame uses the full codestream length
  • keep the single-frame fallback when the BOT is empty

Testing

  • swift test (fails: os.signpost module unavailable on Linux)

https://chatgpt.com/codex/tasks/task_e_68c880d39cc8832ea81c05db620c0dca

@ThalesMMS ThalesMMS merged commit 11a6fe7 into compression-support Sep 15, 2025
1 check failed
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

Codex Review: Here are some suggestions.

Reply with @codex fix comments to fix any unresolved comments.

About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

Comment on lines +94 to 101
let fragments = pixelSequence.items.dropFirst() // Skip BOT item
let totalFragmentLength = fragments.reduce(0) { $0 + ($1.data?.count ?? 0) }

// Get BOT (Basic Offset Table) offsets from first item
if let offsets = pixelSequence.basicOffsetTable(), !offsets.isEmpty {
guard totalFragmentLength > 0 else {
throw FrameIndexError.noFramesFound
}

Choose a reason for hiding this comment

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

[P1] Derive encapsulated size from fragment lengths instead of Data buffers

The new implementation computes totalFragmentLength by summing ($1.data?.count ?? 0). When encapsulated pixel data is read in streaming mode (readPixelSequence with a handler), each fragment’s data remains nil even though its length was parsed from the file. In that common path the total becomes 0, causing buildEncapsulatedFrameIndex to throw noFramesFound even though the Basic Offset Table contains valid frame offsets. The previous version did not depend on fragments being materialized. Consider summing item.length (or falling back to it when data is nil) so streamed pixel data can still be indexed.

Useful? React with 👍 / 👎.

@ThalesMMS ThalesMMS deleted the codex/fix-comments-in-frameindex.swift branch September 15, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant