Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-21 Thread Anton Khirnov
Quoting Vittorio Giovara (2016-06-16 00:43:26)
> On Wed, Jun 15, 2016 at 5:31 PM, Hendrik Leppkes  wrote:
> > On Wed, Jun 15, 2016 at 11:06 PM, Vittorio Giovara
> >  wrote:
> >> On Wed, Jun 15, 2016 at 3:49 PM, Hendrik Leppkes  
> >> wrote:
> >>> On Wed, Jun 15, 2016 at 9:08 PM, Vittorio Giovara
> >>>  wrote:
>  On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  
>  wrote:
> > On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  
> > wrote:
> >> On 15/06/16 20:25, Vittorio Giovara wrote:
> >>> This allows to directly use av_color_*_name() return value as
> >>> command line parameters for the encoder.
> >>> ---
> >>>  libavutil/pixdesc.c | 8 
> >>>  libavutil/version.h | 2 +-
> >>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> I consider the previous names bugs.
> >>
> >
> > Having the string names mismatch the symbol name seems like the real 
> > bug to me.
> 
>  The alternative is to replace/deprecate symbol names, I'm not sure
>  whether a symbol name bikeshed warrants an API break (which would be
>  difficult to track since there cannot be guards for enum names).
> >>>
> >>> The alternative is to keep it as-is, imho.
> >>
> >> That defies the whole existence of this API and the rationale behind this 
> >> set.
> >
> > Your whole set lacks reason, it looks like arbitrary renames to me
> > because you like them better.
> 
> I guess you mean that you don't see the reason, not that it lacks it.
> 
> The API I wrote has been intended for that use since day one, to be
> compatible with x264 settings: a very common usecase is to use
> properties from probe json (which is filled by these functions) and
> use them to call x264 from command line, without having to introduce a
> conversion table from day one.
> 
> Unfortunately this got lost during the addition of new elements to the
> enum, and with the adoption of different names from x264. Now that
> they have been settled though, it seems wise to me to restore this
> functionality. Also only very recent and very new elements are
> affected, which are hardly available to distros probably.

Still, I have to agree with Hendrik that mismatching enum names and
string names is evil. Does anything prevent you from adding aliases for
the enum names as well?

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-21 Thread Anton Khirnov
Quoting Luca Barbato (2016-06-16 01:34:39)
> On 16/06/16 00:43, Vittorio Giovara wrote:
> > Finally with the addition of 4/5 we will be able to use these names
> > when calling the conversion tool too, so we will finally be compatible
> > with ourselves.
> 
> As said, this is a bugfix, at least one of the names before this patch
> looks like a typo.

No, it does not work this way. Yes, that one change may be a bugfix, but
that does not mean the other ones are.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-21 Thread Anton Khirnov
Quoting Luca Barbato (2016-06-16 19:09:45)
> On 16/06/16 18:56, Vittorio Giovara wrote:
> > The symbols with the unfortunate name were added in 4a66422 (17 Sep
> > 2015), they were not part of the 11 release, nor is the API in
> > question ever released in any libav release.
> 
> Then we can change it as you like IMO.

I very very strongly disagree with this "it was not in a release so you
can break it however" stance. Our git master is stable. We do NOT break
it (unless in very special circumstances).

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-20 Thread Luca Barbato
On 20/06/16 20:18, Vittorio Giovara wrote:
> On Thu, Jun 16, 2016 at 1:27 PM, Vittorio Giovara
>  wrote:
>> On Thu, Jun 16, 2016 at 1:09 PM, Luca Barbato  wrote:
>>> On 16/06/16 18:56, Vittorio Giovara wrote:
 The symbols with the unfortunate name were added in 4a66422 (17 Sep
 2015), they were not part of the 11 release, nor is the API in
 question ever released in any libav release.
>>>
>>> Then we can change it as you like IMO.
>>
>> Sure, I just hope the message conveyed is not "because I like it" but
>> rather "because it makes sense to do".
>> --
>> Vittorio
> 
> Any more comments on this set? Perhaps should I add a note in the
> functions documentation?
> 

More documentation is always good.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-20 Thread Vittorio Giovara
On Thu, Jun 16, 2016 at 1:27 PM, Vittorio Giovara
 wrote:
> On Thu, Jun 16, 2016 at 1:09 PM, Luca Barbato  wrote:
>> On 16/06/16 18:56, Vittorio Giovara wrote:
>>> The symbols with the unfortunate name were added in 4a66422 (17 Sep
>>> 2015), they were not part of the 11 release, nor is the API in
>>> question ever released in any libav release.
>>
>> Then we can change it as you like IMO.
>
> Sure, I just hope the message conveyed is not "because I like it" but
> rather "because it makes sense to do".
> --
> Vittorio

Any more comments on this set? Perhaps should I add a note in the
functions documentation?
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-16 Thread Vittorio Giovara
On Thu, Jun 16, 2016 at 1:09 PM, Luca Barbato  wrote:
> On 16/06/16 18:56, Vittorio Giovara wrote:
>> The symbols with the unfortunate name were added in 4a66422 (17 Sep
>> 2015), they were not part of the 11 release, nor is the API in
>> question ever released in any libav release.
>
> Then we can change it as you like IMO.

Sure, I just hope the message conveyed is not "because I like it" but
rather "because it makes sense to do".
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-16 Thread Luca Barbato
On 16/06/16 18:56, Vittorio Giovara wrote:
> The symbols with the unfortunate name were added in 4a66422 (17 Sep
> 2015), they were not part of the 11 release, nor is the API in
> question ever released in any libav release.

Then we can change it as you like IMO.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-16 Thread Vittorio Giovara
On Thu, Jun 16, 2016 at 4:31 AM, Diego Biurrun  wrote:
> On Wed, Jun 15, 2016 at 03:08:57PM -0400, Vittorio Giovara wrote:
>> On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  wrote:
>> > On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
>> >> On 15/06/16 20:25, Vittorio Giovara wrote:
>> >>> This allows to directly use av_color_*_name() return value as
>> >>> command line parameters for the encoder.
>> >>> ---
>> >>>  libavutil/pixdesc.c | 8 
>> >>>  libavutil/version.h | 2 +-
>> >>>  2 files changed, 5 insertions(+), 5 deletions(-)
>> >>
>> >> I consider the previous names bugs.
>> >
>> > Having the string names mismatch the symbol name seems like the real bug 
>> > to me.
>>
>> The alternative is to replace/deprecate symbol names, I'm not sure
>> whether a symbol name bikeshed warrants an API break (which would be
>> difficult to track since there cannot be guards for enum names).
>
> When were these symbols added?  Were they part of a release yet?
> If not...

The symbols with the unfortunate name were added in 4a66422 (17 Sep
2015), they were not part of the 11 release, nor is the API in
question ever released in any libav release.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-16 Thread Diego Biurrun
On Wed, Jun 15, 2016 at 03:08:57PM -0400, Vittorio Giovara wrote:
> On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  wrote:
> > On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
> >> On 15/06/16 20:25, Vittorio Giovara wrote:
> >>> This allows to directly use av_color_*_name() return value as
> >>> command line parameters for the encoder.
> >>> ---
> >>>  libavutil/pixdesc.c | 8 
> >>>  libavutil/version.h | 2 +-
> >>>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> I consider the previous names bugs.
> >
> > Having the string names mismatch the symbol name seems like the real bug to 
> > me.
> 
> The alternative is to replace/deprecate symbol names, I'm not sure
> whether a symbol name bikeshed warrants an API break (which would be
> difficult to track since there cannot be guards for enum names).

When were these symbols added?  Were they part of a release yet?
If not...

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Luca Barbato
On 16/06/16 00:43, Vittorio Giovara wrote:
> Finally with the addition of 4/5 we will be able to use these names
> when calling the conversion tool too, so we will finally be compatible
> with ourselves.

As said, this is a bugfix, at least one of the names before this patch
looks like a typo.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Vittorio Giovara
On Wed, Jun 15, 2016 at 5:31 PM, Hendrik Leppkes  wrote:
> On Wed, Jun 15, 2016 at 11:06 PM, Vittorio Giovara
>  wrote:
>> On Wed, Jun 15, 2016 at 3:49 PM, Hendrik Leppkes  wrote:
>>> On Wed, Jun 15, 2016 at 9:08 PM, Vittorio Giovara
>>>  wrote:
 On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  
 wrote:
> On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
>> On 15/06/16 20:25, Vittorio Giovara wrote:
>>> This allows to directly use av_color_*_name() return value as
>>> command line parameters for the encoder.
>>> ---
>>>  libavutil/pixdesc.c | 8 
>>>  libavutil/version.h | 2 +-
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> I consider the previous names bugs.
>>
>
> Having the string names mismatch the symbol name seems like the real bug 
> to me.

 The alternative is to replace/deprecate symbol names, I'm not sure
 whether a symbol name bikeshed warrants an API break (which would be
 difficult to track since there cannot be guards for enum names).
>>>
>>> The alternative is to keep it as-is, imho.
>>
>> That defies the whole existence of this API and the rationale behind this 
>> set.
>
> Your whole set lacks reason, it looks like arbitrary renames to me
> because you like them better.

I guess you mean that you don't see the reason, not that it lacks it.

The API I wrote has been intended for that use since day one, to be
compatible with x264 settings: a very common usecase is to use
properties from probe json (which is filled by these functions) and
use them to call x264 from command line, without having to introduce a
conversion table from day one.

Unfortunately this got lost during the addition of new elements to the
enum, and with the adoption of different names from x264. Now that
they have been settled though, it seems wise to me to restore this
functionality. Also only very recent and very new elements are
affected, which are hardly available to distros probably.

Finally with the addition of 4/5 we will be able to use these names
when calling the conversion tool too, so we will finally be compatible
with ourselves.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Hendrik Leppkes
On Wed, Jun 15, 2016 at 11:06 PM, Vittorio Giovara
 wrote:
> On Wed, Jun 15, 2016 at 3:49 PM, Hendrik Leppkes  wrote:
>> On Wed, Jun 15, 2016 at 9:08 PM, Vittorio Giovara
>>  wrote:
>>> On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  
>>> wrote:
 On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
> On 15/06/16 20:25, Vittorio Giovara wrote:
>> This allows to directly use av_color_*_name() return value as
>> command line parameters for the encoder.
>> ---
>>  libavutil/pixdesc.c | 8 
>>  libavutil/version.h | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> I consider the previous names bugs.
>

 Having the string names mismatch the symbol name seems like the real bug 
 to me.
>>>
>>> The alternative is to replace/deprecate symbol names, I'm not sure
>>> whether a symbol name bikeshed warrants an API break (which would be
>>> difficult to track since there cannot be guards for enum names).
>>
>> The alternative is to keep it as-is, imho.
>
> That defies the whole existence of this API and the rationale behind this set.

Your whole set lacks reason, it looks like arbitrary renames to me
because you like them better.

- Hendrik
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Vittorio Giovara
On Wed, Jun 15, 2016 at 3:49 PM, Hendrik Leppkes  wrote:
> On Wed, Jun 15, 2016 at 9:08 PM, Vittorio Giovara
>  wrote:
>> On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  wrote:
>>> On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
 On 15/06/16 20:25, Vittorio Giovara wrote:
> This allows to directly use av_color_*_name() return value as
> command line parameters for the encoder.
> ---
>  libavutil/pixdesc.c | 8 
>  libavutil/version.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)

 I consider the previous names bugs.

>>>
>>> Having the string names mismatch the symbol name seems like the real bug to 
>>> me.
>>
>> The alternative is to replace/deprecate symbol names, I'm not sure
>> whether a symbol name bikeshed warrants an API break (which would be
>> difficult to track since there cannot be guards for enum names).
>
> The alternative is to keep it as-is, imho.

That defies the whole existence of this API and the rationale behind this set.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Hendrik Leppkes
On Wed, Jun 15, 2016 at 9:08 PM, Vittorio Giovara
 wrote:
> On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  wrote:
>> On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
>>> On 15/06/16 20:25, Vittorio Giovara wrote:
 This allows to directly use av_color_*_name() return value as
 command line parameters for the encoder.
 ---
  libavutil/pixdesc.c | 8 
  libavutil/version.h | 2 +-
  2 files changed, 5 insertions(+), 5 deletions(-)
>>>
>>> I consider the previous names bugs.
>>>
>>
>> Having the string names mismatch the symbol name seems like the real bug to 
>> me.
>
> The alternative is to replace/deprecate symbol names, I'm not sure
> whether a symbol name bikeshed warrants an API break (which would be
> difficult to track since there cannot be guards for enum names).

The alternative is to keep it as-is, imho.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Vittorio Giovara
On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkes  wrote:
> On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
>> On 15/06/16 20:25, Vittorio Giovara wrote:
>>> This allows to directly use av_color_*_name() return value as
>>> command line parameters for the encoder.
>>> ---
>>>  libavutil/pixdesc.c | 8 
>>>  libavutil/version.h | 2 +-
>>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> I consider the previous names bugs.
>>
>
> Having the string names mismatch the symbol name seems like the real bug to 
> me.

The alternative is to replace/deprecate symbol names, I'm not sure
whether a symbol name bikeshed warrants an API break (which would be
difficult to track since there cannot be guards for enum names).
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Hendrik Leppkes
On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbato  wrote:
> On 15/06/16 20:25, Vittorio Giovara wrote:
>> This allows to directly use av_color_*_name() return value as
>> command line parameters for the encoder.
>> ---
>>  libavutil/pixdesc.c | 8 
>>  libavutil/version.h | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> I consider the previous names bugs.
>

Having the string names mismatch the symbol name seems like the real bug to me.

- Hendrik
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Luca Barbato
On 15/06/16 20:25, Vittorio Giovara wrote:
> This allows to directly use av_color_*_name() return value as
> command line parameters for the encoder.
> ---
>  libavutil/pixdesc.c | 8 
>  libavutil/version.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)

I consider the previous names bugs.

Ok.

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


[libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264

2016-06-15 Thread Vittorio Giovara
This allows to directly use av_color_*_name() return value as
command line parameters for the encoder.
---
 libavutil/pixdesc.c | 8 
 libavutil/version.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 209d107..216452c 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -1612,7 +1612,7 @@ static const char *color_primaries_names[] = {
 [AVCOL_PRI_SMPTE240M] = "smpte240m",
 [AVCOL_PRI_FILM] = "film",
 [AVCOL_PRI_BT2020] = "bt2020",
-[AVCOL_PRI_SMPTEST428_1] = "smptest428-1",
+[AVCOL_PRI_SMPTEST428_1] = "smpte428",
 };
 
 static const char *color_transfer_names[] = {
@@ -1631,9 +1631,9 @@ static const char *color_transfer_names[] = {
 [AVCOL_TRC_BT1361_ECG] = "bt1361e",
 [AVCOL_TRC_IEC61966_2_1] = "iec61966-2-1",
 [AVCOL_TRC_BT2020_10] = "bt2020-10",
-[AVCOL_TRC_BT2020_12] = "bt2020-20",
-[AVCOL_TRC_SMPTEST2084] = "smptest2084",
-[AVCOL_TRC_SMPTEST428_1] = "smptest428-1",
+[AVCOL_TRC_BT2020_12] = "bt2020-12",
+[AVCOL_TRC_SMPTEST2084] = "smpte2084",
+[AVCOL_TRC_SMPTEST428_1] = "smpte428",
 [AVCOL_TRC_ARIB_STD_B67] = "arib-std-b67",
 };
 
diff --git a/libavutil/version.h b/libavutil/version.h
index 7ea434e..09d3713 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -54,7 +54,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR 55
-#define LIBAVUTIL_VERSION_MINOR 14
+#define LIBAVUTIL_VERSION_MINOR 15
 #define LIBAVUTIL_VERSION_MICRO  0
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.8.3

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel