On Thu, Apr 7, 2016 at 8:44 PM, Vittorio Giovara <vittorio.giov...@gmail.com
> wrote:

> On Thu, Apr 7, 2016 at 1:09 PM, Tim W. <tdskywal...@gmail.com> wrote:
> > On Wed, Apr 6, 2016 at 4:40 PM, Vittorio Giovara <
> vittorio.giov...@gmail.com
> >> wrote:
> >
> >> On Tue, Apr 5, 2016 at 11:04 PM, Tim W. <tdskywal...@gmail.com> wrote:
> >> > On Tue, Apr 5, 2016 at 10:19 PM, Vittorio Giovara <
> >> > vittorio.giov...@gmail.com> wrote:
> >> >
> >> >> Use it in av_dump_format() instead of a huge switch case. Needed in
> >> >> the next commit as well.
> >> >> ---
> >> >>  libavformat/dump.c   | 33 +++++----------------------------
> >> >>  libavutil/stereo3d.c | 11 +++++++++++
> >> >>  libavutil/stereo3d.h | 14 ++++++++++++++
> >> >>  libavutil/version.h  |  2 +-
> >> >>  4 files changed, 31 insertions(+), 29 deletions(-)
> >> >>
> >> >> diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
> >> >> index 2dcfddf..c8346aa 100644
> >> >> --- a/libavutil/stereo3d.c
> >> >> +++ b/libavutil/stereo3d.c
> >> >> @@ -41,3 +41,14 @@ AVStereo3D *av_stereo3d_create_side_data(AVFrame
> >> *frame)
> >> >>
> >> >>      return (AVStereo3D *)side_data->data;
> >> >>  }
> >> >> +
> >> >> +static const char *stereo3d_type_names[AV_STEREO3D_NB] = {
> >> >> +    "2D", "side by side", "top and bottom", "frame alternate",
> >> >> +    "checkerboard", "interleaved lines", "interleaved columns",
> >> >> +    "side by side (quincunx subsampling)",
> >> >> +};
> >> >
> >> >
> >> > Yuck… yuck yuck yuck.
> >> >
> >> > May I suggest libavutil/channel_layout.c as a much prettier (and IMO
> >> > better) example?
> >>
> >> You may if you explain why it's better.
> >>
> >
> > In retrospect, I must have been in a bad mood because of tiredness.
> >
> > Here goes: IMO, one line per name is much more readable. The names
> > themselves are fine, AFAICT.
> >
> >
> >> Also note that this is the same style of lavu/pix_desc names too.
> >>
> >
> > Actually, av_pix_fmt_descriptors seems to have more than one line per
> > pix_fmt ;)
>
> I wrote pix_desc not pix_fmt, check out the current style:
> https://github.com/libav/libav/blob/master/libavutil/pixdesc.c#L1598-L1624


To be fair it's the same file, and the pix_fmt descriptors take up 1460
lines, as opposed to those 6 you mentioned and which I missed. Also, it
wasn't immediately obvious that you meant color characteristics by
"pix_desc", I just looked at the file with that name.

I prefer one line per name (more readable, and since we're talking about
enums, you can associate a value with a name by just counting lines, as
opposed to counting items on lines with a variable name count).

Still, I'm not quite as disgusted as I seemed to be the other day so I
guess that part's up to you :)

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

Reply via email to