Support arbitrary SAR anamorphic video

[rzumer]

This adds an aspect ratio input parameter
to the Rust and C APIs, computes the render
size by upscaling one dimension, and codes
it in-bitstream, as well as in Y4M output.

The default aspect ratio is parsed from
Y4M input, or set to 1:1 if absent.

Note that neither aomdec nor dav1d will propagate the bitstream render size to the Y4M files they output.
It doesn't seem to be honored by ffmpeg with aomdec either, but it might be with dav1d (haven't tested it myself, but the code seems present). Reconstruction output is affected as well.
I think we can close #2504 with this, as it implements everything that should be needed for decoders/players to correctly output/display the encoded video.

[coveralls]

Coverage increased (+0.1%) to 83.096% when pulling 1d3eda3 on rzumer:aspect-ratio into fe87644 on xiph:master.

[dwbuiten]

There seems to be a bug that causes only the first frame to have its render width/height changed.

[rzumer]

I'm now aware that 0:0 is a "valid" aspect ratio that should map to 1:1, so such samples are to-test and might be rejected with the current state of the branch.

[dwbuiten]

Thinking more about this, shouldn't it be per-frame?

[rzumer]

I think we can leave that as future work.

[rzumer]

@dwbuiten I moved the parameters to FrameInvariants where they should be. I first thought you were suggesting to make aspect ratio variable per-frame via the API.

[dwbuiten]

@rzumer I was indeed saying it should be per-frame in the API. Aspect ratio may change midstream, and should be supported either via frame properties, or via encoder-reconfig, I believe.

[rzumer]

Then as mentioned earlier I prefer it to be left as future work considering how rare of a use case it is, unless there is already a framework for setting parameters frame by frame (I can't see anything in the api/config module).

[jamrial]

Maybe this should instead set frame_size_override_flag to 1 on inter frames and take the frame and render dimension values from one of the frames they reference in the ref_frame_idx[] array (The frame_size_with_refs() function defined in Section 5.9.7).

That way you'll save 4 bytes per frame header used to code the same render dimension values.

[rzumer]

@jamrial That works as long as frame size is static. We can probably assume it is for now though.

[jamrial]

Afailk, it can also work for non static frame sizes by ensuring a given inter frame references a frame with the exact same values it would otherwise write. If none of the seven potential reference frames share the exact same values, then they are explicitly written instead.

[rzumer]

I see, I'll look into adding that.

[jamrial]

If you'd rather not bother looking at the values for each of the seven reference frame since for now the sizes are fixed, then it's a matter of setting frame_size_override_flag to 1 and implementing frame_size_with_refs() as a single bit found_ref to signal that the first reference frame should be used by the decoder to infer this frame's dimensions.

[rzumer]

After looking into it I don't think this frame_size_override has the same effect as overriding the render_size. Seems to me that frame size is related to the number of luma samples, while render size signals players to rescale the frame when displaying it. Unless this part of the spec is inconsistently written and poorly defined, and the aomdec behaviour matches what you're implying, I'd keep the current implementation.

[jamrial]

No, it doesn't have the same effect. frame_size_override_flag means that either frame dimensions, render dimensions, or both, may be coded in the frame or should be inferred from a reference frame. For this specific case, frame dimensions and render dimensions will be constant for the entire video sequence, so setting both frame_size_override_flag and render_and_frame_size_different to 1 and then taking the first reference frame's dimensions is the same as signaling only render_and_frame_size_different and coding the render dimensions in the frame, except it doesn't use four bytes for it.

See the code below

diff --git a/src/header.rs b/src/header.rs
index 194c0510..7c66fe98 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -538,7 +538,14 @@ impl<W: io::Write> UncompressedHeader for BitWriter<W, BigEndian> {
       //self.write(frame_id_len, fi.current_frame_id);
     }

-    let mut frame_size_override_flag = false;
+    let render_and_frame_size_different = fi.render_width != fi.width as u32
+      || fi.render_height != fi.height as u32;
+
+    // This assumes frame dimensions are the same as sequence header dimensions,
+    // and that only render dimensions may be different but nonetheless constant
+    // for the entire video sequence
+    let mut frame_size_override_flag = render_and_frame_size_different && !fi.intra_only
+      && !fi.error_resilient;
     if fi.frame_type == FrameType::SWITCH {
       frame_size_override_flag = true;
     } else if fi.sequence.reduced_still_picture_hdr {
@@ -606,8 +613,6 @@ impl<W: io::Write> UncompressedHeader for BitWriter<W, BigEndian> {
       if fi.sequence.enable_superres {
         unimplemented!();
       }
-      let render_and_frame_size_different = fi.render_width != fi.width as u32
-        || fi.render_height != fi.height as u32;
       self.write_bit(render_and_frame_size_different)?;
       if render_and_frame_size_different {
         self.write(16, fi.render_width - 1);
@@ -640,7 +645,11 @@ impl<W: io::Write> UncompressedHeader for BitWriter<W, BigEndian> {
       }

       if !fi.error_resilient && frame_size_override_flag {
-        unimplemented!();
+        //TODO: check reference frames to compare frame dimensions
+        self.write_bit(true)?; // found_ref[0]
+        if fi.sequence.enable_superres {
+          unimplemented!();
+        }
       } else {
         if frame_size_override_flag {
           self.write_frame_size_override(fi);
@@ -648,9 +657,6 @@ impl<W: io::Write> UncompressedHeader for BitWriter<W, BigEndian> {
         if fi.sequence.enable_superres {
           unimplemented!();
         }
-        let render_and_frame_size_different = fi.render_width
-          != fi.width as u32
-          || fi.render_height != fi.height as u32;
         self.write_bit(render_and_frame_size_different)?;
         if render_and_frame_size_different {
           self.write(16, fi.render_width - 1);

[rzumer]

I mean, I'm fine with this change but it seems like it should be separate from this PR.

Isn't it applicable regardless of render size anyway since we currently assume a static frame size?

[jamrial]

Sure, i can send a PR for it after this one is merged.

Also, here's a complex sample where frame dimensions vary between frames (But render dimensions are constant), and uses the frame_size_with_refs() logic to avoid coding the values in the bitstream when a reference frame shares the same values, if you're curious.

https://code.videolan.org/videolan/dav1d/uploads/e41cf2423c6ae93e2a0a4fc9ae543730/resize.ivf

[dwbuiten]

Then as mentioned earlier I prefer it to be left as future work considering how rare of a use case it is, unless there is already a framework for setting parameters frame by frame (I can't see anything in the api/config module).

I think it should be doable with an encoder reconfig API, so doing it eoncig level should be OK, I suppose. Color properties are done this way too.