Expose the chroma location of a video frame - #2393
Merged
Merged
Conversation
<details><summary>Claude's draft</summary> `VideoCodecContext.chroma_sample_location` already wrapped `AVCodecContext.chroma_sample_location`, but the frame side of the same field had no wrapper, so the chroma siting a decoder actually reported per frame could not be read at all — only the container's declaration. Add `VideoFrame.chroma_location`, wrapping `AVFrame.chroma_location`. Each property mirrors its C field name, which FFmpeg spells differently on the two structs. Add a `ChromaLocation` enum alongside `ColorRange`, `ColorTrc`, and `ColorPrimaries` in `av.video.reformatter`, naming the `AVCHROMA_LOC_*` values, and document it in the video API enum table. `AVChromaLocation` was already declared in `include/avutil.pxd`; `AVFrame.chroma_location` was not, and is now. `VideoStream` proxies its codec context, so its stub gains the `chroma_sample_location` it was already forwarding. Tests cover the frame property round trip, the codec context property, and an ffv1/matroska encode-decode round trip that carries TOPLEFT through to the decoded frames. Resume this Claude session: ``` cd /home/mark/git/PyAV claude --resume f640d6be-5fdb-42cd-b340-64abdde5eadb ``` </details>
<details><summary>Claude's draft</summary> `docs/api/video.rst` documented none of `VideoFrame`'s color attributes, so `chroma_location` had no list to join. Add a Colors section covering `colorspace`, `color_range`, `color_trc`, `color_primaries`, and `chroma_location`, noting that setting one relabels the frame rather than converting its pixels, and that a codec context carries the same values for a whole stream. The enum table listed only `Interpolation`, `Colorspace`, and `ColorRange`, so the enums naming three of those five attributes were undocumented. Add `ColorTrc` and `ColorPrimaries` next to the `ChromaLocation` entry, label the section so the Colors prose can link to it, and correct `ColorRange`'s `AVCOL*` to `AVCOL_RANGE_*`. Each attribute's docstring now says which enum names its values and declares its `int` type, on both `VideoFrame` and `VideoCodecContext`. The codec context's four color docstrings claimed to wrap `AVFrame` fields; they wrap the `AVCodecContext` ones. `docs/conf.py` gains the real doxygen anchors for the fields those docstrings reference, so the `:ffmpeg:` links land on the member rather than the top of the struct page. Docs build clean, with no warnings. Resume this Claude session: ``` cd /home/mark/git/PyAV claude --resume f640d6be-5fdb-42cd-b340-64abdde5eadb ``` </details>
<details><summary>Claude's draft</summary> The `:ffmpeg:` role falls back to `#<member>` when a struct member is not in its lookup table, but doxygen anchors members by an MD5 hash, so every unmapped reference landed at the top of the struct page instead of on the field. Fourteen references were falling back that way. Add their anchors, taken from the FFmpeg 8.0 doxygen pages the role already links to. The four already in the table were re-derived the same way and match, so the extraction is sound. `AVFrame.key_frame` had no anchor to add: FFmpeg removed the field in 7.0 in favour of the `AV_FRAME_FLAG_KEY` flag, which is what `Frame.key_frame` has been reading all along. Its docstring now says so and links to `AVFrame.flags`. Every `:ffmpeg:` member link in a built copy of the docs now resolves to a real anchor. Resume this Claude session: ``` cd /home/mark/git/PyAV claude --resume f640d6be-5fdb-42cd-b340-64abdde5eadb ``` </details>
Contributor
Author
|
thanks. |
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
2 participants
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.
As the title says.
There was to be a value VideoCodecContext, but no "per frame property"
This meerly exposes the ffmpeg property
Claude's draft
VideoCodecContext.chroma_sample_locationalready wrappedAVCodecContext.chroma_sample_location, but the frame side of the same field had no wrapper, so the chroma siting a decoder actually reported per frame could not be read at all — only the container's declaration.Add
VideoFrame.chroma_location, wrappingAVFrame.chroma_location. Each property mirrors its C field name, which FFmpeg spells differently on the two structs.Add a
ChromaLocationenum alongsideColorRange,ColorTrc, andColorPrimariesinav.video.reformatter, naming theAVCHROMA_LOC_*values, and document it in the video API enum table.AVChromaLocationwas already declared ininclude/avutil.pxd;AVFrame.chroma_locationwas not, and is now.VideoStreamproxies its codec context, so its stub gains thechroma_sample_locationit was already forwarding.Tests cover the frame property round trip, the codec context property, and an ffv1/matroska encode-decode round trip that carries TOPLEFT through to the decoded frames.
Resume this Claude session: