On Wed, Dec 23, 2015 at 11:25:44AM +0100, Mats Peterson wrote: [...] > From: Mats Peterson <matsp...@yahoo.com>
So are you the sole author of this? > Date: Wed, 23 Dec 2015 11:19:06 +0100 > Subject: [PATCH] libavformat: palettized QuickTime video in Matroska, round 4 > Use "[PATCH v4]" instead of the suffix as you did, otherwise it will end up in the final commit message. Also, correct prefix would be "lavf: ..." > --- > libavformat/Makefile | 1 + > libavformat/matroskadec.c | 30 ++++++++++- > libavformat/mov.c | 92 ++++++++------------------------- > libavformat/qtpalette.c | 124 > +++++++++++++++++++++++++++++++++++++++++++++ > libavformat/qtpalette.h | 4 ++ > 5 files changed, 178 insertions(+), 73 deletions(-) > create mode 100644 libavformat/qtpalette.c > > diff --git a/libavformat/Makefile b/libavformat/Makefile > index 110e9e3..e03c73e 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -18,6 +18,7 @@ OBJS = allformats.o \ > mux.o \ > options.o \ > os_support.o \ > + qtpalette.o \ > riff.o \ > sdp.o \ > url.o \ > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index c574749..28bc44f 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -64,6 +64,8 @@ > #include <zlib.h> > #endif > > +#include "qtpalette.h" > + > typedef enum { > EBML_NONE, > EBML_UINT, > @@ -312,6 +314,9 @@ typedef struct MatroskaDemuxContext { > > /* WebM DASH Manifest live flag/ */ > int is_live; > + > + uint32_t palette[256]; nit: AVPALETTE_COUNT [...] > + if (ff_get_qtpalette(codec_id, track->codec_priv.data + 16, > + NULL, matroska->palette)) { nit: vertical align, more below > + bit_depth &= 0x1F; > + /* Behave like V_MS/VFW/FOURCC; copy the palette to > + * extradata */ > + if (! (extradata = av_malloc(AVPALETTE_SIZE))) > + return AVERROR(ENOMEM); Use ff_alloc_extradata() or pad your extradata with AV_INPUT_BUFFER_PADDING_SIZE also, no space after '!', more below [...] > diff --git a/libavformat/qtpalette.c b/libavformat/qtpalette.c > new file mode 100644 > index 0000000..90268e1 > --- /dev/null > +++ b/libavformat/qtpalette.c > @@ -0,0 +1,124 @@ > +/* > + * QuickTime palette handling > + * Copyright (c) 2001 Fabrice Bellard > + * Copyright (c) 2009 Baptiste Coudurier <baptiste dot coudurier at gmail > dot com> > + * Copyright (c) 2015 Mats Peterson > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +#include <stdio.h> > +#include <inttypes.h> Don't you just need stdint.h? > + > +#include "libavcodec/avcodec.h" > +#include "libavformat/avio.h" This file is already in libavformat. > +#include "libavutil/intreadwrite.h" > +#include "qtpalette.h" > + > +int ff_get_qtpalette(int codec_id, uint8_t *stsd, AVIOContext *pb, > + uint32_t *palette) stsd data isn't touched, so you can mark it const [...] > + if ((color_start <= 255) && (color_end <= 255)) { > + uint8_t *p = stsd + 78; > + for (i = color_start; i <= color_end; i++) { > + /* each A, R, G, or B component is 16 bits; > + * only use the top 8 bits */ > + if (pb) { > + a = avio_r8(pb); > + avio_r8(pb); > + r = avio_r8(pb); > + avio_r8(pb); > + g = avio_r8(pb); > + avio_r8(pb); > + b = avio_r8(pb); > + avio_r8(pb); > + } else { > + a = *p++; p++; > + r = *p++; p++; > + g = *p++; p++; > + b = *p++; p++; I see no read boundary checks, and it frighten me. > + } > + palette[i] = (a << 24 ) | (r << 16) | (g << 8) | (b); a << 24 is undefined if a msb is set. Try git grep '#define RGBA('. > + } > + } > + } > + > + return 1; > + } > + > + return 0; Usually, you have 0 for success, and < 0 (AVERROR*) for error. > +} > diff --git a/libavformat/qtpalette.h b/libavformat/qtpalette.h > index 7d6802f..8f875ae 100644 > --- a/libavformat/qtpalette.h > +++ b/libavformat/qtpalette.h > @@ -24,6 +24,7 @@ > #define AVFORMAT_QTPALETTE_H > > #include <inttypes.h> > +#include "libavformat/avio.h" This file is already in libavformat. [...] -- Clément B.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel