On Fri, Apr 03, 2020 at 01:19:26PM +0200, Mauro Carvalho Chehab wrote:
> Em Fri, 3 Apr 2020 13:47:02 +0300
> Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu:
>
> > > > +static noinline_for_stack
> > > > +char *fourcc_string(char *buf, char *end, const u32 *fourcc,
> > > > + struct printf_spec spec, const char *fmt)
> > > > +{
> > > > +#define FOURCC_STRING_BE "-BE"
> > > > + char s[sizeof(*fourcc) + sizeof(FOURCC_STRING_BE)] = { 0 };
> > > > +
> > > > + if (check_pointer(&buf, end, fourcc, spec))
> > > > + return buf;
> > > > +
> > > > + if (fmt[1] != 'c' || fmt[2] != 'c')
> > > > + return error_string(buf, end, "(%p4?)", spec);
> > > > +
> > > > + put_unaligned_le32(*fourcc & ~BIT(31), s);
> > > > +
> > > > + if (*fourcc & BIT(31))
> > > > + strscpy(s + sizeof(*fourcc), FOURCC_STRING_BE,
> > > > + sizeof(FOURCC_STRING_BE));
> > > > +
> > > > + return string(buf, end, s, spec);
> > >
> > > Taking V4L2_PIX_FMT_Y16_BE as an example, this will print 'Y16 -BE'
> > > (without quotes). There are other 4CCs that contain spaces and would
> > > suffer from a similar issue. Even in little-endian format, it would
> > > result in additional spaces in the output string. Is this what we want ?
> > > Should the caller always enclose the 4CC in quotes or brackets for
> > > clarity ? Or should still be done here ?
> >
> > Good question. Space is indeed a valid character in a 4cc code.
> >
> > If I omit one or more spaces, it will no longer be a 4cc, but a 3cc or even
> > a 2cc. Jokes aside, there are probably fair arguments both ways.
> >
> > I presume there's no 4cc code where the location of a space would make a
> > difference but all of the spaces are trailing spaces.
>
> Yes. I guess it doesn't make any sense to allow a 4cc code with an
> space before or in the middle.
>
> Btw, on a quick search at the Internet for non-Linux definitions,
> a Fourcc code "Y8 " is actually shown at the lists as just "Y8",
> e. g. removing the leading spaces:
>
> https://www.fourcc.org/codecs.php
> http://abcavi.kibi.ru/fourcc.php
> https://softron.zendesk.com/hc/en-us/articles/207695697-List-of-FourCC-codes-for-video-codecs
> https://www.free-codecs.com/guides/guides.php?f=fourcc
>
> One interesting detail there is that some tables show some codes
> like "BGR(15)". While I'm not sure how this is encoded, I suspect
> that the fourcc is actually "BGR\x15".
>
> We don't do that on V4L, nor we have plans to do so. Not sure if
> DRM would accept something like that. Of so, then the logic should
> have some special handler if the code is below 32.
It is easy to achieve I think, with help of string_escape*() functions.
> > It's also worth noting that the formats printed are mostly for debugging
> > purpose and thus even getting a hypothetical case wrong is not a grave
> > issue. This would also support just printing them as-is though.
> >
> > I'm leaning slightly towards omitting any spaces if the code has them.
>
> I would just remove trailing spaces, and then use a loop from the end
> to remove trailing spaces (and optionally handle codes ending with a
> value below 32, if are there any such case with DRM fourcc codes).
>
> On the other hand, I don't mind if you prefer to use just one for()
> loop and just trip any spaces inside it.
>
> > This is something that couldn't be done by using a macro...
>
> Well, I suspect that it might be possible to write a macro
> for doing that too, for example using preprocessor concatenation
> logic that could produce the same results. If you do something
> like that, however, I suspect that te macro would face some
> portability issues, as, as far as I know, not all C compilers
> would handle string concatenation the same way.
>
> Thanks,
> Mauro
--
With Best Regards,
Andy Shevchenko
