On 12/23/16, Andreas Cadhalpun <andreas.cadhal...@googlemail.com> wrote:
> On 22.12.2016 22:52, Paul B Mahol wrote:
>> ffmpeg | branch: master | Paul B Mahol <one...@gmail.com> | Fri Dec 2
>> 20:30:50 2016 +0100| [73651090ca1183f37753ee30a7e206ca4fb9f4f0] |
>> committer: Paul B Mahol
>>
>> avcodec: add Apple Pixlet decoder
>>
>> Signed-off-by: Paul B Mahol <one...@gmail.com>
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=73651090ca1183f37753ee30a7e206ca4fb9f4f0
>> ---
>>
>> Changelog | 1 +
>> doc/general.texi | 1 +
>> libavcodec/Makefile | 1 +
>> libavcodec/allcodecs.c | 1 +
>> libavcodec/avcodec.h | 1 +
>> libavcodec/codec_desc.c | 7 +
>> libavcodec/pixlet.c | 672
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> libavcodec/version.h | 2 +-
>> libavformat/isom.c | 2 +
>> 9 files changed, 687 insertions(+), 1 deletion(-)
> [...]
>> +static int read_high_coeffs(AVCodecContext *avctx, uint8_t *src, int16_t
>> *dst, int size,
>> + int c, int a, int d,
>> + int width, ptrdiff_t stride)
>> +{
>> + PixletContext *ctx = avctx->priv_data;
>> + GetBitContext *b = &ctx->gbit;
>> + unsigned cnt1, shbits, rlen, nbits, length, i = 0, j = 0, k;
>> + int ret, escape, pfx, value, yflag, xflag, flag = 0;
>> + int64_t state = 3, tmp;
>> +
>> + if ((ret = init_get_bits8(b, src,
>> bytestream2_get_bytes_left(&ctx->gb))) < 0)
>> + return ret;
>> +
>> + if ((a >= 0) + (a ^ (a >> 31)) - (a >> 31) != 1) {
>> + nbits = 33 - ff_clz((a >= 0) + (a ^ (a >> 31)) - (a >> 31) - 1);
>> + if (nbits > 16)
>> + return AVERROR_INVALIDDATA;
>> + } else {
>> + nbits = 1;
>> + }
>> +
>> + length = 25 - nbits;
>> +
>> + while (i < size) {
>> + if (state >> 8 != -3) {
>> + value = ff_clz((state >> 8) + 3) ^ 0x1F;
>> + } else {
>> + value = -1;
>> + }
>> +
>> + cnt1 = get_unary(b, 0, length);
>> +
>> + if (cnt1 >= length) {
>> + cnt1 = get_bits(b, nbits);
>> + } else {
>> + pfx = 14 + ((((uint64_t)(value - 14)) >> 32) & (value - 14));
>> + cnt1 *= (1 << pfx) - 1;
>> + shbits = show_bits(b, pfx);
>> + if (shbits <= 1) {
>> + skip_bits(b, pfx - 1);
>> + } else {
>> + skip_bits(b, pfx);
>> + cnt1 += shbits - 1;
>> + }
>> + }
>> +
>> + xflag = flag + cnt1;
>> + yflag = xflag;
>> +
>> + if (flag + cnt1 == 0) {
>> + value = 0;
>> + } else {
>> + xflag &= 1u;
>> + tmp = c * ((yflag + 1) >> 1) + (c >> 1);
>
> This can overflow.
And?
>
>> + value = xflag + (tmp ^ -xflag);
>> + }
>> +
>> + i++;
>> + dst[j++] = value;
>> + if (j == width) {
>> + j = 0;
>> + dst += stride;
>> + }
>> + state += d * yflag - (d * state >> 8);
>
> This can overflow, too.
And?
>
>> +
>> + flag = 0;
>> +
>> + if (state * 4 > 0xFF || i >= size)
>> + continue;
>> +
>> + pfx = ((state + 8) >> 5) + (state ? ff_clz(state): 32) - 24;
>> + escape = av_mod_uintp2(16383, pfx);
>
> Here pfx can be negative, but av_mod_uintp2 takes an unsigned argument, so
> it's
> interpreted as very large exponent, causing undefined shifts.
I think I will make it always >= 0
>
>> + cnt1 = get_unary(b, 0, 8);
>> + if (cnt1 < 8) {
>> + value = show_bits(b, pfx);
>
> And a negative pfx triggers an assertion in show_bits.
>
> [...]
>> +static void postprocess_chroma(AVFrame *frame, int w, int h, int depth)
>> +{
>> + uint16_t *dstu = (uint16_t *)frame->data[1];
>> + uint16_t *dstv = (uint16_t *)frame->data[2];
>> + int16_t *srcu = (int16_t *)frame->data[1];
>> + int16_t *srcv = (int16_t *)frame->data[2];
>> + ptrdiff_t strideu = frame->linesize[1] / 2;
>> + ptrdiff_t stridev = frame->linesize[2] / 2;
>> + const int add = 1 << (depth - 1);
>> + const int shift = 16 - depth;
>> + int i, j;
>> +
>> + for (j = 0; j < h; j++) {
>> + for (i = 0; i < w; i++) {
>> + dstu[i] = (add + srcu[i]) << shift;
>> + dstv[i] = (add + srcv[i]) << shift;
>
> These result in undefined shifts, since the value to be shifted can be
> negative.
OK.
>
>> + }
>> + dstu += strideu;
>> + dstv += stridev;
>> + srcu += strideu;
>> + srcv += stridev;
>> + }
>> +}
>> +
>> +static int decode_plane(AVCodecContext *avctx, int plane, AVPacket
>> *avpkt, AVFrame *frame)
>> +{
>> + PixletContext *ctx = avctx->priv_data;
>> + ptrdiff_t stride = frame->linesize[plane] / 2;
>> + unsigned shift = plane > 0;
>> + int16_t *dst;
>> + int i, ret;
>> +
>> + for (i = ctx->levels - 1; i >= 0; i--) {
>> + ctx->scaling[plane][H][i] = 1000000. /
>> sign_extend(bytestream2_get_be32(&ctx->gb), 32);
>> + ctx->scaling[plane][V][i] = 1000000. /
>> sign_extend(bytestream2_get_be32(&ctx->gb), 32);
>
> Here the denominators can be zero.
And?
>
>> +static int pixlet_decode_frame(AVCodecContext *avctx, void *data,
>> + int *got_frame, AVPacket *avpkt)
>> +{
>> + PixletContext *ctx = avctx->priv_data;
>> + int i, w, h, width, height, ret, version;
>> + AVFrame *p = data;
>> + ThreadFrame frame = { .f = data };
>> + uint32_t pktsize;
>> +
>> + bytestream2_init(&ctx->gb, avpkt->data, avpkt->size);
>> +
>> + pktsize = bytestream2_get_be32(&ctx->gb);
>> + if (pktsize <= 44 || pktsize - 4 >
>> bytestream2_get_bytes_left(&ctx->gb)) {
>> + av_log(avctx, AV_LOG_ERROR, "Invalid packet size %u.\n",
>> pktsize);
>> + return AVERROR_INVALIDDATA;
>> + }
>> +
>> + version = bytestream2_get_le32(&ctx->gb);
>> + if (version != 1)
>> + avpriv_request_sample(avctx, "Version %d", version);
>> +
>> + bytestream2_skip(&ctx->gb, 4);
>> + if (bytestream2_get_be32(&ctx->gb) != 1)
>> + return AVERROR_INVALIDDATA;
>> + bytestream2_skip(&ctx->gb, 4);
>> +
>> + width = bytestream2_get_be32(&ctx->gb);
>> + height = bytestream2_get_be32(&ctx->gb);
>> +
>> + w = FFALIGN(width, 1 << (NB_LEVELS + 1));
>> + h = FFALIGN(height, 1 << (NB_LEVELS + 1));
>> +
>> + ctx->levels = bytestream2_get_be32(&ctx->gb);
>> + if (ctx->levels != NB_LEVELS)
>> + return AVERROR_INVALIDDATA;
>> + ctx->depth = bytestream2_get_be32(&ctx->gb);
>> + if (ctx->depth < 8 || ctx->depth > 15) {
>> + avpriv_request_sample(avctx, "Depth %d", ctx->depth);
>> + return AVERROR_INVALIDDATA;
>> + }
>> +
>> + ret = ff_set_dimensions(avctx, w, h);
>> + if (ret < 0)
>> + return ret;
>> + avctx->width = width;
>> + avctx->height = height;
>> +
>> + if (ctx->w != w || ctx->h != h) {
>> + free_buffers(avctx);
>> + ctx->w = w;
>> + ctx->h = h;
>> +
>> + ret = init_decoder(avctx);
>> + if (ret < 0) {
>> + free_buffers(avctx);
>> + ctx->w = 0;
>> + ctx->h = 0;
>> + return ret;
>> + }
>> + }
>> +
>> + bytestream2_skip(&ctx->gb, 8);
>> +
>> + p->pict_type = AV_PICTURE_TYPE_I;
>> + p->key_frame = 1;
>> + p->color_range = AVCOL_RANGE_JPEG;
>> +
>> + ret = ff_thread_get_buffer(avctx, &frame, 0);
>> + if (ret < 0)
>> + return ret;
>> +
>> + for (i = 0; i < 3; i++) {
>> + ret = decode_plane(avctx, i, avpkt, frame.f);
>> + if (ret < 0)
>> + return ret;
>> + if (avctx->flags & AV_CODEC_FLAG_GRAY)
>> + break;
>> + }
>> +
>> + postprocess_luma(frame.f, ctx->w, ctx->h, ctx->depth);
>> + postprocess_chroma(frame.f, ctx->w >> 1, ctx->h >> 1, ctx->depth);
>> +
>> + *got_frame = 1;
>> +
>> + return pktsize;
>
> Since this is a video decoder, this should always return the full
> avpkt->size.
Wrong. It returns what it consumes.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
