[FFmpeg-devel] comma at the end of enumerator lists
Hi all, first of all thanks for providing this great library. Today my pull request https://github.com/FFmpeg/FFmpeg/pull/237 has been rejected which tried to remove the comma at the end of enumerator lists which triggers a warning when compiling with gcc and -Wpedantic. I still think it could be a good idea to remove this, especially in the cases where the enumerator list already ends in some _NB item, which seems to be the final one "forever", but I am here to learn :-). Best regards, Michael signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] comma at the end of enumerator lists
Am 16.10.2016 um 23:24 schrieb Clément Bœsch: > On Sun, Oct 16, 2016 at 09:46:20PM +0200, Michael Behrisch wrote: >> Hi all, >> first of all thanks for providing this great library. >> >> Today my pull request https://github.com/FFmpeg/FFmpeg/pull/237 has been >> rejected which tried to remove the comma at the end of enumerator lists >> which triggers a warning when compiling with gcc and -Wpedantic. I still >> think it could be a good idea to remove this, especially in the cases >> where the enumerator list already ends in some _NB item, which seems to >> be the final one "forever", but I am here to learn :-). > > The enum with a final _NB (or similar) entry are the only ones where > removing the comma is relevant. > So would a patch removing only those have a chance of being accepted? Best regards, Michael signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] comma at the end of enumerator lists
Am 17.10.2016 um 15:29 schrieb Michael Niedermayer:
> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote:
>> On Mon, 17 Oct 2016 13:09:36 +0200
>> Michael Niedermayer wrote:
>>
>>> On Mon, Oct 17, 2016 at 10:07:42AM +0200, Nicolas George wrote:
Le sextidi 26 vendémiaire, an CCXXV, Michael Niedermayer a écrit :
> probably, yes
I would have said exactly the opposite. It is nothing but a waste of time
and a pollution of the history.
>>>
>>> My idea here is to maximize the number of developers
>>> And if in cases where one doesnt really care much either way and
>>> someone else seems caring more one says, "ok" that may result in a happy
>>> new contributor.
>>> Saying "no" is more likely to turn someone away.
>>> and again, it doesnt really matter if the , is there after a
>>> final sentinel /count entry as no next field would ever be added
>>
>> Are you kidding me. Patches should be judged on their technical merrit,
>> not whether you might piss someone off by rejecting it.
>
> this is about a cosmetic change having no real technical effect
So here are my cosmetics for libavutil. It simply helps with keeping
track of real warnings in downstream projects.
Best regards,
Michael
diff -rup FFmpeg-master/libavutil/log.h FFmpeg/libavutil/log.h
--- FFmpeg-master/libavutil/log.h 2016-10-17 19:47:57.0 +0200
+++ FFmpeg/libavutil/log.h 2016-10-17 20:30:38.545498875 +0200
@@ -44,7 +44,7 @@ typedef enum {
AV_CLASS_CATEGORY_DEVICE_AUDIO_INPUT,
AV_CLASS_CATEGORY_DEVICE_OUTPUT,
AV_CLASS_CATEGORY_DEVICE_INPUT,
-AV_CLASS_CATEGORY_NB, ///< not part of ABI/API
+AV_CLASS_CATEGORY_NB ///< not part of ABI/API
}AVClassCategory;
#define AV_IS_INPUT_DEVICE(category) \
diff -rup FFmpeg-master/libavutil/pixfmt.h FFmpeg/libavutil/pixfmt.h
--- FFmpeg-master/libavutil/pixfmt.h 2016-10-17 19:47:57.0 +0200
+++ FFmpeg/libavutil/pixfmt.h 2016-10-17 20:30:38.565499123 +0200
@@ -306,7 +306,7 @@ enum AVPixelFormat {
AV_PIX_FMT_MEDIACODEC, ///< hardware decoding through MediaCodec
-AV_PIX_FMT_NB,///< number of pixel formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions
+AV_PIX_FMT_NB ///< number of pixel formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions
};
#if AV_HAVE_BIGENDIAN
@@ -401,7 +401,7 @@ enum AVColorPrimaries {
AVCOL_PRI_SMPTEST428_1 = 10, ///< SMPTE ST 428-1 (CIE 1931 XYZ)
AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011)
AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 D65 (2010)
-AVCOL_PRI_NB, ///< Not part of ABI
+AVCOL_PRI_NB///< Not part of ABI
};
/**
@@ -427,7 +427,7 @@ enum AVColorTransferCharacteristic {
AVCOL_TRC_SMPTEST2084 = 16, ///< SMPTE ST 2084 for 10-, 12-, 14- and 16-bit systems
AVCOL_TRC_SMPTEST428_1 = 17, ///< SMPTE ST 428-1
AVCOL_TRC_ARIB_STD_B67 = 18, ///< ARIB STD-B67, known as "Hybrid log-gamma"
-AVCOL_TRC_NB,///< Not part of ABI
+AVCOL_TRC_NB ///< Not part of ABI
};
/**
@@ -446,7 +446,7 @@ enum AVColorSpace {
AVCOL_SPC_BT2020_NCL = 9, ///< ITU-R BT2020 non-constant luminance system
AVCOL_SPC_BT2020_CL = 10, ///< ITU-R BT2020 constant luminance system
AVCOL_SPC_SMPTE2085 = 11, ///< SMPTE 2085, Y'D'zD'x
-AVCOL_SPC_NB, ///< Not part of ABI
+AVCOL_SPC_NB///< Not part of ABI
};
#define AVCOL_SPC_YCGCO AVCOL_SPC_YCOCG
@@ -458,7 +458,7 @@ enum AVColorRange {
AVCOL_RANGE_UNSPECIFIED = 0,
AVCOL_RANGE_MPEG= 1, ///< the normal 219*2^(n-8) "MPEG" YUV ranges
AVCOL_RANGE_JPEG= 2, ///< the normal 2^n-1 "JPEG" YUV ranges
-AVCOL_RANGE_NB, ///< Not part of ABI
+AVCOL_RANGE_NB ///< Not part of ABI
};
/**
@@ -484,7 +484,7 @@ enum AVChromaLocation {
AVCHROMA_LOC_TOP = 4,
AVCHROMA_LOC_BOTTOMLEFT = 5,
AVCHROMA_LOC_BOTTOM = 6,
-AVCHROMA_LOC_NB, ///< Not part of ABI
+AVCHROMA_LOC_NB ///< Not part of ABI
};
#endif /* AVUTIL_PIXFMT_H */
signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] comma at the end of enumerator lists
Hi Ronald, Am 17.10.2016 um 21:37 schrieb Ronald S. Bultje: > Hi Michael, > > On Mon, Oct 17, 2016 at 3:16 PM, Michael Behrisch > wrote: > >> Am 17.10.2016 um 15:29 schrieb Michael Niedermayer: >>> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote: >>>> On Mon, 17 Oct 2016 13:09:36 +0200 Michael Niedermayer >>>> wrote: >>>> >>> this is about a cosmetic change having no real technical effect >> >> So here are my cosmetics for libavutil. It simply helps with >> keeping track of real warnings in downstream projects. > > > Why are you using -Wpedantic? My main reason is that we are compiling with different compilers for different platforms and -Wpedantic at least promises to keep the code closer to the standard and thus better transferable. I never tested whether this is actually true, but I like the fact that the project currently compiles with gcc, clang and msvc and welcome every tool and option that helps me to keep it this way. See also here: http://stackoverflow.com/questions/2855121/what-is-the-purpose-of-using-pedantic-in-gcc-g-compiler > > Most people use warnings as a way for the compiler to inform them of > potential bugs in their code; has -Wpedantic ever helped you find > bugs? I cannot think of any but to be honest I cannot even tell exactly which warnings are enabled by which of the -Wall, -Wextra and -Wpedantic flags and it is surprisingly hard to find out. Best regards, Michael signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] comma at the end of enumerator lists
Hi Ronald, Am 17.10.2016 um 23:45 schrieb Ronald S. Bultje: > Hi Michael, > > On Mon, Oct 17, 2016 at 5:23 PM, Michael Behrisch wrote: > >> Hi Ronald, >> >> Am 17.10.2016 um 21:37 schrieb Ronald S. Bultje: >>> Hi Michael, >>> >>> On Mon, Oct 17, 2016 at 3:16 PM, Michael Behrisch >>> wrote: >>> >>>> Am 17.10.2016 um 15:29 schrieb Michael Niedermayer: >>>>> On Mon, Oct 17, 2016 at 01:34:55PM +0200, wm4 wrote: >>>>>> On Mon, 17 Oct 2016 13:09:36 +0200 Michael Niedermayer >>>>>> wrote: >>>>>> >>>>> this is about a cosmetic change having no real technical effect >>>> >>>> So here are my cosmetics for libavutil. It simply helps with >>>> keeping track of real warnings in downstream projects. >>> >>> >>> Why are you using -Wpedantic? >> >> My main reason is that we are compiling with different compilers for >> different platforms and -Wpedantic at least promises to keep the code >> closer to the standard and thus better transferable. I never tested >> whether this is actually true, but I like the fact that the project >> currently compiles with gcc, clang and msvc and welcome every tool and >> option that helps me to keep it this way. See also here: >> http://stackoverflow.com/questions/2855121/what-is-the- >> purpose-of-using-pedantic-in-gcc-g-compiler >> >>> >>> Most people use warnings as a way for the compiler to inform them of >>> potential bugs in their code; has -Wpedantic ever helped you find >>> bugs? >> >> I cannot think of any but to be honest I cannot even tell exactly which >> warnings are enabled by which of the -Wall, -Wextra and -Wpedantic flags >> and it is surprisingly hard to find out. > > > FFmpeg compiles with msvc, clang and gcc (without -Wpedantic), so the logic > seems a little strange. I am not claiming that -Wpedantic is the only way to achieve this. I only think it helps or at least I assume that was the intention when it was invented. After all, that is one of the reasons why standards exist (in my opinion), to keep code compiler agnostic such that (in theory) I do not have to check by myself whether it compiles with all the compilers out there, as long as it adheres to the standard. And "-Wpedantic" is just a "standard" checker to me. Maybe it checks for an outdated version of the standard or for some version which is of no relevance to FFmpeg, then please tell me. Best regards, Michael signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] removing comma at final enumeration items to fix pedantic warnings
Signed-off-by: Michael Behrisch
---
libavutil/log.h| 2 +-
libavutil/pixfmt.h | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libavutil/log.h b/libavutil/log.h
index 0acc1b9..f0a5738 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -44,7 +44,7 @@ typedef enum {
AV_CLASS_CATEGORY_DEVICE_AUDIO_INPUT,
AV_CLASS_CATEGORY_DEVICE_OUTPUT,
AV_CLASS_CATEGORY_DEVICE_INPUT,
-AV_CLASS_CATEGORY_NB, ///< not part of ABI/API
+AV_CLASS_CATEGORY_NB ///< not part of ABI/API
}AVClassCategory;
#define AV_IS_INPUT_DEVICE(category) \
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index b15c0ef..7a3f68b 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -306,7 +306,7 @@ enum AVPixelFormat {
AV_PIX_FMT_MEDIACODEC, ///< hardware decoding through MediaCodec
-AV_PIX_FMT_NB,///< number of pixel formats, DO NOT USE THIS
if you want to link with shared libav* because the number of formats
might differ between versions
+AV_PIX_FMT_NB ///< number of pixel formats, DO NOT USE THIS
if you want to link with shared libav* because the number of formats
might differ between versions
};
#if AV_HAVE_BIGENDIAN
@@ -401,7 +401,7 @@ enum AVColorPrimaries {
AVCOL_PRI_SMPTEST428_1 = 10, ///< SMPTE ST 428-1 (CIE 1931 XYZ)
AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011)
AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 D65 (2010)
-AVCOL_PRI_NB, ///< Not part of ABI
+AVCOL_PRI_NB///< Not part of ABI
};
/**
@@ -427,7 +427,7 @@ enum AVColorTransferCharacteristic {
AVCOL_TRC_SMPTEST2084 = 16, ///< SMPTE ST 2084 for 10-, 12-, 14-
and 16-bit systems
AVCOL_TRC_SMPTEST428_1 = 17, ///< SMPTE ST 428-1
AVCOL_TRC_ARIB_STD_B67 = 18, ///< ARIB STD-B67, known as "Hybrid
log-gamma"
-AVCOL_TRC_NB,///< Not part of ABI
+AVCOL_TRC_NB ///< Not part of ABI
};
/**
@@ -446,7 +446,7 @@ enum AVColorSpace {
AVCOL_SPC_BT2020_NCL = 9, ///< ITU-R BT2020 non-constant
luminance system
AVCOL_SPC_BT2020_CL = 10, ///< ITU-R BT2020 constant luminance system
AVCOL_SPC_SMPTE2085 = 11, ///< SMPTE 2085, Y'D'zD'x
-AVCOL_SPC_NB, ///< Not part of ABI
+AVCOL_SPC_NB///< Not part of ABI
};
#define AVCOL_SPC_YCGCO AVCOL_SPC_YCOCG
@@ -458,7 +458,7 @@ enum AVColorRange {
AVCOL_RANGE_UNSPECIFIED = 0,
AVCOL_RANGE_MPEG= 1, ///< the normal 219*2^(n-8) "MPEG" YUV
ranges
AVCOL_RANGE_JPEG= 2, ///< the normal 2^n-1 "JPEG" YUV
ranges
-AVCOL_RANGE_NB, ///< Not part of ABI
+AVCOL_RANGE_NB ///< Not part of ABI
};
/**
@@ -484,7 +484,7 @@ enum AVChromaLocation {
AVCHROMA_LOC_TOP = 4,
AVCHROMA_LOC_BOTTOMLEFT = 5,
AVCHROMA_LOC_BOTTOM = 6,
-AVCHROMA_LOC_NB, ///< Not part of ABI
+AVCHROMA_LOC_NB ///< Not part of ABI
};
#endif /* AVUTIL_PIXFMT_H */
--
2.6.6
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] removing comma at final enumeration items to fix pedantic warnings
Am 20.10.2016 um 04:18 schrieb Muhammad Faiz: > On 10/20/16, Michael Behrisch wrote: >> Signed-off-by: Michael Behrisch >> --- >> libavutil/log.h| 2 +- >> libavutil/pixfmt.h | 12 ++-- >> 2 files changed, 7 insertions(+), 7 deletions(-) > > If you have difficulty to send your patch directly (you should use git > send-email, not just copy paste), you may send it as an attachment. Sorry about that, still struggling a little with git. I did not know how to insert the signed off when using send-email. I hope the new patch (in the separate mail) is fine. Best regards, Michael signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] removing comma at final enumeration items to fix pedantic warnings
Am 20.10.2016 um 10:00 schrieb Clément Bœsch:
> On Thu, Oct 20, 2016 at 09:55:17AM +0200, Nicolas George wrote:
>> Le nonidi 29 vendémiaire, an CCXXV, Clement Boesch a écrit :
>>> it's really a trivial and harmless patch.
>>
>> Which is not enough to accept it.
>>
>
> but the patch is perfectly fine and semantically more correct (it
> explicits that it's wrong to add entry afterward).
>
>> I am sure there are better uses of contributors' time than that.
>>
>
> people are free to do whatever they feel like; you're just wasting yours
> by bikeshedding about this stuff you could just ignore.
>
So here is the patch again, this time without the line breaks, hopefully.
Best regards,
Michael
--- Begin Message ---
---
libavutil/log.h| 2 +-
libavutil/pixfmt.h | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libavutil/log.h b/libavutil/log.h
index 0acc1b9..f0a5738 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -44,7 +44,7 @@ typedef enum {
AV_CLASS_CATEGORY_DEVICE_AUDIO_INPUT,
AV_CLASS_CATEGORY_DEVICE_OUTPUT,
AV_CLASS_CATEGORY_DEVICE_INPUT,
-AV_CLASS_CATEGORY_NB, ///< not part of ABI/API
+AV_CLASS_CATEGORY_NB ///< not part of ABI/API
}AVClassCategory;
#define AV_IS_INPUT_DEVICE(category) \
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index b15c0ef..7a3f68b 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -306,7 +306,7 @@ enum AVPixelFormat {
AV_PIX_FMT_MEDIACODEC, ///< hardware decoding through MediaCodec
-AV_PIX_FMT_NB,///< number of pixel formats, DO NOT USE THIS if you
want to link with shared libav* because the number of formats might differ
between versions
+AV_PIX_FMT_NB ///< number of pixel formats, DO NOT USE THIS if you
want to link with shared libav* because the number of formats might differ
between versions
};
#if AV_HAVE_BIGENDIAN
@@ -401,7 +401,7 @@ enum AVColorPrimaries {
AVCOL_PRI_SMPTEST428_1 = 10, ///< SMPTE ST 428-1 (CIE 1931 XYZ)
AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011)
AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 D65 (2010)
-AVCOL_PRI_NB, ///< Not part of ABI
+AVCOL_PRI_NB///< Not part of ABI
};
/**
@@ -427,7 +427,7 @@ enum AVColorTransferCharacteristic {
AVCOL_TRC_SMPTEST2084 = 16, ///< SMPTE ST 2084 for 10-, 12-, 14- and
16-bit systems
AVCOL_TRC_SMPTEST428_1 = 17, ///< SMPTE ST 428-1
AVCOL_TRC_ARIB_STD_B67 = 18, ///< ARIB STD-B67, known as "Hybrid log-gamma"
-AVCOL_TRC_NB,///< Not part of ABI
+AVCOL_TRC_NB ///< Not part of ABI
};
/**
@@ -446,7 +446,7 @@ enum AVColorSpace {
AVCOL_SPC_BT2020_NCL = 9, ///< ITU-R BT2020 non-constant luminance system
AVCOL_SPC_BT2020_CL = 10, ///< ITU-R BT2020 constant luminance system
AVCOL_SPC_SMPTE2085 = 11, ///< SMPTE 2085, Y'D'zD'x
-AVCOL_SPC_NB, ///< Not part of ABI
+AVCOL_SPC_NB///< Not part of ABI
};
#define AVCOL_SPC_YCGCO AVCOL_SPC_YCOCG
@@ -458,7 +458,7 @@ enum AVColorRange {
AVCOL_RANGE_UNSPECIFIED = 0,
AVCOL_RANGE_MPEG= 1, ///< the normal 219*2^(n-8) "MPEG" YUV ranges
AVCOL_RANGE_JPEG= 2, ///< the normal 2^n-1 "JPEG" YUV ranges
-AVCOL_RANGE_NB, ///< Not part of ABI
+AVCOL_RANGE_NB ///< Not part of ABI
};
/**
@@ -484,7 +484,7 @@ enum AVChromaLocation {
AVCHROMA_LOC_TOP = 4,
AVCHROMA_LOC_BOTTOMLEFT = 5,
AVCHROMA_LOC_BOTTOM = 6,
-AVCHROMA_LOC_NB, ///< Not part of ABI
+AVCHROMA_LOC_NB ///< Not part of ABI
};
#endif /* AVUTIL_PIXFMT_H */
--
2.6.6
--- End Message ---
signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] removing comma at final enumeration items to fix pedantic warnings
Hi all, Am 21.10.2016 um 10:32 schrieb Michael Behrisch: > Am 20.10.2016 um 10:00 schrieb Clément Bœsch: >> On Thu, Oct 20, 2016 at 09:55:17AM +0200, Nicolas George wrote: >>> Le nonidi 29 vendémiaire, an CCXXV, Clement Boesch a écrit : >>>> it's really a trivial and harmless patch. >>> >>> Which is not enough to accept it. >>> >> >> but the patch is perfectly fine and semantically more correct (it >> explicits that it's wrong to add entry afterward). >> >>> I am sure there are better uses of contributors' time than that. >>> >> >> people are free to do whatever they feel like; you're just wasting >> yours by bikeshedding about this stuff you could just ignore. >> > > So here is the patch again, this time without the line breaks, > hopefully. does the lack of further reaction mean that the patch was rejected or does the discussion continue elsewhere? Best regards, Michael signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
