rav1e
rav1e copied to clipboard
Support max_width/max_height options.
Fixes #2467
It could be useful to state in the doc comments what implications these options have on the decoded bitstream. Without reading the spec it's hard to tell what they might be used for.
Coverage decreased (-0.02%) to 83.278% when pulling 7d049a94a635ae7e7f94d8e5bcbd5cfcb2d75653 on tdaede:max_frame_width into 54b667cdc4ea49709ba744e37c3a515252d23c09 on xiph:master.
Added some doc comments.
it needs a rebase.
Also, the spec states that if reduced_still_picture_header is enabled (currently always forced for still_picture), dimensions in the sequence header and the frame header must be the same, so you should check the user provided arguments to ensure this is the case.
This patch needs are rebase, should we land it before 0.5?
This has rotted pretty badly, it'll need some work but I would like it in.
A long time coming, but I've rebased this and fixed the nits.
LGTM besides one comment
If i set a max_width value lower than the width of the actual frames (e.g. a 720x480 y4m source encoded with --max-width=64) i get this error:
!! Invalid configuration: invalid width 720 (expected >= 16, <= 65535)
Error: Generic { msg: "Invalid configuration", e: "invalid width 720 (expected >= 16, <= 65535)" }
Which is confusing and wrong.
Would you prefer that the error message for width/height be updated to indicate that it should be less than 65535 or max_width, or that a new error message for explicitly max_width be created? (and if so, we could technically error either for the width or max_width setting?)
FWIW for the S-Frame content that this is useful for right now, the muxing happens outside of rav1e, so it itself doesn't have to see the width/height change dynamically.
The first option sounds good. Basically, replacing the hardcoded 65535 with something like min(65535, max_width).
OK, it now displays:
!! Invalid configuration: invalid width 720 (expected >= 16, <= 720)
Error: Generic { msg: "Invalid configuration", e: "invalid width 720 (expected >= 16, <= 720)" }
(we should probably remove that redundant CLI error printing)
!! Invalid configuration: invalid width 720 (expected >= 16, <= 720)
But 720 is <= 720.
Also, if i passed --max_width=64, it should report !! Invalid configuration: invalid width 720 (expected >= 16, <= 64)
for an input video with width 720.
Ah indeed I forgot to update a parameter of the format string. Now displays:
!! Invalid configuration: invalid width 1280 (expected >= 16, <= 720)
Error: Generic { msg: "Invalid configuration", e: "invalid width 1280 (expected >= 16, <= 720)" }
Should be good then. Thanks.