Revert "Revert "Enable and test high bit depth input (#437)" (#442)" and fix reconstruction output #447
Conversation
This file contains 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
Currently compiles, encodes and decodes with desynchronization
Commented out by default, to enable as needed for local testing.
|
The test failure in CI is due to CDEF changes introduced after #437. Setting |
What do you mean? Stream copy example works fine for me with nyan12.y4m, producing playable 12-bit file. |
|
@Kagami interesting, on my end encoding the reconstruction in rav1e panicks with "bad input" above 8-bit, which I thought was happening on |
|
Regarding the reconstruction output at high bit depths, here is the relevant issue in We can perform our own conversion for now using |
lu-zero
reviewed
Aug 13, 2018
src/lib.rs
Outdated
| fs.rec.planes[1].cfg.stride, | ||
| fs.rec.planes[2].cfg.stride); | ||
|
|
||
| for (y, line) in fs.rec.planes[0].data.chunks_mut(stride_y).enumerate() { |
There was a problem hiding this comment.
Rec does not mutate and you can use chunks_mut() on rec_y and friends.
for (line, line_out) in fs.rec.planes[0].data.chunks().zip(rec_y.chunks_mut(pitch_y))
|
LGTM. |
lu-zero
approved these changes
Aug 13, 2018
rzumer
changed the title
barrbrain
pushed a commit
to barrbrain/rav1e
that referenced
this pull request
Oct 5, 2022
…h#442)" (xiph#447) * Attempt to process 10-bit Y4M input Currently compiles, encodes and decodes with desynchronization * Use high bit depth quantization tables * Move context::clamp() to util.rs * Fix partition context initialization for high bit depth * Enable and test 10-bit input * Add 10- and 12-bit test clips to build.sh Commented out by default, to enable as needed for local testing. * Use the same bit depth as y4m_dec for y4m_enc * Fix benchmark module compilation * Fix high bit depth test encoding in 8-bit * Fix header syntax for 12-bit 4:2:0 input * Enable and test 12-bit input * Reflect 12-bit support in README.md * Keep the default C420jpeg color space in y4m_encoder * Do not allow reconstruction output at high bit depths * Fix reconstruction output at high bit depths * Clean up reconstruction copy to frame buffers
barrbrain
pushed a commit
to barrbrain/rav1e
that referenced
this pull request
Oct 5, 2022
…h#442)" (xiph#447) * Attempt to process 10-bit Y4M input Currently compiles, encodes and decodes with desynchronization * Use high bit depth quantization tables * Move context::clamp() to util.rs * Fix partition context initialization for high bit depth * Enable and test 10-bit input * Add 10- and 12-bit test clips to build.sh Commented out by default, to enable as needed for local testing. * Use the same bit depth as y4m_dec for y4m_enc * Fix benchmark module compilation * Fix high bit depth test encoding in 8-bit * Fix header syntax for 12-bit 4:2:0 input * Enable and test 12-bit input * Reflect 12-bit support in README.md * Keep the default C420jpeg color space in y4m_encoder * Do not allow reconstruction output at high bit depths * Fix reconstruction output at high bit depths * Clean up reconstruction copy to frame buffers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
After investigating the errors caused by the original PR, it seems that the tools used for comparison are interpolating samples from Y4M files labelled as using the
C420jpegcolor space, which the libaom decoder always uses, regardless of the chroma positioning indicated in the header.In order to support outputting Y4M reconstructions at high bit depths, a change in
rav1e.rsset the Y4M encoder's color space to be the same as that of the input file's. This means that rav1e was now outputtingC420Mpeg2reconstructions for input files using that color space, while the decoder output was alwaysC420jpegand therefore required interpolating chroma samples in order to be converted to a raw image. Of course, the result was that due to interpolation error, there was a difference in raw output between the reconstructed output and the AOM decoder output.The change that causes this issue was reverted in order to maintain compatibility between the two output streams, and because it does not currently affect compatibility with high bit depth streams, asHowever, the real issue that prompted a reversal of the original PR stems from:thewe do not yet support encoding Y4M files at high bit depths.y4mcrate doesI recommend testing encoder output by comparing the reconstructed and decoded Y4M files, excluding the header, in the future.
The changes to this PR compared to #437 are as follows:
the Y4M encoder used to output reconstruction is no longer initialized to a non-default color spaceusing the-roption at high bit depths will panic, as they4mcrate does not currently support high bit depth encodingRelevant libaom issue: https://bugs.chromium.org/p/aomedia/issues/detail?id=2096