From 7d905adf106d493092af459e0ddda98db810142c Mon Sep 17 00:00:00 2001
From: Oliver Atkinson <oliveratkinsoneng@gmail.com>
Date: Tue, 5 Sep 2023 08:21:17 -0600
Subject: [PATCH] code cleanup and comments

---
 src/out.rs | 105 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 38 deletions(-)

diff --git a/src/out.rs b/src/out.rs
index 35e2017..ec33a13 100644
--- a/src/out.rs
+++ b/src/out.rs
@@ -13,62 +13,91 @@ pub enum LinesFormat {
     Left,
 }
 
+#[derive(Debug)]
+pub enum ParseError {
+    NotRGB24,
+    NotSquareAspectRatio,
+    VideoIndexOutOfBounds,
+}
+
+impl std::fmt::Display for ParseError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let s = match self {
+            ParseError::NotRGB24 => "Not in RGB24 format!",
+            ParseError::NotSquareAspectRatio => "Not in a square (1:1) aspect ratio!",
+            ParseError::VideoIndexOutOfBounds => "Selected video index is not valid (Out of Bounds)!",
+        };
+        write!(f, "{s}")
+    }
+}
+
+impl std::error::Error for ParseError {} 
+
 /// Must use square images
-pub fn print_square(frame: &ffmpeg::frame::Video, lines_format: LinesFormat, video_index: usize) {
-    if frame.format() != Pixel::RGB24 { panic!("Must use Pixel::RGB24"); }
-    if frame.width() != frame.height() { panic!("Must be 1:1 aspect ratio"); } 
+/// Images must be in RGB24
+pub fn print_square(frame: &ffmpeg::frame::Video, lines_format: LinesFormat, video_index: usize) -> Result<(), ParseError> {
+    if frame.format() != Pixel::RGB24   { return Err(ParseError::NotRGB24); }
+    if frame.width() != frame.height()  { return Err(ParseError::NotSquareAspectRatio); } 
+    // 8 is the value hard-coded in AVFrame
+    if !(0..=8).contains(&video_index)  { return Err(ParseError::VideoIndexOutOfBounds); } 
 
-    // This will panic if it's out of bounds.
-    let z = video_index;
+    let pixels_wide = frame.width() as usize;
 
-    let size = frame.width() as usize;
-    // How many bytes per pixel RGBA
-    let linesize = unsafe { std::ptr::addr_of!((*frame.as_ptr()).linesize) };
-    let step = (unsafe { *linesize })[z] as usize;
-    
     let mut stdout = stdout();
     let mut lock = stdout.lock();
 
-    let buf = frame.data(z); 
+    // Entire frame, as rgb data
+    let buf = frame.data(video_index); 
+
+    // How many bytes per line
+    let step = buf.len() / frame.height() as usize;
+
+    // Explanation?: (12x12 image:)
+    // The full buffer will be 1152 bytes.
+    // 1152/12=96   Buffer length   / Pixels high       = 96 Bytes per line.
+    // 96/12=8      Bytes per line  / Pixels per line   = 8 Bytes per pixel.
+    //
+    // But RGB24 should only use 3 bytes per pixel.
+    // The other 5 we just discard.
+    //
+
     // (Assuming square) Step thru buffer.
-    for i in 0..size {
+    for i in 0..pixels_wide {
         let j = i*step;
-        // Slices non-inclusively, ie: Even though j(n)+step = j(n+1) the ranges don't overlap. It take from..until
-        let pre_slice = &buf[j..j+step];
-        // Only take the size*3 of bytes, ignoring alpha values which are stored after.
-        // let slice = &pre_slice[0..size*3];
-        let mut slice = Vec::from(&pre_slice[0..size*3]);
-        // let lol = &buf[j..j+step][0..size*3];
+        // Only take the size*3 of bytes, ignoring alpha(?) values which are stored after.
+        // Have to take ownership, by implicilty Cloning the data. (Copying?)
+        let mut slice = Vec::from(&buf[j..j+pixels_wide*3]);
 
         match lines_format {
-            LinesFormat::Right => {/* Do nothing */},
-            LinesFormat::RightLeft => if i & 1 == 1 { reverse(&mut slice) },
-            LinesFormat::LeftRight => if i & 1 != 1 { reverse(&mut slice) },
-            LinesFormat::Left => slice.reverse(),
+            LinesFormat::RightLeft =>  if i & 1 == 1 { rev_in_place_with_groupings(&mut slice) },
+            LinesFormat::LeftRight  => if i & 1 != 1 { rev_in_place_with_groupings(&mut slice) },
+            LinesFormat::Left       => rev_in_place_with_groupings(&mut slice),
+            LinesFormat::Right      => {},
         }
 
-
         // DEBUG
         // println!("{:?} || {}-{}", slice, j, j+step);
         // NORMAL
         lock.write(&slice).unwrap();
     }
     stdout.flush().unwrap();
+    Ok(())
 }
 
-
-/// Reverses the order of every grouping of 3 bytes.
-fn reverse(raw: & mut [u8]) {
-    // I would prefer this only went over the elements once,
-    // not twice.
-
-    let x = raw
-        .iter()
-        .array_chunks::<3>()
-        .rev()
-        .flatten()
-        .map(|f| *f)
-        .collect::<Vec<u8>>();
-    
-    x.iter().enumerate().for_each(|f| raw[f.0] = *f.1);
+fn rev_in_place_with_groupings(raw: &mut Vec<u8>) {
+    for i in 0..raw.len() {
+        match i % 3 {
+            // Think of these as the indexes of each
+            // RGB tuple. [0, 1, 2] [R, G, B].
+            // 0 and 2 have to be switched, while 1 can stay.
+            2 => {
+                let zero = raw[i-2];
+                let two  = raw[i];
+                raw[i-2] = two;
+                raw[i] = zero;
+            },
+            _ => { /* 0 and 1*/ },
+        } 
+    }
 }
+