Re: [FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-04-03 Thread Andreas Rheinhardt
Steve Lhomme:
> On 2020-03-25 23:24, Andreas Rheinhardt wrote:
>> Steve Lhomme:
>>> From: Steve Lhomme 
>>>
>>> Always use a comma at the end, order elements by value.
>>> ---
>>>   libavformat/matroska.h | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
>>> index 9e33e51c94..e177cd027f 100644
>>> --- a/libavformat/matroska.h
>>> +++ b/libavformat/matroska.h
>>> @@ -286,13 +286,13 @@ typedef enum {
>>>   typedef enum {
>>>   MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED = 0,
>>>   MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED   = 1,
>>> -    MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2
>>> +    MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2,
>>>   } MatroskaVideoInterlaceFlag;
>>>     typedef enum {
>>>   MATROSKA_VIDEO_FIELDORDER_PROGRESSIVE  = 0,
>>> -    MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
>>>   MATROSKA_VIDEO_FIELDORDER_TT   = 1,
>>> +    MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
>>>   MATROSKA_VIDEO_FIELDORDER_BB   = 6,
>>>   MATROSKA_VIDEO_FIELDORDER_TB   = 9,
>>>   MATROSKA_VIDEO_FIELDORDER_BT   = 14,
>>>
>> Would it actually be allowed to add new values to the range of
>> FlagInterlaced (to which the MatroskaVideoInterlaceFlag enum
>> corresponds)? If no, then we should not add a comma, as this signals
>> extensibility.
>> The reordering looks good either way.
> 
> Yes, dependending on the DocType version it might be possible that some
> new values are added to enums (not just this one).
> 
> Also I'm not sure I can tell an element in the last of an "list" in XSLT
> so that would be more trouble to generate the code.

Applied.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-03-28 Thread Steve Lhomme

On 2020-03-25 23:24, Andreas Rheinhardt wrote:

Steve Lhomme:

From: Steve Lhomme 

Always use a comma at the end, order elements by value.
---
  libavformat/matroska.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 9e33e51c94..e177cd027f 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -286,13 +286,13 @@ typedef enum {
  typedef enum {
  MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED = 0,
  MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED   = 1,
-MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2
+MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2,
  } MatroskaVideoInterlaceFlag;
  
  typedef enum {

  MATROSKA_VIDEO_FIELDORDER_PROGRESSIVE  = 0,
-MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
  MATROSKA_VIDEO_FIELDORDER_TT   = 1,
+MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
  MATROSKA_VIDEO_FIELDORDER_BB   = 6,
  MATROSKA_VIDEO_FIELDORDER_TB   = 9,
  MATROSKA_VIDEO_FIELDORDER_BT   = 14,


Would it actually be allowed to add new values to the range of
FlagInterlaced (to which the MatroskaVideoInterlaceFlag enum
corresponds)? If no, then we should not add a comma, as this signals
extensibility.
The reordering looks good either way.


Yes, dependending on the DocType version it might be possible that some 
new values are added to enums (not just this one).


Also I'm not sure I can tell an element in the last of an "list" in XSLT 
so that would be more trouble to generate the code.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-03-27 Thread Anton Khirnov
Quoting Moritz Barsnick (2020-03-24 09:47:14)
> On Sun, Mar 22, 2020 at 09:59:21 +0100, Steve Lhomme wrote:
> > Always use a comma at the end, order elements by value.
> 
> IIUC, this is strictly a C99(+) "feature", so should probably be
> mentioned here:
> https://ffmpeg.org/developer.html#C-language-features
> in case "all compilers we care about" support this.
> 
> (Yes, I am aware that many other enums in said file already are
> formatted thus.)
> 
> Deeply into nitpick country, ;-)

IMO we should stop picking specific C99 features we require and just
say we assume all of it, except some forbidden features (VLA, complex
numbers). We even borderline-depend on a C11 feature (atomics).

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-03-25 Thread Andreas Rheinhardt
Steve Lhomme:
> From: Steve Lhomme 
> 
> Always use a comma at the end, order elements by value.
> ---
>  libavformat/matroska.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 9e33e51c94..e177cd027f 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -286,13 +286,13 @@ typedef enum {
>  typedef enum {
>  MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED = 0,
>  MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED   = 1,
> -MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2
> +MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2,
>  } MatroskaVideoInterlaceFlag;
>  
>  typedef enum {
>  MATROSKA_VIDEO_FIELDORDER_PROGRESSIVE  = 0,
> -MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
>  MATROSKA_VIDEO_FIELDORDER_TT   = 1,
> +MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
>  MATROSKA_VIDEO_FIELDORDER_BB   = 6,
>  MATROSKA_VIDEO_FIELDORDER_TB   = 9,
>  MATROSKA_VIDEO_FIELDORDER_BT   = 14,
> 
Would it actually be allowed to add new values to the range of
FlagInterlaced (to which the MatroskaVideoInterlaceFlag enum
corresponds)? If no, then we should not add a comma, as this signals
extensibility.
The reordering looks good either way.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-03-24 Thread Moritz Barsnick
On Sun, Mar 22, 2020 at 09:59:21 +0100, Steve Lhomme wrote:
> Always use a comma at the end, order elements by value.

IIUC, this is strictly a C99(+) "feature", so should probably be
mentioned here:
https://ffmpeg.org/developer.html#C-language-features
in case "all compilers we care about" support this.

(Yes, I am aware that many other enums in said file already are
formatted thus.)

Deeply into nitpick country, ;-)
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 02/14] avformat/matroska: clean the structure formatting

2020-03-22 Thread Steve Lhomme
From: Steve Lhomme 

Always use a comma at the end, order elements by value.
---
 libavformat/matroska.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 9e33e51c94..e177cd027f 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -286,13 +286,13 @@ typedef enum {
 typedef enum {
 MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED = 0,
 MATROSKA_VIDEO_INTERLACE_FLAG_INTERLACED   = 1,
-MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2
+MATROSKA_VIDEO_INTERLACE_FLAG_PROGRESSIVE  = 2,
 } MatroskaVideoInterlaceFlag;
 
 typedef enum {
 MATROSKA_VIDEO_FIELDORDER_PROGRESSIVE  = 0,
-MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
 MATROSKA_VIDEO_FIELDORDER_TT   = 1,
+MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
 MATROSKA_VIDEO_FIELDORDER_BB   = 6,
 MATROSKA_VIDEO_FIELDORDER_TB   = 9,
 MATROSKA_VIDEO_FIELDORDER_BT   = 14,
-- 
2.18.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".