diff mbox series

[FFmpeg-devel] FLIF16 GSOC Project - Added RGB to YCoCg macros

Message ID 20200318053122.20528-1-kartikkhullar840@gmail.com
State New
Headers show
Series [FFmpeg-devel] FLIF16 GSOC Project - Added RGB to YCoCg macros | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Kartik K. Khullar March 18, 2020, 5:31 a.m. UTC
From: Kartik K. Khullar<kartikkhullar840@gmail.com>

---
 FFmpeg/libavutil/colorspace.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Jai Luthra March 21, 2020, 9:24 a.m. UTC | #1
Please mark the patch as [WIP] or [RFC] if you are not sending it for a merge 
review.

On Wed, Mar 18, 2020 at 11:01:22AM +0530, Kartik wrote:
>From: Kartik K. Khullar<kartikkhullar840@gmail.com>
>
>---
> FFmpeg/libavutil/colorspace.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
>diff --git a/FFmpeg/libavutil/colorspace.h b/FFmpeg/libavutil/colorspace.h
>index ef6f610..bf53afe 100644
>--- a/FFmpeg/libavutil/colorspace.h
>+++ b/FFmpeg/libavutil/colorspace.h
>@@ -94,6 +94,17 @@ static inline int C_JPEG_TO_CCIR(int y) {
>     return y;
> }
>
>+#define RGB_TO_YCoCg(R, G, B, Y, Co, Cg){\
>+	Y = (((R+B)>>1) + G)>>1;\
>+    Co = R - B;\
>+    Cg = G - ((R+B)>>1);\
>+}

You don't need to take Y,Co,Cg as inputs to the macro, look at other similar 
macros.

I would also suggest defining different macros for different output channels 
as it does not assume that Y,Co,Cg will be variables defined in the caller's 
context. Take a look at RGB_TO_Y/U/V_JPEG etc.

>+
>+#define YCoCg_TO_RGB(R, G, B, Y, Co, Cg){\
>+	Y = (((R+B)>>1) + G)>>1;\
>+    Co = R - B;\
>+    Cg = G - ((R+B)>>1);\
>+}

Only the name is inverse here, it still does the forward transform.

>
> #define RGB_TO_Y_CCIR(r, g, b) \
> ((FIX(0.29900*219.0/255.0) * (r) + FIX(0.58700*219.0/255.0) * (g) + \
>-- 
>2.20.1.windows.1
>
Lynne March 21, 2020, 10:25 a.m. UTC | #2
Mar 21, 2020, 09:24 by me@jailuthra.in:

> Please mark the patch as [WIP] or [RFC] if you are not sending it for a merge review.
>
> On Wed, Mar 18, 2020 at 11:01:22AM +0530, Kartik wrote:
>
>> From: Kartik K. Khullar<kartikkhullar840@gmail.com>
>>
>> ---
>> FFmpeg/libavutil/colorspace.h | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/FFmpeg/libavutil/colorspace.h b/FFmpeg/libavutil/colorspace.h
>> index ef6f610..bf53afe 100644
>> --- a/FFmpeg/libavutil/colorspace.h
>> +++ b/FFmpeg/libavutil/colorspace.h
>> @@ -94,6 +94,17 @@ static inline int C_JPEG_TO_CCIR(int y) {
>>  return y;
>> }
>>
>> +#define RGB_TO_YCoCg(R, G, B, Y, Co, Cg){\
>> +	Y = (((R+B)>>1) + G)>>1;\
>> +    Co = R - B;\
>> +    Cg = G - ((R+B)>>1);\
>> +}
>>
>
> You don't need to take Y,Co,Cg as inputs to the macro, look at other similar macros.
>
> I would also suggest defining different macros for different output channels as it does not assume that Y,Co,Cg will be variables defined in the caller's context. Take a look at RGB_TO_Y/U/V_JPEG etc.
>
>> +
>> +#define YCoCg_TO_RGB(R, G, B, Y, Co, Cg){\
>> +	Y = (((R+B)>>1) + G)>>1;\
>> +    Co = R - B;\
>> +    Cg = G - ((R+B)>>1);\
>> +}
>>
>
> Only the name is inverse here, it still does the forward transform.
>
>>
>> #define RGB_TO_Y_CCIR(r, g, b) \
>> ((FIX(0.29900*219.0/255.0) * (r) + FIX(0.58700*219.0/255.0) * (g) + \
>> -- 
>> 2.20.1.windows.1
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org>  with subject "unsubscribe".
>


Why are you even adding those macros in? We don't covert colorspaces or pixel formats in the decoders. If you have a ycgco output just mark it as yuv420p with a ycgco colorspace.
diff mbox series

Patch

diff --git a/FFmpeg/libavutil/colorspace.h b/FFmpeg/libavutil/colorspace.h
index ef6f610..bf53afe 100644
--- a/FFmpeg/libavutil/colorspace.h
+++ b/FFmpeg/libavutil/colorspace.h
@@ -94,6 +94,17 @@  static inline int C_JPEG_TO_CCIR(int y) {
     return y;
 }
 
+#define RGB_TO_YCoCg(R, G, B, Y, Co, Cg){\
+	Y = (((R+B)>>1) + G)>>1;\
+    Co = R - B;\
+    Cg = G - ((R+B)>>1);\
+}
+
+#define YCoCg_TO_RGB(R, G, B, Y, Co, Cg){\
+	Y = (((R+B)>>1) + G)>>1;\
+    Co = R - B;\
+    Cg = G - ((R+B)>>1);\
+}
 
 #define RGB_TO_Y_CCIR(r, g, b) \
 ((FIX(0.29900*219.0/255.0) * (r) + FIX(0.58700*219.0/255.0) * (g) + \