Skip to content

Allow setting the rotation sidedata via new FFMPEG APIs#2287

Merged
WyattBlue merged 2 commits into
PyAV-Org:mainfrom
hmaarrfk:allow_setting_rotation_sidedata
Jun 7, 2026
Merged

Allow setting the rotation sidedata via new FFMPEG APIs#2287
WyattBlue merged 2 commits into
PyAV-Org:mainfrom
hmaarrfk:allow_setting_rotation_sidedata

Conversation

@hmaarrfk
Copy link
Copy Markdown
Contributor

@hmaarrfk hmaarrfk commented Jun 6, 2026

It would be great to be able to set this rotation side data

@hmaarrfk @claude
Allow setting the display (rotation) matrix on output streams
508a752 
Expose two methods on Stream:

- set_display_matrix(matrix): write a raw 9-integer AV_PKT_DATA_DISPLAYMATRIX
  matrix (16.16 fixed point, 2.30 for the third column).
- set_display_rotation(degrees, hflip=False, vflip=False): build the matrix
  with av_display_rotation_set() / av_display_matrix_flip(); the angle is
  counter-clockwise, matching VideoFrame.rotation on read.

The matrix is written as coded side data on the output stream's codecpar
inside _finalize_for_output() (after avcodec_parameters_from_context, which
would otherwise overwrite it), so muxers record it in the container -- e.g.
the MP4/MOV tkhd transformation matrix.

Adds the required libav bindings (AVCodecParameters.coded_side_data,
AV_PKT_DATA_DISPLAYMATRIX, av_packet_side_data_new, av_display_rotation_set,
av_display_matrix_flip), .pyi stubs, a CHANGELOG entry, and tests covering
all 8 EXIF orientations across several codecs.

Addresses PyAV-Org#1045 and PyAV-Org#1012.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hmaarrfk hmaarrfk force-pushed the allow_setting_rotation_sidedata branch from 41b6977 to 508a752 Compare June 6, 2026 18:34
@hmaarrfk hmaarrfk marked this pull request as ready for review June 6, 2026 18:38
@hmaarrfk hmaarrfk changed the title WIP: Allow setting the rotation sidedata via new FFMPEG APIs Allow setting the rotation sidedata via new FFMPEG APIs Jun 6, 2026
@WyattBlue
Simplify display matrix API and move it to VideoStream
c2b88dd 
Collapse the two stream fields into a single packed-bytes field by
building the rotation matrix eagerly in set_display_rotation(), and move
the field and both setters from Stream to VideoStream so audio/subtitle
streams no longer expose them.
@WyattBlue
Copy link
Copy Markdown
Member

WyattBlue commented Jun 7, 2026

Thanks for this! I pushed a cleanup commit (c2b88dd) on top:

  • Moved set_display_matrix/set_display_rotation (and the storage) from the base Stream to VideoStream, so audio/subtitle/data streams no longer expose them.
  • Collapsed the two cached fields into one: set_display_rotation now builds the matrix eagerly, so there's a single store and one code path in _apply_display_matrix (no more rotation-vs-matrix branch / mutual-exclusion bookkeeping).
  • Store it as a C array (int32_t _display_matrix[9] + a uint8_t set flag) instead of a bytes object — no heap allocation, helpers write straight into the member, and _apply is a single memcpy.

All of tests/test_display_matrix.py still passes, plus the encode/decode/streams suites, and make lint is clean. Let me know if anything looks off.

@hmaarrfk
Copy link
Copy Markdown
Contributor Author

hmaarrfk commented Jun 7, 2026

thanks much cleaner, i really appreciate your quick review on this one.

As a note, my findings show that only quicktime actually follows this correctly, but hopefully this can help us expose more bugs in the future!

@WyattBlue
Copy link
Copy Markdown
Member

WyattBlue commented Jun 7, 2026

Good to know.
/btw I've decided that LLMs shouldn't be tagged as co-authors in commit messages (In this commit, I'll think I'll just tag myself as co-author). You can cite the AI model that you used in the gh comments, but I don't think they're appropriate in the canonical history

@WyattBlue WyattBlue merged commit 28d50dd into PyAV-Org:main Jun 7, 2026
7 checks passed
@hmaarrfk
Copy link
Copy Markdown
Contributor Author

hmaarrfk commented Jun 7, 2026

yeah i agree, it tagged itself, i do review the code the code that it writes, i just don't have enough git skills to remove it.....

@hmaarrfk
Copy link
Copy Markdown
Contributor Author

hmaarrfk commented Jun 7, 2026

any chance of a release with this feature? i've been waiting for it for a long time, and with these AI agents, i've had a new willingness to code all the hard things i thought were impossible....

@WyattBlue
Copy link
Copy Markdown
Member

WyattBlue commented Jun 7, 2026

You can add

  "attribution": {
    "commit": "",
    "pr": ""
  },

to ~/.claude/settings.json to remove attribution

@hmaarrfk
Copy link
Copy Markdown
Contributor Author

hmaarrfk commented Jun 7, 2026

Thanks.

@WyattBlue
Copy link
Copy Markdown
Member

WyattBlue commented Jun 7, 2026

I'll release soon, once I know the new linux-armv7 target is working.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hmaarrfk @WyattBlue