Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-10 Thread Michael Niedermayer
On Tue, Sep 01, 2015 at 05:02:10PM +0100, Kevin Wheatley wrote:
> Rather than rewrite history or anything like that people could either
> apply this on top of my previous patches, or for a view of the whole
> kit and kaboodle:
> 
> https://github.com/FFmpeg/FFmpeg/compare/master...KevinJW:image_exr_transfer_characteristics
> 
> Happy to take further feedback,

equivalent patch from github applied (as it doesnt have the reserved _)

thanks

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread Kevin Wheatley
Rather than rewrite history or anything like that people could either
apply this on top of my previous patches, or for a view of the whole
kit and kaboodle:

https://github.com/FFmpeg/FFmpeg/compare/master...KevinJW:image_exr_transfer_characteristics

Happy to take further feedback,

Kevin

On Tue, Sep 1, 2015 at 1:03 PM, wm4  wrote:
> On Tue, 1 Sep 2015 12:55:49 +0100
> Kevin Wheatley  wrote:
>
>> On Tue, Sep 1, 2015 at 12:49 PM, wm4  wrote:
>> > Identifiers starting with _ are reserved by the system on file scope.
>>
>> oh yes, switching between different programming languages never a good
>> thing for my brain :-)
>>
>> Would the ffmpeg style be prefixing them up with anything, or just
>> leaving off the underscore?
>
> Yes. For static functions, any identifier should be fine; for
> identifiers which are not static and are needed in other files,
> but are still library-private, we use a ff_ prefix.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From 313d6f147b1beb05ab5676d8cd0a7c745c60051d Mon Sep 17 00:00:00 2001
From: Kevin Wheatley 
Date: Tue, 1 Sep 2015 14:02:31 +0100
Subject: [PATCH 8/8] avoid the use of reserved names

---
 libavutil/color_utils.c |   48 +++---
 1 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/libavutil/color_utils.c b/libavutil/color_utils.c
index f84b5f5..81ba447 100644
--- a/libavutil/color_utils.c
+++ b/libavutil/color_utils.c
@@ -57,7 +57,7 @@ double avpriv_get_gamma_from_trc(enum AVColorTransferCharacteristic trc)
 #define BT709_alpha 1.099296826809442
 #define BT709_beta 0.018053968510807
 
-static double _trc_bt709(double Lc)
+static double avpriv_trc_bt709(double Lc)
 {
 const double a = BT709_alpha;
 const double b = BT709_beta;
@@ -67,17 +67,17 @@ static double _trc_bt709(double Lc)
  :  a * pow(Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_gamma22(double Lc)
+static double avpriv_trc_gamma22(double Lc)
 {
 return (0.0 > Lc) ? 0.0 : pow(Lc, 1.0/ 2.2);
 }
 
-static double _trc_gamma28(double Lc)
+static double avpriv_trc_gamma28(double Lc)
 {
 return (0.0 > Lc) ? 0.0 : pow(Lc, 1.0/ 2.8);
 }
 
-static double _trc_smpte240M(double Lc)
+static double avpriv_trc_smpte240M(double Lc)
 {
 const double a = 1.1115;
 const double b = 0.0228;
@@ -87,23 +87,23 @@ static double _trc_smpte240M(double Lc)
  :  a * pow(Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_linear(double Lc)
+static double avpriv_trc_linear(double Lc)
 {
 return Lc;
 }
 
-static double _trc_log(double Lc)
+static double avpriv_trc_log(double Lc)
 {
 return (0.01 > Lc) ? 0.0 : 1.0 + log10(Lc) / 2.0;
 }
 
-static double _trc_log_sqrt(double Lc)
+static double avpriv_trc_log_sqrt(double Lc)
 {
 // sqrt(10) / 1000
 return (0.00316227766 > Lc) ? 0.0 : 1.0 + log10(Lc) / 2.5;
 }
 
-static double _trc_iec61966_2_4(double Lc)
+static double avpriv_trc_iec61966_2_4(double Lc)
 {
 const double a = BT709_alpha;
 const double b = BT709_beta;
@@ -113,7 +113,7 @@ static double _trc_iec61966_2_4(double Lc)
  :   a * pow( Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_bt1361(double Lc)
+static double avpriv_trc_bt1361(double Lc)
 {
 const double a = BT709_alpha;
 const double b = BT709_beta;
@@ -123,7 +123,7 @@ static double _trc_bt1361(double Lc)
  :   a * pow( Lc, 0.45) - (a - 1.0);
 }
 
-static double _trc_iec61966_2_1(double Lc)
+static double avpriv_trc_iec61966_2_1(double Lc)
 {
 const double a = 1.055;
 const double b = 0.0031308;
@@ -133,7 +133,7 @@ static double _trc_iec61966_2_1(double Lc)
  :  a * pow(Lc, 1.0  / 2.4) - (a - 1.0);
 }
 
-static double _trc_smpte_st2084(double Lc)
+static double avpriv_trc_smpte_st2084(double Lc)
 {
 const double c1 = 3424.0 / 4096.0; // c3-c2 + 1
 const double c2 =  32.0 * 2413.0 / 4096.0;
@@ -148,7 +148,7 @@ static double _trc_smpte_st2084(double Lc)
 
 }
 
-static double _trc_smpte_st428_1(double Lc)
+static double avpriv_trc_smpte_st428_1(double Lc)
 {
 return (0.0 > Lc) ? 0.0
  :  pow(48.0 * Lc / 52.37, 1.0 / 2.6);
@@ -162,50 +162,50 @@ avpriv_trc_function avpriv_get_trc_function_from_trc(enum AVColorTransferCharact
 case AVCOL_TRC_SMPTE170M:
 case AVCOL_TRC_BT2020_10:
 case AVCOL_TRC_BT2020_12:
-func = _trc_bt709;
+func = avpriv_trc_bt709;
 break;
 
 case AVCOL_TRC_GAMMA22:
-func = _trc_gamma22;
+func = avpriv_trc_gamma22;
 break;
 case AVCOL_TRC_GAMMA28:
-func = _trc_gamma28;
+func = avpriv_trc_gamma28;
 break;
 
 case AVCOL_TRC_SMPTE240M:
-func = _trc_smpte240M;
+func = avpriv_trc_smpte240M;
 break;
 
 case AVCOL_TRC_L

Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread wm4
On Tue, 1 Sep 2015 12:55:49 +0100
Kevin Wheatley  wrote:

> On Tue, Sep 1, 2015 at 12:49 PM, wm4  wrote:
> > Identifiers starting with _ are reserved by the system on file scope.
> 
> oh yes, switching between different programming languages never a good
> thing for my brain :-)
> 
> Would the ffmpeg style be prefixing them up with anything, or just
> leaving off the underscore?

Yes. For static functions, any identifier should be fine; for
identifiers which are not static and are needed in other files,
but are still library-private, we use a ff_ prefix.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread Kevin Wheatley
On Tue, Sep 1, 2015 at 12:49 PM, wm4  wrote:
> Identifiers starting with _ are reserved by the system on file scope.

oh yes, switching between different programming languages never a good
thing for my brain :-)

Would the ffmpeg style be prefixing them up with anything, or just
leaving off the underscore?

Thanks

Kevin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread wm4
On Tue, 1 Sep 2015 12:02:06 +0100
Kevin Wheatley  wrote:

> From 59299c23489fadeb11330b51f83b152194f0810e Mon Sep 17 00:00:00 2001
> From: Kevin Wheatley 
> Date: Tue, 1 Sep 2015 11:41:38 +0100
> Subject: [PATCH 4/5] Add basic transfer functions for each 
> AVColorTransferCharacteristic
> 
> Most functions are valid over a domain and range of [0.0-1.0] but
> some are defined over greater. This patch does not deal with
> AVColorRange and assumes AVCOL_RANGE_JPEG for the returned values.
> 
> Signed-off-by: Kevin Wheatley 
> ---
>  libavutil/color_utils.c |  166 
> +++
>  libavutil/color_utils.h |   17 +
>  2 files changed, 183 insertions(+), 0 deletions(-)
> 
> diff --git a/libavutil/color_utils.c b/libavutil/color_utils.c
> index 59146be..f84b5f5 100644
> --- a/libavutil/color_utils.c
> +++ b/libavutil/color_utils.c
> @@ -18,6 +18,9 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> +#include 
> +#include 
> +
>  #include "libavutil/color_utils.h"
>  #include "libavutil/pixfmt.h"
>  
> @@ -50,3 +53,166 @@ double avpriv_get_gamma_from_trc(enum 
> AVColorTransferCharacteristic trc)
>  }
>  return gamma;
>  }
> +
> +#define BT709_alpha 1.099296826809442
> +#define BT709_beta 0.018053968510807
> +
> +static double _trc_bt709(double Lc)
> +{


Identifiers starting with _ are reserved by the system on file scope.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread Kevin Wheatley
On Tue, Sep 1, 2015 at 12:33 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Tue, Sep 1, 2015 at 7:02 AM, Kevin Wheatley 
> wrote:
>
>> Following on from my previous email, this adds some functions to
>> actually convert from linear to non-linear encoding. I have another
>> that changes the OpenEXR codec to actually use these.
>
>
> Won't performance be crumblingly slow with this kind of API? It seems to
> make more sense to write a filter that can convert between different TRC
> types, or do it in swscale, and then add some fast implementations
> (LUT-based, or approximation-based) alongside the slow versions.

I'm using the functions to construct a LUT in the OpenEXR reader when
possible, the cleanest design would be to pass floating point into the
swscale engine for sure, but I'm not sure I'd personally want to go
changing that code. I'm happy for some other approach to be put
forward, I have some users who wanted the feature, so I have something
that works for their needs so my urgency is minimal.

I've performed a few more tests and spotted something missing in one
of my earlier changes in the pixel description code, patch to
follow...

Kevin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread wm4
On Tue, 1 Sep 2015 07:33:26 -0400
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Tue, Sep 1, 2015 at 7:02 AM, Kevin Wheatley 
> wrote:
> 
> > Following on from my previous email, this adds some functions to
> > actually convert from linear to non-linear encoding. I have another
> > that changes the OpenEXR codec to actually use these.
> 
> 
> Won't performance be crumblingly slow with this kind of API? It seems to
> make more sense to write a filter that can convert between different TRC
> types, or do it in swscale, and then add some fast implementations
> (LUT-based, or approximation-based) alongside the slow versions.

I suppose these functions could be used to build a LUT in the first
place.

To do this in a filter, we'll probably need a float pixel format, and
of course float support in swscale. (Who volunteers?)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avutil: Add basic transfer functions for each AVColorTransferCharacteristic

2015-09-01 Thread Ronald S. Bultje
Hi,

On Tue, Sep 1, 2015 at 7:02 AM, Kevin Wheatley 
wrote:

> Following on from my previous email, this adds some functions to
> actually convert from linear to non-linear encoding. I have another
> that changes the OpenEXR codec to actually use these.


Won't performance be crumblingly slow with this kind of API? It seems to
make more sense to write a filter that can convert between different TRC
types, or do it in swscale, and then add some fast implementations
(LUT-based, or approximation-based) alongside the slow versions.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel