[vlc-devel] [PATCH 5/7] vlc_fourcc: add a macro to get the string for a FOURCC

[Filip Roséen]

Hi Steve,

On 2017-05-12 17:32, Steve Lhomme wrote:

> On Fri, May 12, 2017 at 4:31 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> > Le torstaina 11. toukokuuta 2017, 9.36.07 EEST Steve Lhomme a écrit :
> >> ---
> >>  include/vlc_fourcc.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/include/vlc_fourcc.h b/include/vlc_fourcc.h
> >> index b950b0622b..d6a2c5085e 100644
> >> --- a/include/vlc_fourcc.h
> >> +++ b/include/vlc_fourcc.h
> >> @@ -24,6 +24,8 @@
> >>  #ifndef VLC_FOURCC_H
> >>  #define VLC_FOURCC_H 1
> >>
> >> +#define FOURCC_STR(fc) ((const char *)&(fc))
> >> +
> >>  /* Video codec */
> >>  #define VLC_CODEC_MPGV            VLC_FOURCC('m','p','g','v')
> >>  #define VLC_CODEC_MP4V            VLC_FOURCC('m','p','4','v')
> >
> > This is hugely misleading as the result is not nul-terminated. Nack.
> 
> I have a less misleading one:
> 
> /* use this macro with a %4.4s string formating */
> #define FOURCC_44S(fc) ((fc) ? (const char *)&(fc) : "NULL")
> 
> That avoids the ugly logs wherever we use %4.4s for a chroma/codec
> that may be 0.

As already stated a few days ago, I do not see how the original macro
is helpful, nor the now proposed version. If one really wants such a
helper that you seem to be after, I would recommend doing something as
the below.

    #define VLC_FOURCC_STR(fcc) \
        ( fcc == 0 ? "N/A" : (char const*)&(char const[5]){ \
            ((char*)&fcc)[0], \
            ((char*)&fcc)[1], \
            ((char*)&fcc)[2], \
            ((char*)&fcc)[3], 0 } \
        )

Allowing for the below:

    vlc_fourcc_t foo = VLC_FOURCC( 't', 'i', 'h', 'i' );
    vlc_fourcc_t bar = 0;

    msg_Dbg( "foo: %s, bar: %s",
      VLC_FOURCC_STR( foo ),
      VLC_FOURCC_STR( bar ) ); /* foo: tihi, bar: N/A */

Notes:

 - If, for some reason, you also want to have support for
   `VLC_FOURCC_STR( VLC_FOURCC( ... ) )` you would of course have to
   modify the above to support *rvalues*.

 - The proposed macro is, of course, only usable from *C* (an
   alternative implementation needs to be written for *C++*).
 
 - If one wants it in the codebase, one should elaborate it with
   either;
    - `_Generic` to prevent it being used with the wrong source-type,
      or;
    - conditionally ill-formed *constant-expression* to fail
      compilation if `sizeof(fcc) < 4`.

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170512/a7f614e0/attachment.html>