Re: [libav-devel] [PATCH 2/5] pixdesc: Make sure color properties names match x264
Quoting Vittorio Giovara (2016-06-16 00:43:26) > On Wed, Jun 15, 2016 at 5:31 PM, Hendrik Leppkeswrote: > > 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
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
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
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
On Thu, Jun 16, 2016 at 1:27 PM, Vittorio Giovarawrote: > 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
On Thu, Jun 16, 2016 at 1:09 PM, Luca Barbatowrote: > 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
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
On Thu, Jun 16, 2016 at 4:31 AM, Diego Biurrunwrote: > 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
On Wed, Jun 15, 2016 at 03:08:57PM -0400, Vittorio Giovara wrote: > On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkeswrote: > > 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
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
On Wed, Jun 15, 2016 at 5:31 PM, Hendrik Leppkeswrote: > 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
On Wed, Jun 15, 2016 at 11:06 PM, Vittorio Giovarawrote: > 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
On Wed, Jun 15, 2016 at 3:49 PM, Hendrik Leppkeswrote: > 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
On Wed, Jun 15, 2016 at 9:08 PM, Vittorio Giovarawrote: > 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
On Wed, Jun 15, 2016 at 3:00 PM, Hendrik Leppkeswrote: > 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
On Wed, Jun 15, 2016 at 8:53 PM, Luca Barbatowrote: > 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
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
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