Skip to content

fix: BE bitfield guards for audio structs and endian-safe wValue/wIndex extraction in UAC2 examples#3618

Open
Copilot wants to merge 5 commits into
masterfrom
copilot/fix-audio-bitfield-order-issues
Open

fix: BE bitfield guards for audio structs and endian-safe wValue/wIndex extraction in UAC2 examples#3618
Copilot wants to merge 5 commits into
masterfrom
copilot/fix-audio-bitfield-order-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 25, 2026

Follow-up to #3597: three big-endian correctness gaps missed by that PR's bitfield and setup-packet fixes.

src/class/audio/audio.h

  • audio10_desc_as_iso_data_ep_t.bmAttributes — added TU_BITFIELD_ORDER LE/BE guards (same xfer/sync/usage packing as tusb_desc_endpoint_t, which was fixed in fix #3596: big-endian host support for SETUP packet handling #3597 but this duplicate was missed). Read at runtime in audio_device.c to classify ISO endpoints; reversed bits on BE would silently misclassify them.

  • audio20_control_request_t.bmRequestType_bit — added TU_BITFIELD_ORDER LE/BE guards mirroring the fix already applied to tusb_control_request_t in tusb_types.h.

UAC2 example files (cdc_uac2, uac2_headset, uac2_speaker_fb)

After #3597 stores wValue/wIndex in host byte order, the alias-cast pattern (audio20_control_request_t const *)p_request breaks on BE — bytes that map to bChannelNumber/bControlSelector/bEntityID/bInterface are swapped relative to the struct layout.

Replaced alias casts with explicit TU_U16_LOW/TU_U16_HIGH extraction, consistent with how audio_device.c already handles these fields:

// Before (BE-broken alias cast)
audio20_control_request_t const *request = (audio20_control_request_t const *) p_request;
if (request->bEntityID == UAC2_ENTITY_CLOCK) { ... }

// After (endian-safe)
uint8_t const entity_id   = TU_U16_HIGH(p_request->wIndex);
uint8_t const ctrl_sel    = TU_U16_HIGH(p_request->wValue);
uint8_t const channel_num = TU_U16_LOW(p_request->wValue);
if (entity_id == UAC2_ENTITY_CLOCK) { ... }

Ref: #3597 (comment)

Original prompt

Implement the remaining big-endian fixes identified in PR review comment #3597 (comment).

Repository: hathach/tinyusb
Target branch: master

Context
PR #3597 introduced host-endian conversion for setup packet fields in dcd_event_setup_received and added bitfield-order guards in some common USB types. A follow-up review identified 3 remaining endian issues in audio-related code and examples.

Tasks

  1. Add TU_BITFIELD_ORDER guards for audio10_desc_as_iso_data_ep_t.bmAttributes in src/class/audio/audio.h.

    • This struct currently defines bitfields:
      • xfer:2, sync:2, usage:2, padding :2
    • Mirror the approach used in src/common/tusb_types.h for endpoint attributes:
      • LE layout: xfer, sync, usage, pad
      • BE layout: pad, usage, sync, xfer
    • Include #if (TU_BITFIELD_ORDER == TU_BITFIELD_LE) ... #elif (TU_BITFIELD_ORDER == TU_BITFIELD_BE) ... #else #error ... #endif pattern consistent with repository style.
  2. Add TU_BITFIELD_ORDER guards for audio20_control_request_t.bmRequestType_bit in src/class/audio/audio.h.

    • Current fields are LE-ordered:
      • recipient:5, type:2, direction:1
    • Add mirrored BE order:
      • direction:1, type:2, recipient:5
    • Use the same #if/#elif/#else #error pattern as above and consistent with tusb_control_request_t in src/common/tusb_types.h.
  3. Fix UAC2 examples that rely on alias-casting/LE byte assumptions for wValue/wIndex subfields.

    • Affected files:
      • examples/device/cdc_uac2/src/uac2_app.c
      • examples/device/uac2_headset/src/main.c
      • examples/device/uac2_speaker_fb/src/main.c
    • Ensure extraction of selector/entity/interface/channel bytes from p_request->wValue and p_request->wIndex is endian-safe after host-order conversion in setup event path.
    • Prefer helpers such as TU_U16_LOW, TU_U16_HIGH (or existing tinyusb helper equivalents) instead of raw alias assumptions.
    • Preserve behavior on little-endian while making big-endian correct.

Validation expectations

  • Build should remain clean for existing examples and core.
  • No unrelated refactors.
  • Keep changes focused and minimal.

Deliverables

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: fix issue in #3597 (comment)

Assistant: I can do that, but I need one clarification before I can make changes:

Do you want me to open a PR with the fix (I can do that directly), or just identify the fix and provide the patch here?

If you want the PR opened, I’ll proceed against hathach/tinyusb and target the appropriate base branch.

User: identify the fix and open PR after my confirmation

Assistant: Perfect — I’ll do this in two steps:

  1. Investigate PR fix #3596: big-endian host support for SETUP packet handling #3597 comment #issuecomment-4320042007 and identify the exact fix
  2. I’ll report back with the proposed change and diff summary, then wait for your explicit confirmation before opening the PR.

User: do it

Assistant: Identified. The comment points out 3 remaining big-endian correctness gaps after PR #3597.

Proposed fix

  1. Guard audio10_desc_as_iso_data_ep_t.bmAttributes bitfields by TU_BITFIELD_ORDER

    • File: src/class/audio/audio.h
    • Why: this bitfield is actively consumed in audio_device.c and will decode incorrectly on BE without mirrored layout.
  2. Guard audio20_control_request_t.bmRequestType_bit bitfields by TU_BITFIELD_ORDER

    • File: src/class/audio/audio.h
    • Why: public UAC2 alias struct currently has LE-only bitfield layout; BE users would get wrong recipient/type/direction.
  3. Fix UAC2 example alias-cast pattern that assumes LE byte placement

    • Files:
      • examples/device/cdc_uac2/src/uac2_app.c
      • examples/device/uac2_headset/src/main.c
      • examples/device/uac2_speaker_fb/src/main.c
    • Why: after setup packet fields are converted to host order in dcd_event_setup_received, aliasing wValue/wIndex bytes directly is wrong on BE.
    • Safer approach: extract via TU_U16_LOW/HIGH(p_request->wValue / wIndex) (or equivalent helper usage), not by raw byte reinterpretation assumptions.

This exactly addresses the three items in:
#3597 (comment)

If you confirm, I’ll open a PR with these changes.

User: yes

Assistant: Great — I’m ready to open it, but I need one repo permission detail from you first:

Please confirm the target repository for the PR is exactly:

  • hathach/tinyusb

(If yes, I’ll pro...

This pull request was created from Copilot chat.

Copilot AI and others added 4 commits April 25, 2026 18:09
…e field extraction

- Add TU_BITFIELD_ORDER guards for audio10_desc_as_iso_data_ep_t.bmAttributes in audio.h
- Add TU_BITFIELD_ORDER guards for audio20_control_request_t.bmRequestType_bit in audio.h
- Fix cdc_uac2/src/uac2_app.c: replace alias cast with TU_U16_LOW/HIGH field extraction
- Fix uac2_headset/src/main.c: replace alias cast with TU_U16_LOW/HIGH field extraction
- Fix uac2_speaker_fb/src/main.c: replace alias cast with TU_U16_LOW/HIGH field extraction

Addresses review comment: #3597 (comment)

Agent-Logs-Url: https://github.com/hathach/tinyusb/sessions/8b695271-74b3-4a26-b3d8-c48ecdf2e481

Co-authored-by: HiFiPhile <4375114+HiFiPhile@users.noreply.github.com>
Co-authored-by: HiFiPhile <4375114+HiFiPhile@users.noreply.github.com>
… tracking

Co-authored-by: HiFiPhile <4375114+HiFiPhile@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix remaining big-endian issues in audio related code fix: BE bitfield guards for audio structs and endian-safe wValue/wIndex extraction in UAC2 examples Apr 25, 2026
Copilot AI requested a review from HiFiPhile April 25, 2026 18:15
@HiFiPhile
Copy link
Copy Markdown
Collaborator

BE support for NET and MSC will be hard...

@HiFiPhile HiFiPhile marked this pull request as ready for review April 26, 2026 10:21
Copilot AI review requested due to automatic review settings April 26, 2026 10:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR completes follow-up big-endian correctness fixes for TinyUSB’s audio class types and UAC2 examples, addressing remaining gaps after #3597’s setup-packet host-endian conversion and common bitfield-order guards.

Changes:

  • Added TU_BITFIELD_ORDER LE/BE guards for UAC1 ISO endpoint bmAttributes packing in audio10_desc_as_iso_data_ep_t.
  • Added TU_BITFIELD_ORDER LE/BE guards for UAC2 audio20_control_request_t.bmRequestType_bit.
  • Updated UAC2 example request parsing to extract wValue/wIndex subfields via TU_U16_LOW/HIGH instead of BE-broken alias casts.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/class/audio/audio.h Adds BE-safe bitfield layouts for duplicated endpoint attributes and UAC2 request-type bitfields.
examples/device/cdc_uac2/src/uac2_app.c Removes alias-cast parsing and uses endian-safe extraction from wValue/wIndex.
examples/device/uac2_headset/src/main.c Makes UAC2 entity/control/channel parsing endian-safe by extracting bytes from wValue/wIndex.
examples/device/uac2_speaker_fb/src/main.c Same endian-safe extraction refactor for UAC2 control handling paths.
.gitignore Adds ignore rules for fetched dependencies (not described in PR scope).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

3 participants