Re: [FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-30 Thread Christophe Gisquet
Hi,

2014-08-24 18:56 GMT+02:00 Michael Niedermayer michae...@gmx.at:
 aggree, a PRORES_PROFILE_AUTO seems like a good idea
 with it mismatching profile and pixel format can be errors while
 by default the profile can be selected based on the pixel format

The attached patch makes it selects either  or HQ profile
depending on whether there's an alpha channel. If a profile is
selected that does not have alpha but the input data has alpha, it
warns that it will not encode it.

Best regards,
-- 
Christophe
From 0b134da9eba4f6f08422d22a1a0428f564fe8f11 Mon Sep 17 00:00:00 2001
From: Christophe Gisquet christophe.gisq...@gmail.com
Date: Mon, 18 Aug 2014 11:27:50 +0200
Subject: [PATCH] proresenc_ks: allow auto-selecting profile

The user may not know how to select the profile, nor what he needs, in
particular to encode alpha.

Therefore, use an automatic selection as default, and warn when the
manually selected profile may cause issues.
---
 libavcodec/proresenc_kostya.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index c30c9c0..cdeb917 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -40,6 +40,7 @@
 #define MAX_PLANES 4
 
 enum {
+PRORES_PROFILE_AUTO  = -1,
 PRORES_PROFILE_PROXY = 0,
 PRORES_PROFILE_LT,
 PRORES_PROFILE_STANDARD,
@@ -1146,7 +1147,21 @@ static av_cold int encode_init(AVCodecContext *avctx)
there should be an integer power of two MBs per slice\n);
 return AVERROR(EINVAL);
 }
+if (ctx-profile == PRORES_PROFILE_AUTO) {
+ctx-profile = av_pix_fmt_desc_get(avctx-pix_fmt)-flags  AV_PIX_FMT_FLAG_ALPHA
+ ? PRORES_PROFILE_ : PRORES_PROFILE_HQ;
+av_log(avctx, AV_LOG_INFO, Autoselected %s. It can be overriden 
+   through -profile option.\n, ctx-profile == PRORES_PROFILE_
+   ? 4:4:4:4 profile because of the alpha channel
+   : HQ profile to keep best quality);
+}
 if (av_pix_fmt_desc_get(avctx-pix_fmt)-flags  AV_PIX_FMT_FLAG_ALPHA) {
+if (ctx-profile != PRORES_PROFILE_) {
+// force alpha and warn
+av_log(avctx, AV_LOG_WARNING, Profile selected will not 
+   encode alpha. Override with -profile if needed.\n);
+ctx-alpha_bits = 0;
+}
 if (ctx-alpha_bits  7) {
 av_log(avctx, AV_LOG_ERROR, alpha bits should be 0, 8 or 16\n);
 return AVERROR(EINVAL);
@@ -1280,8 +1295,10 @@ static const AVOption options[] = {
 { mbs_per_slice, macroblocks per slice, OFFSET(mbs_per_slice),
 AV_OPT_TYPE_INT, { .i64 = 8 }, 1, MAX_MBS_PER_SLICE, VE },
 { profile,   NULL, OFFSET(profile), AV_OPT_TYPE_INT,
-{ .i64 = PRORES_PROFILE_STANDARD },
-PRORES_PROFILE_PROXY, PRORES_PROFILE_, VE, profile },
+{ .i64 = PRORES_PROFILE_AUTO },
+PRORES_PROFILE_AUTO, PRORES_PROFILE_, VE, profile },
+{ auto, NULL, 0, AV_OPT_TYPE_CONST, { .i64 = PRORES_PROFILE_AUTO },
+0, 0, VE, profile },
 { proxy, NULL, 0, AV_OPT_TYPE_CONST, { .i64 = PRORES_PROFILE_PROXY },
 0, 0, VE, profile },
 { lt,NULL, 0, AV_OPT_TYPE_CONST, { .i64 = PRORES_PROFILE_LT },
-- 
1.9.2.msysgit.0

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


Re: [FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-30 Thread Michael Niedermayer
On Sat, Aug 30, 2014 at 01:13:56PM +0200, Christophe Gisquet wrote:
 Hi,
 
 2014-08-24 18:56 GMT+02:00 Michael Niedermayer michae...@gmx.at:
  aggree, a PRORES_PROFILE_AUTO seems like a good idea
  with it mismatching profile and pixel format can be errors while
  by default the profile can be selected based on the pixel format
 
 The attached patch makes it selects either  or HQ profile
 depending on whether there's an alpha channel. If a profile is
 selected that does not have alpha but the input data has alpha, it
 warns that it will not encode it.
 
 Best regards,
 -- 
 Christophe

  proresenc_kostya.c |   21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)
 cf627c08cf26aa889a17ded6b3e9684115eec296  
 0001-proresenc_ks-allow-auto-selecting-profile.patch
 From 0b134da9eba4f6f08422d22a1a0428f564fe8f11 Mon Sep 17 00:00:00 2001
 From: Christophe Gisquet christophe.gisq...@gmail.com
 Date: Mon, 18 Aug 2014 11:27:50 +0200
 Subject: [PATCH] proresenc_ks: allow auto-selecting profile
 
 The user may not know how to select the profile, nor what he needs, in
 particular to encode alpha.
 
 Therefore, use an automatic selection as default, and warn when the
 manually selected profile may cause issues.

applied

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


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


Re: [FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-24 Thread Michael Niedermayer
On Mon, Aug 18, 2014 at 07:26:32PM +0200, Christophe Gisquet wrote:
 Hi,
 
 2014-08-18 13:40 GMT+02:00 Carl Eugen Hoyos ceho...@ag.or.at:
  Christophe Gisquet christophe.gisquet at gmail.com writes:
  If a profile was set, the automatic setting should
  probably not overwrite it.
  (If this is possible.)
 
  But I consider the usecase where a user wants to encode
  alpha but at the same time using a profile that
  indicates no alpha of very limited usefulness.
 
 0001-proresenc-skip-warn-on-incorrect-profile.patch is the
 continuation of the first patch: we still encode the alpha with the
 incorrect profile (ie no change in behaviour) thereby not overriding
 the user decision. If he selected strict conformance and a profile
 without alpha, alpha channel coding is skipped.
 

 Second patch 0001-proresenc-force-correct-profile-for-alpha.patch
 overrides the profile information. The default profile is
 PRORES_PROFILE_STANDARD, maye a PRORES_PROFILE_AUTO could be
 introduced to control the behavior of this patch.

aggree, a PRORES_PROFILE_AUTO seems like a good idea
with it mismatching profile and pixel format can be errors while
by default the profile can be selected based on the pixel format


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


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


[FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-18 Thread Christophe Gisquet
Hi,

Ticket #3846 is about an incorrect profile being used:
- The user wants alpha but the profile indicates to software there's no alpha
- Also, the encoder still encodes the alpha channel in spite of the profile

The attached patch is not the most user-friendly, but is preferred by
the encoder author, and has the merit to leave the final decision to
the user.

An alternative would be to select the profile based on the pixel
format. If a user wants to encode without alpha, he would then need to
change the pixel format as well as specify the proper profile, which
may not be obvious to him.

-- 
Christophe
From d9679c80df991e98b1f00fd15f9c1c3c689ec426 Mon Sep 17 00:00:00 2001
From: Christophe Gisquet christophe.gisq...@gmail.com
Date: Mon, 18 Aug 2014 11:27:50 +0200
Subject: [PATCH] proresenc_kostya: warn/reject on incorrect profile

This fixes the following situations:
- Reject encoding when profile selected encodes alpha, but the pixel format
  does not have an alpha channel;
- Warn if the pixel format has an alpha channel but the profile does not
  encode it;
- Do not encode alpha if the profile does not specify it.
---
 libavcodec/proresenc_kostya.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index 46f81db..d2419e9c 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -1145,11 +1145,22 @@ static av_cold int encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 if (av_pix_fmt_desc_get(avctx-pix_fmt)-flags  AV_PIX_FMT_FLAG_ALPHA) {
-if (ctx-alpha_bits  7) {
+if (ctx-profile != PRORES_PROFILE_) {
+// ignore alpha but warn
+av_log(avctx, AV_LOG_WARNING, If you want alpha to be encoded,
+   please specify -profile \n);
+ctx-alpha_bits = 0;
+}
+else if (ctx-alpha_bits  7) {
 av_log(avctx, AV_LOG_ERROR, alpha bits should be 0, 8 or 16\n);
 return AVERROR(EINVAL);
 }
 } else {
+if (ctx-profile == PRORES_PROFILE_) {
+av_log(avctx, AV_LOG_ERROR, attempt to encode alpha while 
+   content has no alpha\n);
+return AVERROR(EINVAL);
+}
 ctx-alpha_bits = 0;
 }
 
-- 
1.9.2.msysgit.0

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


Re: [FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-18 Thread Carl Eugen Hoyos
Christophe Gisquet christophe.gisquet at gmail.com writes:

 An alternative would be to select the profile based on 
 the pixel format.

What would be the disadvantage in this case?

 If a user wants to encode without alpha, he would then 
 need to change the pixel format as well as specify the 
 proper profile, which may not be obvious to him.

If a profile was set, the automatic setting should 
probably not overwrite it.
(If this is possible.)

But I consider the usecase where a user wants to encode 
alpha but at the same time using a profile that 
indicates no alpha of very limited usefulness.

Carl Eugen

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


Re: [FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-18 Thread Christophe Gisquet
2014-08-18 19:26 GMT+02:00 Christophe Gisquet christophe.gisq...@gmail.com:
 Hi,

And with all patches.

-- 
Christophe
From 89b54fbd698eb9c68f0bf30105bcaf72c5575d27 Mon Sep 17 00:00:00 2001
From: Christophe Gisquet christophe.gisq...@gmail.com
Date: Mon, 18 Aug 2014 11:27:50 +0200
Subject: [PATCH] proresenc: force correct profile for alpha

Irrespective of the strictness level, if the pixel format contains alpha,
the profile is forced to be 4:4:4:4. Otherwise, the file may contain
alpha, but software like Final Cut Pro or After Effects will consider
this file to not contain alpha data.
---
 libavcodec/proresenc_kostya.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index 46f81db..08ea906 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -1145,6 +1145,13 @@ static av_cold int encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 if (av_pix_fmt_desc_get(avctx-pix_fmt)-flags  AV_PIX_FMT_FLAG_ALPHA) {
+if (ctx-profile != PRORES_PROFILE_) {
+// force alpha and warn
+av_log(avctx, AV_LOG_WARNING, Profile 4:4:4:4 forced to 
+   allow encoding of alpha channel. Change pixel format
+if you actually want this.\n);
+ctx-profile = PRORES_PROFILE_;
+}
 if (ctx-alpha_bits  7) {
 av_log(avctx, AV_LOG_ERROR, alpha bits should be 0, 8 or 16\n);
 return AVERROR(EINVAL);
-- 
1.9.2.msysgit.0

From f24ec2b80392918e436fd5b5564b0641e37131f9 Mon Sep 17 00:00:00 2001
From: Christophe Gisquet christophe.gisq...@gmail.com
Date: Mon, 18 Aug 2014 11:27:50 +0200
Subject: [PATCH] proresenc: skip/warn on incorrect profile

This does, if strictness level is high enough:
- Skip encoding of alpha when profile does not allow it;
- Warn about profile allowing alpha when content does not have alpha.

Some software such as After Effects and Final Cut Pro entirely skip
alpha channel decoding if the profile is not .
---
 libavcodec/proresenc_kostya.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index 46f81db..90e9d74 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -1145,11 +1145,22 @@ static av_cold int encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 if (av_pix_fmt_desc_get(avctx-pix_fmt)-flags  AV_PIX_FMT_FLAG_ALPHA) {
-if (ctx-alpha_bits  7) {
+if (ctx-profile != PRORES_PROFILE_ 
+avctx-strict_std_compliance  FF_COMPLIANCE_NORMAL) {
+// ignore alpha but warn
+av_log(avctx, AV_LOG_WARNING, If you want alpha to be 
+   encoded, please specify -profile \n);
+ctx-alpha_bits = 0;
+} else if (ctx-alpha_bits  7) {
 av_log(avctx, AV_LOG_ERROR, alpha bits should be 0, 8 or 16\n);
 return AVERROR(EINVAL);
 }
 } else {
+if (ctx-profile == PRORES_PROFILE_ 
+avctx-strict_std_compliance  FF_COMPLIANCE_NORMAL)
+av_log(avctx, AV_LOG_WARNING, profile with alpha but 
+   content has no alpha\n);
+
 ctx-alpha_bits = 0;
 }
 
-- 
1.9.2.msysgit.0

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


Re: [FFmpeg-devel] [RFC] [PATCH] proresenc_kostya: warn/reject on incorrect profile

2014-08-18 Thread Christophe Gisquet
Hi,

2014-08-18 13:40 GMT+02:00 Carl Eugen Hoyos ceho...@ag.or.at:
 Christophe Gisquet christophe.gisquet at gmail.com writes:
 If a profile was set, the automatic setting should
 probably not overwrite it.
 (If this is possible.)

 But I consider the usecase where a user wants to encode
 alpha but at the same time using a profile that
 indicates no alpha of very limited usefulness.

0001-proresenc-skip-warn-on-incorrect-profile.patch is the
continuation of the first patch: we still encode the alpha with the
incorrect profile (ie no change in behaviour) thereby not overriding
the user decision. If he selected strict conformance and a profile
without alpha, alpha channel coding is skipped.

Second patch 0001-proresenc-force-correct-profile-for-alpha.patch
overrides the profile information. The default profile is
PRORES_PROFILE_STANDARD, maye a PRORES_PROFILE_AUTO could be
introduced to control the behavior of this patch.

-- 
Christophe
From f24ec2b80392918e436fd5b5564b0641e37131f9 Mon Sep 17 00:00:00 2001
From: Christophe Gisquet christophe.gisq...@gmail.com
Date: Mon, 18 Aug 2014 11:27:50 +0200
Subject: [PATCH] proresenc: skip/warn on incorrect profile

This does, if strictness level is high enough:
- Skip encoding of alpha when profile does not allow it;
- Warn about profile allowing alpha when content does not have alpha.

Some software such as After Effects and Final Cut Pro entirely skip
alpha channel decoding if the profile is not .
---
 libavcodec/proresenc_kostya.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index 46f81db..90e9d74 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -1145,11 +1145,22 @@ static av_cold int encode_init(AVCodecContext *avctx)
 return AVERROR(EINVAL);
 }
 if (av_pix_fmt_desc_get(avctx-pix_fmt)-flags  AV_PIX_FMT_FLAG_ALPHA) {
-if (ctx-alpha_bits  7) {
+if (ctx-profile != PRORES_PROFILE_ 
+avctx-strict_std_compliance  FF_COMPLIANCE_NORMAL) {
+// ignore alpha but warn
+av_log(avctx, AV_LOG_WARNING, If you want alpha to be 
+   encoded, please specify -profile \n);
+ctx-alpha_bits = 0;
+} else if (ctx-alpha_bits  7) {
 av_log(avctx, AV_LOG_ERROR, alpha bits should be 0, 8 or 16\n);
 return AVERROR(EINVAL);
 }
 } else {
+if (ctx-profile == PRORES_PROFILE_ 
+avctx-strict_std_compliance  FF_COMPLIANCE_NORMAL)
+av_log(avctx, AV_LOG_WARNING, profile with alpha but 
+   content has no alpha\n);
+
 ctx-alpha_bits = 0;
 }
 
-- 
1.9.2.msysgit.0

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