[FFmpeg-devel] [PATCH 1/2] avcodec/vda_h264: use multichar literal portably

wm4 nfxjfg at googlemail.com
Sun Sep 20 19:39:50 CEST 2015


On Sun, 20 Sep 2015 11:18:46 -0400
Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:

> On Sun, Sep 20, 2015 at 10:41 AM, wm4 <nfxjfg at googlemail.com> wrote:
> > On Fri, 18 Sep 2015 18:16:18 -0400
> > Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
> >
> >> Multichar literals are implementation defined, and thus trigger -Wmultichar:
> >> http://fate.ffmpeg.org/log.cgi?time=20150918202532&log=compile&slot=x86_64-darwin-gcc-5.
> >> http://www.zipcon.net/~swhite/docs/computers/languages/c_multi-char_const.html
> >> gives a good summary of how to deal with them; in particular this patch
> >> results in behavior identical to that generated by GCC (which I assume is correct this case).
> >>
> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> >> ---
> >>  libavcodec/vda_h264.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/vda_h264.c b/libavcodec/vda_h264.c
> >> index 8c526c0..3279afa 100644
> >> --- a/libavcodec/vda_h264.c
> >> +++ b/libavcodec/vda_h264.c
> >> @@ -339,7 +339,9 @@ int ff_vda_default_init(AVCodecContext *avctx)
> >>      CFMutableDictionaryRef buffer_attributes;
> >>      CFMutableDictionaryRef io_surface_properties;
> >>      CFNumberRef cv_pix_fmt;
> >> -    int32_t fmt = 'avc1', pix_fmt = vda_ctx->cv_pix_fmt_type;
> >> +#define LE_CHR(a,b,c,d) ( ((a)<<24) | ((b)<<16) | ((c)<<8) | (d) )
> >> +    int32_t fmt = LE_CHR( 'a', 'v', 'c', '1');
> >> +    int32_t pix_fmt = vda_ctx->cv_pix_fmt_type;
> >>
> >>      // kCVPixelFormatType_420YpCbCr8Planar;
> >>
> >
> > Sorry, but this is probably completely pointless. Apple and code using
> > Apple APIs use this in a lot of places. Also, since this is Apple
> > specific anyway (and thus runs on only 1 compiler and 1 platform), the
> > portability argument doesn't make sense either.
> 
> please see patchv2.
> 

Nothing changed about this point, though.

I'm not terribly opposed to your patch, but I'm wondering if it doesn't
make the code harder to read (not adhering to Apple's conventions when
using their API), and could potentially break things for whatever
reason.



More information about the ffmpeg-devel mailing list