2018-12-06 11:56 GMT+01:00, Paul B Mahol <one...@gmail.com>: > Signed-off-by: Paul B Mahol <one...@gmail.com> > --- > libavcodec/dpx.c | 36 ++++++++++++++++++++++++++++++++---- > 1 file changed, 32 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c > index 0297287938..84894abda5 100644 > --- a/libavcodec/dpx.c > +++ b/libavcodec/dpx.c > @@ -50,6 +50,24 @@ static unsigned int read32(const uint8_t **ptr, int > is_big) > return temp; > } > > +static uint16_t read10in32_gray(const uint8_t **ptr, uint32_t *lbuf, > + int *n_datum, int is_big, int shift) > +{ > + uint16_t temp; > + > + if (*n_datum) > + (*n_datum)--; > + else { > + *lbuf = read32(ptr, is_big); > + *n_datum = 2; > + } > +
> + temp = *lbuf >> shift & 0x3FF; > + *lbuf = *lbuf >> 10; > + > + return temp; Isn't my code simpler? One variable less, same number of operations iirc. > +} > + > static uint16_t read10in32(const uint8_t **ptr, uint32_t * lbuf, > int * n_datum, int is_big, int shift) > { > @@ -107,6 +125,7 @@ static int decode_frame(AVCodecContext *avctx, > AVFrame *const p = data; > uint8_t *ptr[AV_NUM_DATA_POINTERS]; > uint32_t header_version, version = 0; > + const uint8_t *creator; > > unsigned int offset; > int magic_num, endian; > @@ -151,6 +170,9 @@ static int decode_frame(AVCodecContext *avctx, > av_log(avctx, AV_LOG_WARNING, "Unknown header format version > %s.\n", > av_fourcc2str(header_version)); > > + buf = avpkt->data + 160; > + creator = buf; I wonder why you split the version but not the creator and why the metadata isn't set. > + > // Check encryption > buf = avpkt->data + 660; > ret = read32(&buf, endian); > @@ -320,6 +342,7 @@ static int decode_frame(AVCodecContext *avctx, > case 51121: > avctx->pix_fmt = AV_PIX_FMT_GBRAP12; > break; > + case 6100: Looks unrelated. > case 6101: > avctx->pix_fmt = AV_PIX_FMT_GRAY10; > break; > @@ -373,13 +396,17 @@ static int decode_frame(AVCodecContext *avctx, > (uint16_t*)ptr[1], > (uint16_t*)ptr[2], > (uint16_t*)ptr[3]}; > - int shift = packing == 1 ? 22 : 20; > + int shift = elements > 1 ? packing == 1 ? 22 : 20 : packing == > 1 ? 2 : 0; Wouldn't it be simpler to add "20" in read10in32()? > for (y = 0; y < avctx->width; y++) { > if (elements >= 3) > *dst[2]++ = read10in32(&buf, &rgbBuffer, > &n_datum, endian, shift); > - *dst[0]++ = read10in32(&buf, &rgbBuffer, > - &n_datum, endian, shift); > + if (elements == 1) > + *dst[0]++ = read10in32_gray(&buf, &rgbBuffer, > + &n_datum, endian, shift); > + else > + *dst[0]++ = read10in32(&buf, &rgbBuffer, > + &n_datum, endian, shift); > if (elements >= 2) > *dst[1]++ = read10in32(&buf, &rgbBuffer, > &n_datum, endian, shift); > @@ -388,7 +415,8 @@ static int decode_frame(AVCodecContext *avctx, > read10in32(&buf, &rgbBuffer, > &n_datum, endian, shift); > } > - n_datum = 0; > + if (version != 2 || !memcmp(creator, "GraphicsMagick", 14)) This looks wrong to me: Where in the specification does it say that this depends on the version? > + n_datum = 0; > for (i = 0; i < elements; i++) > ptr[i] += p->linesize[i]; > } Thank you, Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel