[FFmpeg-devel] [PATCH] FLIF16 GSOC Project Range Transformation YCoCg

[Jai Luthra]

Hi Kartik,

Please mark the patch as [WIP] or [RFC] if you are not sending it for a merge 
review. You may also mark it as [GSoC].

You've done a good job of reading the spec and turning the pseudocode there to 
a method, but I'm afraid it is not enough.

I would suggest you run the reference codec's encoder (and add log statements 
wherever you feel like in the transform related modules) to see how range and 
crange work.

Then try to create test cases for the module you're writing for FFmpeg, i.e. 
pass same values of crange/range etc. and try to see if it behaves similar to 
what the reference decoder would do.

Share your patch with the testcases as well, even if you have a main() 
function and it is not suitable for FFmpeg master just yet.

On Wed, Mar 18, 2020 at 10:56:54AM +0530, Kartik wrote:
>From: Kartik K. Khullar<kartikkhullar840 at gmail.com>
>
>---
> FFmpeg/libavcodec/flif16transform.c | 53 +++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
>diff --git a/FFmpeg/libavcodec/flif16transform.c b/FFmpeg/libavcodec/flif16transform.c
>index e69de29..febf5e9 100644
>--- a/FFmpeg/libavcodec/flif16transform.c
>+++ b/FFmpeg/libavcodec/flif16transform.c
>@@ -0,0 +1,53 @@
>+#include <math.h>
>+#include <stdint.h>
>+
>+//array of 2 elements is used as parameter because it represents min, max pair.
>+void TransformYCoCg(int16_t *range[2], int16_t *crange[2], int16_t yval, int16_t coval){	

What are yval and coval here? Who defines them and calls this method?

I see they are defined in the spec, but take a look at the reference codec to 
figure out what they are and how they work.

>+	int max_temp;
>+	if (range[1][0] > range[1][1]){
>+		if (range[1][0] > range[1][2]){
>+			max_temp = range[1][0];
>+		}
>+		else{
>+			max_temp = range[1][2];
>+		}
>+	}
>+	else{
>+		max_temp  = range[1][1];
>+	}
>+	int origmax4 = max_temp / 4 + 1;
>+	int newmax = 4*origmax4 - 1;
>+	
>+	//Updating color ranges
>+	range[0][0] = 0;		//first channel minimum
>+	range[1][0] = newmax;	//first channek maximum
>+	range[0][1] = -newmax;	//second channel minimum
>+	range[1][1] = newmax;	//second channel maximum
>+	range[0][2] = -newmax;	//third channel minimum
>+	range[1][2] = newmax;	//third channel maximum
>+	
>+	//Updating conditional range values
>+	crange[0][0] = range[0][0];
>+	crange[1][0] = range[1][0];
>+	
>+	if (yval < origmax4 - 1){
>+		crange[0][1] = -3 + 4*yval;
>+		crange[1][1] = 3 + 4*yval;
>+		crange[0][2] = -2 - 2*yval;
>+		crange[1][2] = 1 + 2 * yval - 2 * (abs(coval)/2);
>+	}
>+	else if (yval > 3*origmax4 - 1){
>+		crange[0][1] = 4*(yval - newmax);
>+		crange[1][1] = 4*(newmax - yval);
>+		crange[0][2] = -2*(newmax - yval) + 2 * ((abs(coval)+1)/2);
>+		crange[1][2] = 2*(newmax - yval);
>+	}
>+	else{	
>+		crange[0][1] = -newmax;
>+		crange[1][1] = newmax;
>+		crange[0][2] = (((2*newmax - 2*yval - 2*abs(coval) + 1) < (2 * yval + 1)) ?
>+		(2*newmax - 2*yval - 2*abs(coval) + 1) : (2 * yval + 1)) / 2;
>+		crange[1][2] = (2*(yval - newmax)) < (-2*yval - 1 + 2*(abs(coval)/2)) ?
>+		(2*(yval - newmax)) : (-2*yval - 1 + 2*(abs(coval)/2));
>+	}
>+}

Probably as all transformation passses (like YCoCg here) change the range and 
crange, it would be better to create a context struct and define the ranges 
there. And pass that struct's instance as an argument for each transformation 
pass.

Cheers,
Jai