-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add cropping metadata #346
base: master
Are you sure you want to change the base?
Conversation
| metadata_crop( ) { | **Type** | ||
| @@crop_width_minus_1 | f(16) | ||
| @@crop_height_minus_1 | f(16) | ||
| @@crop_offset_present | f(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we make it byte-aligned in the first place? having something like:
f(7) reserved
f(1) crop_offset_present;
would be better IMO than relying on the trailing_bits
at the end of the OBU.
07.bitstream.semantics.md
Outdated
#### Metadata crop semantics | ||
|
||
When present the metadata_crop OBU applies starting at the next frame in the sequence | ||
with matching temporal_id and spatial_id, and shall apply to all matching frames until the next Key Frame |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When present the metadata_crop OBU applies starting at the next frame in the sequence
with matching temporal_id and spatial_id, ...
Alexis and myself are wondering here what the behavior should be if no extension header is present. Perhaps in that case it should apply to all layers? Then the language on the persistence should also be clarified.
Also instead of:
and shall apply to all matching frames
it would be better to write "and shall apply to all frames with a matching temporal_id and spatial_id"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I think it should apply to all layers if no header is present. I will add wording clarifying that.
07.bitstream.semantics.md
Outdated
When present the metadata_crop OBU applies starting at the next frame in the sequence | ||
with matching temporal_id and spatial_id, and shall apply to all matching frames until the next Key Frame | ||
or metadata_crop OBU. The output picture should be cropped to the region as specified in the | ||
cropping metadata OBU. When applied, the crop shall be after all normal decode operations as a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When applied, the crop shall be after all normal decode operations as a post-processing step (after film grain synthesis in the output process). This metadata has no effect on the decoding process.
Maybe it would be better to re-write like this:
The crop shall be applied after all normal decode operations as a post-processing step. This metadata information has no effect on the decoding process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used your new wording.
07.bitstream.semantics.md
Outdated
|
||
**crop_y_offset** specifies the minimum pixel row containing picture data which should be rendered. | ||
|
||
**crop_width_minus_1** specifies the number of pixel columns minus 1 which which should be rendered after applying crop_x_offset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which which => which
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
07.bitstream.semantics.md
Outdated
When muxed into a container that supports signaling cropping information, this metadata should | ||
be removed from the bitstream and included in the container’s signaling mechanism. | ||
If both the container and bitstream signal cropping information, then the container’s cropping | ||
information takes precedence. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of:
When muxed into a container that supports signaling cropping information, this metadata should
be removed from the bitstream and included in the container’s signaling mechanism.
If both the container and bitstream signal cropping information, then the container’s cropping
information takes precedence.
We could write:
In cases where cropping information is present in both the bitstream and the delivery or container format, the latter should be preferred.
NOTE: Container or delivery formats that package the AV1 bitstream are recommended to address this redundancy, potentially by excluding bitstream cropping information when it is already available in the container or delivery format.
Then we will have to update the AV1 ISOBMFF spec and say there that we use the clap
box and remove this metadata OBU. Thoughts @cconcolato ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to new wording.
Some relevant discussion in the file format group on how such metadata can co-exist MPEGGroup/FileFormat#77 |
Unlike H264/H265, AV1 contains no fields to crop encoded output to specific sizes. AMD's hardware cannot handle encoding of unaligned dimensions for AV1, hence it codes 1920x1080 as 1920x1088. Add side data to crop the output back to the original dimensions. There's an AV1-spec extension planned to fix this here: AOMediaCodec/av1-spec#346 But it seems to have stuck for now.
Unlike H264/H265, AV1 contains no fields to crop encoded output to specific sizes. AMD's hardware cannot handle encoding of unaligned dimensions for AV1, hence it codes 1920x1080 as 1920x1088. Add side data to crop the output back to the original dimensions. There's an AV1-spec extension planned to fix this here: AOMediaCodec/av1-spec#346 But it seems to have stuck for now.
Unlike H264/H265, AV1 contains no fields to crop encoded output to specific sizes. AMD's hardware cannot handle encoding of unaligned dimensions for AV1, hence it codes 1920x1080 as 1920x1088. Add side data to crop the output back to the original dimensions. There's an AV1-spec extension planned to fix this here: AOMediaCodec/av1-spec#346 But it seems to have stuck for now.
This adds cropping metadata as proposed in CWG-D038. Accepted in the 10/10/2023 CWG meeting.
Most of the text is the same as the proposal document with some minor adjustments: