Re: [libav-devel] [PATCH 2/2] eatqi: Remove mpegvideo dependency

2016-01-25 Thread Diego Biurrun
On Mon, Jan 25, 2016 at 12:55:05PM -0500, Vittorio Giovara wrote:
> On Fri, Jan 22, 2016 at 1:36 AM, Diego Biurrun  wrote:
> > On Thu, Jan 21, 2016 at 04:53:56PM -0500, Vittorio Giovara wrote:
> >> --- a/libavcodec/eatqi.c
> >> +++ b/libavcodec/eatqi.c
> >> @@ -137,15 +149,15 @@ static int tqi_decode_frame(AVCodecContext *avctx,
> >> -
> >> -s->last_dc[0] = s->last_dc[1] = s->last_dc[2] = 0;
> >> +t->last_dc[0] = t->last_dc[1] = t->last_dc[2] = 0;
> >
> > Please break these lines while you're at it for readability.
> 
> I'll change this line to memset(t->last_dc, 0,
> sizeof(t->last_dc)); if that's all right

No, please don't add (more) unrelated changes.  Do them separately
if you need to.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [FFmpeg-devel] [PATCHv3] avcodec: Cineform HD Decoder

2016-01-25 Thread Kieran Kunhya
On Mon, Jan 25, 2016 at 2:42 PM, Ronald S. Bultje  wrote:
> Hi,
>
> On Sun, Jan 24, 2016 at 7:34 PM, Kieran Kunhya  wrote:
>
>> +static inline void filter(int16_t *output, ptrdiff_t out_stride, int16_t
>> *low, ptrdiff_t low_stride,
>> +  int16_t *high, ptrdiff_t high_stride, int len,
>> uint8_t clip)
>
>
> Should this be a DSP function? (That is, the functions calling this.)
>
> They seem very SIMD'able.
>
> Ronald

I plan to split this all up in the future, for now I want to get it in.
Kieran
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [FFmpeg-devel] [PATCH] svq1enc: fix out of bounds reads

2016-01-25 Thread Andreas Cadhalpun
On 22.01.2016 00:57, Michael Niedermayer wrote:
> On Thu, Jan 21, 2016 at 11:04:14PM +0100, Andreas Cadhalpun wrote:
>> level can be up to 5, but there are only four codebooks.
>>
>> Fixes ubsan runtime error: index 5 out of bounds for type 'int8_t
>> [4][96]'
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavcodec/svq1enc.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
>> index 1e1745e..7ff72b4 100644
>> --- a/libavcodec/svq1enc.c
>> +++ b/libavcodec/svq1enc.c
>> @@ -104,7 +104,7 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, 
>> uint8_t *ref,
>>  best_score = 0;
>>  // FIXME: Optimize, this does not need to be done multiple times.
>>  if (intra) {
>> -codebook_sum   = svq1_intra_codebook_sum[level];
>> +codebook_sum   = level < 4 ? svq1_intra_codebook_sum[level] : NULL;
>>  codebook   = ff_svq1_intra_codebooks[level];
>>  mean_vlc   = ff_svq1_intra_mean_vlc;
>>  multistage_vlc = ff_svq1_intra_multistage_vlc[level];
>> @@ -117,7 +117,7 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, 
>> uint8_t *ref,
>>  }
>>  }
>>  } else {
>> -codebook_sum   = svq1_inter_codebook_sum[level];
>> +codebook_sum   = level < 4 ? svq1_inter_codebook_sum[level] : NULL;
>>  codebook   = ff_svq1_inter_codebooks[level];
>>  mean_vlc   = ff_svq1_inter_mean_vlc + 256;
>>  multistage_vlc = ff_svq1_inter_multistage_vlc[level];
> 
>> @@ -143,7 +143,7 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, 
>> uint8_t *ref,
>>  const int8_t *vector;
>>  
>>  for (i = 0; i < 16; i++) {
>> -int sum = codebook_sum[stage * 16 + i];
>> +int sum = codebook_sum ? codebook_sum[stage * 16 + i] : 0;
>>  int sqr, diff, score;
> 
> this is uneeded, it cannot be NULL

Indeed. That explains how FATE could pass...

> the other 2 checks should be commented to say something about the
>> = 4 case being unused

Attached patch has comments there.

Best regards,
Andreas

>From 5168bee94d1e7e09ebfcfe2bdab94430d4366cb2 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun 
Date: Thu, 21 Jan 2016 22:36:36 +0100
Subject: [PATCH] svq1enc: fix out of bounds reads

level can be 5, but there are only four codebooks.

Fixes ubsan runtime error: index 5 out of bounds for type 'int8_t
[4][96]'

Signed-off-by: Andreas Cadhalpun 
---
 libavcodec/svq1enc.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 1e1745e..d968d36 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -104,7 +104,9 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
 best_score = 0;
 // FIXME: Optimize, this does not need to be done multiple times.
 if (intra) {
-codebook_sum   = svq1_intra_codebook_sum[level];
+// level is 5 when encode_block is called from svq1_encode_plane
+// and always < 4 when called recursively from this function.
+codebook_sum   = level < 4 ? svq1_intra_codebook_sum[level] : NULL;
 codebook   = ff_svq1_intra_codebooks[level];
 mean_vlc   = ff_svq1_intra_mean_vlc;
 multistage_vlc = ff_svq1_intra_multistage_vlc[level];
@@ -117,7 +119,8 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
 }
 }
 } else {
-codebook_sum   = svq1_inter_codebook_sum[level];
+// level is 5 or < 4, see above for details.
+codebook_sum   = level < 4 ? svq1_inter_codebook_sum[level] : NULL;
 codebook   = ff_svq1_inter_codebooks[level];
 mean_vlc   = ff_svq1_inter_mean_vlc + 256;
 multistage_vlc = ff_svq1_inter_multistage_vlc[level];
-- 
2.7.0.rc3

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] lavf: remove the concat protocol

2016-01-25 Thread Andreas Cadhalpun
On 22.01.2016 00:34, Luca Barbato wrote:
> Let's try to make sure we are talking about the same problem here.
> 
> by using hls you might craft a playlist containing a concat of a
> playlist w/out a final new line.
> 
> So you would send the initial part of the file together with the url.
> 
> This is an information leak.

Yes.

> It is moderate since the url has a maximum size that is sort of small
> and libav's concat does not have a mean to send a file in chunks.
> 
> So you cannot send /etc/shadow as whole anyway.
> 
> The ways to fix the specific problem problem:
> 
> - provide a blacklist/whitelist option in hls (from me, first
> solution)-> Anton disliked it since it is too specific,

However, the hls demuxer should somehow validate the URLs it's reading.
Your suggestion would be one way to do that, others are possible as well.

> courmish pointed out you can feed such line over any playlist some
> application using avformat supports.

Likewise such applications should check URLs before passing them
to libavformat. Though, in practice, there will always be some that don't.

> - have a pluggable avio-wide infrastructure to policy protocols and
> paths (from Anton) -> It isn't simple to complete it.

It's certainly a nice feature for applications to be able to implement
their IO policy, but this still leaves the question of what should happen
by default open.
So this patch set is rather orthogonal to finding a solution for
preventing these kind of information leaks.

> - drop concat (agreed by me, Anton and RĂ©mi) -> It is radical, but the
> feature itself is quite fringe and I would rather drop it.

That's more like admitting defeat. It's also no real solution as other
protocols possibly added in the future could be used for similar exploits.

> avconv itself may stay compatible with scripts by implementing it in
> program itself

That sounds like a giant hack.

> or providing a better interface for it while at it.

Isn't the concat demuxer such a better interface?

On 22.01.2016 13:37, Anton Khirnov wrote:
> Just so it's clear what we're talking about, what is "properly" for you?

That would be a more or less general mechanism, which would have prevented
the information leak in the hls demuxer by default. It should also prevent
any such possible problems in other demuxers.
This could be done by implementing a protocol_whitelist with sensible
defaults.

> And what do you see as "the underlying problem"?

I think that is libavformat mixing local and remote data. If it wouldn't
to do that by default, such information leaks shouldn't be possible.

Best regards,
Andreas

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCHv3] avcodec: Cineform HD Decoder

2016-01-25 Thread Kieran Kunhya
On Mon, Jan 25, 2016 at 4:51 PM, Vittorio Giovara
 wrote:
> On Sun, Jan 24, 2016 at 7:34 PM, Kieran Kunhya  wrote:
>> Decodes YUV 4:2:2 10-bit and RGB 12-bit files.
>> Older files with more subbands, skips, Bayer, alpha not supported.
>> Alpha requires addition of GBRAP12 pixel format.
>
> Actually you could set AV_PIX_FMT_GBRAP16 and tweak the
> bits_per_raw_sample, not sure about the repercussion on the users
> though.
>
>> ---
>> +static av_cold int cfhd_decode_init(AVCodecContext *avctx)
>
> is this function _decode or _init? or is it decoder_init? imho names
> would be simpler with just _ scheme.

Dunno was told to do that.

>> +{
>> +CFHDContext *s = avctx->priv_data;
>> +
>> +avctx->pix_fmt = AV_PIX_FMT_YUV422P10;
>
> if the decoder supports multiple pixel formats it's better not to
> initialize the pixel format here, and wait until decode(). Otherwise
> it's going to cause a "parameters changed" warning and reinit any
> previous filter chain.

There are some samples which don't have a pixel format flagged and are
implicitly AV_PIX_FMT_YUV422P10.

>> +avctx->bits_per_raw_sample = 10;
>> +s->avctx   = avctx;
>> +avctx->width   = 0;
>> +avctx->height  = 0;
>
> isn't the context mallocz anyway?

No it's allocated from the demuxer

>> +return ff_cfhd_init_vlcs(s);
>> +}
>> +
>
>> +static inline void filter(int16_t *output, ptrdiff_t out_stride, int16_t 
>> *low, ptrdiff_t low_stride,
>> +  int16_t *high, ptrdiff_t high_stride, int len, 
>> uint8_t clip)
>> +{
>> +int16_t tmp;
>> +
>> +int i;
>> +for (i = 0; i < len; i++) {
>> +if (i == 0) {
>> +tmp = (11*low[0*low_stride] - 4*low[1*low_stride] + 
>> low[2*low_stride] + 4) >> 3;
>> +output[(2*i+0)*out_stride] = (tmp + high[0*high_stride]) >> 1;
>> +if (clip)
>> +output[(2*i+0)*out_stride] = 
>> av_clip_uintp2(output[(2*i+0)*out_stride], clip);
>> +
>> +tmp = ( 5*low[0*low_stride] + 4*low[1*low_stride] - 
>> low[2*low_stride] + 4) >> 3;
>> +output[(2*i+1)*out_stride] = (tmp - high[0*high_stride]) >> 1;
>> +if (clip)
>> +output[(2*i+1)*out_stride] = 
>> av_clip_uintp2(output[(2*i+1)*out_stride], clip);
>> +}
>> +else if (i == len-1){
>
> nit, spacing and new line are still off
>
>> +tmp = ( 5*low[i*low_stride] + 4*low[(i-1)*low_stride] - 
>> low[(i-2)*low_stride] + 4) >> 3;
>> +output[(2*i+0)*out_stride] = (tmp + high[i*high_stride]) >> 1;
>> +if (clip)
>> +output[(2*i+0)*out_stride] = 
>> av_clip_uintp2(output[(2*i+0)*out_stride], clip);
>> +
>> +tmp = (11*low[i*low_stride] - 4*low[(i-1)*low_stride] + 
>> low[(i-2)*low_stride] + 4) >> 3;
>> +output[(2*i+1)*out_stride] = (tmp - high[i*high_stride]) >> 1;
>> +if (clip)
>> +output[(2*i+1)*out_stride] = 
>> av_clip_uintp2(output[(2*i+1)*out_stride], clip);
>> +}
>> +else {
>> +tmp = (low[(i-1)*low_stride] - low[(i+1)*low_stride] + 4) >> 3;
>> +output[(2*i+0)*out_stride] = (tmp + low[i*low_stride] + 
>> high[i*high_stride]) >> 1;
>> +if (clip)
>> +output[(2*i+0)*out_stride] = 
>> av_clip_uintp2(output[(2*i+0)*out_stride], clip);
>> +
>> +tmp = (low[(i+1)*low_stride] - low[(i-1)*low_stride] + 4) >> 3;
>> +output[(2*i+1)*out_stride] = (tmp + low[i*low_stride] - 
>> high[i*high_stride]) >> 1;
>> +if (clip)
>> +output[(2*i+1)*out_stride] = 
>> av_clip_uintp2(output[(2*i+1)*out_stride], clip);
>> +}
>> +}
>> +}
>
> +1 on the dsp suggestion
>
>> +}
>> +
>> +static int alloc_buffers(AVCodecContext *avctx)
>> +{
>> +CFHDContext *s = avctx->priv_data;
>> +int i, j, k;
>> +
>> +avctx->width = s->coded_width;
>> +avctx->height = s->coded_height;
>
> I think ff_set_dimensions is the function you're looking for (make
> sure to check its return value)
>
>> +avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_x_shift, 
>> &s->chroma_y_shift);
>
> this again? :)
>
>> +for (i = 0; i < 3; i++) {
>> +int width = i ? avctx->width >> s->chroma_x_shift : avctx->width;
>> +int height = i ? avctx->height >> s->chroma_y_shift : avctx->height;
>
> could these be candidates for AV_CEIL_RSHIFT?
>
>> +int stride = FFALIGN(width / 8, 8) * 8;
>> +int w8, h8, w4, h4, w2, h2;
>> +height = FFALIGN(height / 8, 2) * 8;
>> +s->plane[i].width = width;
>> +s->plane[i].height = height;
>> +s->plane[i].stride = stride;
>> +
>> +w8 = FFALIGN(s->plane[i].width / 8, 8);
>> +h8 = FFALIGN(s->plane[i].height / 8, 2);
>> +w4 = w8 * 2;
>> +h4 = h8 * 2;
>> +w2 = w4 * 2;
>> +h2 = h4 * 2;
>> +
>> +s->plane[i].idwt_buf = av_malloc(height * strid

[libav-devel] [PATCH] mpeg12dec: Always close reader on error

2016-01-25 Thread Vittorio Giovara
A reader left open may lead to hangs.

Signed-off-by: Vittorio Giovara 
---
 libavcodec/mpeg12dec.c | 63 ++
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 711b310..882fdf3 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -128,9 +128,10 @@ static int mpeg_decode_motion(MpegEncContext *s, int 
fcode, int pred)
 return sign_extend(val, 5 + shift);
 }
 
+#define MAX_INDEX (64 - 1)
 #define check_scantable_index(ctx, x) \
 do {  \
-if ((x) > 63) {   \
+if ((x) > MAX_INDEX) {\
 av_log(ctx->avctx, AV_LOG_ERROR, "ac-tex damaged at %d %d\n", \
ctx->mb_x, ctx->mb_y); \
 return AVERROR_INVALIDDATA;   \
@@ -170,7 +171,8 @@ static inline int mpeg1_decode_block_intra(MpegEncContext 
*s,
 break;
 } else if (level != 0) {
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 level = (level * qscale * quant_matrix[j]) >> 4;
 level = (level - 1) | 1;
@@ -192,7 +194,8 @@ static inline int mpeg1_decode_block_intra(MpegEncContext 
*s,
 LAST_SKIP_BITS(re, &s->gb, 8);
 }
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 if (level < 0) {
 level = -level;
@@ -209,6 +212,9 @@ static inline int mpeg1_decode_block_intra(MpegEncContext 
*s,
 }
 CLOSE_READER(re, &s->gb);
 }
+
+check_scantable_index(s, i);
+
 s->block_last_index[n] = i;
 return 0;
 }
@@ -250,7 +256,8 @@ static inline int mpeg1_decode_block_inter(MpegEncContext 
*s,
 
 if (level != 0) {
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 level = ((level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
 level = (level - 1) | 1;
@@ -272,7 +279,8 @@ static inline int mpeg1_decode_block_inter(MpegEncContext 
*s,
 SKIP_BITS(re, &s->gb, 8);
 }
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 if (level < 0) {
 level = -level;
@@ -294,6 +302,9 @@ end:
 LAST_SKIP_BITS(re, &s->gb, 2);
 CLOSE_READER(re, &s->gb);
 }
+
+check_scantable_index(s, i);
+
 s->block_last_index[n] = i;
 return 0;
 }
@@ -330,7 +341,8 @@ static inline int 
mpeg1_fast_decode_block_inter(MpegEncContext *s,
 
 if (level != 0) {
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 level = ((level * 2 + 1) * qscale) >> 1;
 level = (level - 1) | 1;
@@ -352,7 +364,8 @@ static inline int 
mpeg1_fast_decode_block_inter(MpegEncContext *s,
 SKIP_BITS(re, &s->gb, 8);
 }
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 if (level < 0) {
 level = -level;
@@ -374,6 +387,9 @@ end:
 LAST_SKIP_BITS(re, &s->gb, 2);
 CLOSE_READER(re, &s->gb);
 }
+
+check_scantable_index(s, i);
+
 s->block_last_index[n] = i;
 return 0;
 }
@@ -419,7 +435,8 @@ static inline int 
mpeg2_decode_block_non_intra(MpegEncContext *s,
 
 if (level != 0) {
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 level = ((level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
 level = (level ^ SHOW_SBITS(re, &s->gb, 1)) -
@@ -434,7 +451,8 @@ static inline int 
mpeg2_decode_block_non_intra(MpegEncContext *s,
 SKIP_BITS(re, &s->gb, 12);
 
 i += run;
-check_scantable_index(s, i);
+if (i > MAX_INDEX)
+break;
 j = scantable[i];
 if (level < 0) {
 level = ((-level * 2 + 1) * qscale * quant_matrix[j]) >> 5;
@@ -456,6 +474,8 @@ end:
 }

[libav-devel] [PATCH 2/3] mpeg1: Make intra-block decoding independent of MpegEncContext

2016-01-25 Thread Vittorio Giovara
This allows untangling the eatqi decoder from the MPEG-1 decoder.

Signed-off-by: Vittorio Giovara 
---
 libavcodec/eatqi.c |   9 -
 libavcodec/mpeg12.c|  89 +++
 libavcodec/mpeg12.h|   6 ++-
 libavcodec/mpeg12dec.c | 101 +++--
 4 files changed, 114 insertions(+), 91 deletions(-)

diff --git a/libavcodec/eatqi.c b/libavcodec/eatqi.c
index f5dcb53..59bba97 100644
--- a/libavcodec/eatqi.c
+++ b/libavcodec/eatqi.c
@@ -66,9 +66,14 @@ static int tqi_decode_mb(MpegEncContext *s, int16_t 
(*block)[64])
 {
 int n;
 s->bdsp.clear_blocks(block[0]);
-for (n=0; n<6; n++)
-if (ff_mpeg1_decode_block_intra(s, block[n], n) < 0)
+for (n = 0; n < 6; n++) {
+int ret = ff_mpeg1_decode_block_intra(&s->gb,
+  s->intra_matrix,
+  s->intra_scantable.permutated,
+  s->last_dc, block[n], n, 1);
+if (ret < 0)
 return -1;
+}
 
 return 0;
 }
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 69c6d0a..9e610d3 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -236,3 +236,92 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const 
uint8_t *buf, int buf_size,
 pc->state = state;
 return END_NOT_FOUND;
 }
+
+int ff_mpeg1_decode_block_intra(GetBitContext *gb,
+const uint16_t *quant_matrix,
+uint8_t *const scantable, int last_dc[3],
+int16_t *block, int index, int qscale)
+{
+int dc, diff, component;
+int i, j;
+
+/* DC coefficient */
+component = index <= 3 ? 0 : index - 4 + 1;
+
+diff = decode_dc(gb, component);
+if (diff >= 0x)
+return AVERROR_INVALIDDATA;
+
+dc  = last_dc[component];
+dc += diff;
+last_dc[component] = dc;
+
+block[0] = dc * quant_matrix[0];
+ff_dlog(NULL, "dc=%d diff=%d\n", dc, diff);
+i = 0;
+
+{
+OPEN_READER(re, gb);
+/* now quantify & encode AC coefficients */
+while (1) {
+int level, run;
+RLTable *rl = &ff_rl_mpeg1;
+
+UPDATE_CACHE(re, gb);
+GET_RL_VLC(level, run, re, gb, rl->rl_vlc[0], TEX_VLC_BITS, 2, 0);
+
+if (level == 127) {
+break;
+} else if (level != 0) {
+i += run;
+if (i > 63)
+break;
+
+j = scantable[i];
+level = (level * qscale * quant_matrix[j]) >> 4;
+level = (level - 1) | 1;
+level = (level ^ SHOW_SBITS(re, gb, 1)) -
+SHOW_SBITS(re, gb, 1);
+LAST_SKIP_BITS(re, gb, 1);
+} else {
+/* escape */
+run = SHOW_UBITS(re, gb, 6) + 1;
+LAST_SKIP_BITS(re, gb, 6);
+UPDATE_CACHE(re, gb);
+level = SHOW_SBITS(re, gb, 8);
+SKIP_BITS(re, gb, 8);
+
+if (level == -128) {
+level = SHOW_UBITS(re, gb, 8) - 256;
+LAST_SKIP_BITS(re, gb, 8);
+} else if (level == 0) {
+level = SHOW_UBITS(re, gb, 8);
+LAST_SKIP_BITS(re, gb, 8);
+}
+
+i += run;
+if (i > 63)
+break;
+
+j = scantable[i];
+if (level < 0) {
+level = -level;
+level = (level * qscale * quant_matrix[j]) >> 4;
+level = (level - 1) | 1;
+level = -level;
+} else {
+level = (level * qscale * quant_matrix[j]) >> 4;
+level = (level - 1) | 1;
+}
+}
+
+block[j] = level;
+}
+CLOSE_READER(re, gb);
+}
+
+if (i > 63)
+i = AVERROR_INVALIDDATA;
+
+return i;
+}
diff --git a/libavcodec/mpeg12.h b/libavcodec/mpeg12.h
index fda1a53..c0a86b6 100644
--- a/libavcodec/mpeg12.h
+++ b/libavcodec/mpeg12.h
@@ -50,7 +50,11 @@ static inline int decode_dc(GetBitContext *gb, int component)
 return diff;
 }
 
-int ff_mpeg1_decode_block_intra(MpegEncContext *s, int16_t *block, int n);
+int ff_mpeg1_decode_block_intra(GetBitContext *gb,
+const uint16_t *quant_matrix,
+uint8_t *const scantable, int last_dc[3],
+int16_t *block, int index, int qscale);
+
 void ff_mpeg1_clean_buffers(MpegEncContext *s);
 int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int 
buf_size, AVCodecParserContext *s);
 
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 1e1e4a5..9b6c732 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg1

[libav-devel] [PATCH 3/3] eatqi: Remove MpegEncContext dependency

2016-01-25 Thread Vittorio Giovara
Signed-off-by: Vittorio Giovara 
---
 configure   |  2 +-
 libavcodec/Makefile |  2 +-
 libavcodec/eatqi.c  | 90 ++---
 3 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/configure b/configure
index c5bcb78..b819945 100755
--- a/configure
+++ b/configure
@@ -1942,7 +1942,7 @@ eac3_decoder_select="ac3_decoder"
 eac3_encoder_select="ac3_encoder"
 eamad_decoder_select="aandcttables blockdsp bswapdsp idctdsp mpegvideo"
 eatgq_decoder_select="aandcttables idctdsp"
-eatqi_decoder_select="aandcttables blockdsp bswapdsp idctdsp 
mpeg1video_decoder"
+eatqi_decoder_select="aandcttables blockdsp bswapdsp idctdsp"
 exr_decoder_deps="zlib"
 ffv1_decoder_select="golomb rangecoder"
 ffv1_encoder_select="rangecoder"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 4800779..0fbe56d 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -208,7 +208,7 @@ OBJS-$(CONFIG_EAMAD_DECODER)   += eamad.o eaidct.o 
mpeg12.o \
   mpeg12data.o
 OBJS-$(CONFIG_EATGQ_DECODER)   += eatgq.o eaidct.o
 OBJS-$(CONFIG_EATGV_DECODER)   += eatgv.o
-OBJS-$(CONFIG_EATQI_DECODER)   += eatqi.o eaidct.o
+OBJS-$(CONFIG_EATQI_DECODER)   += eatqi.o eaidct.o mpeg12.o 
mpeg12data.o
 OBJS-$(CONFIG_EIGHTBPS_DECODER)+= 8bps.o
 OBJS-$(CONFIG_EIGHTSVX_EXP_DECODER)+= 8svx.o
 OBJS-$(CONFIG_EIGHTSVX_FIB_DECODER)+= 8svx.o
diff --git a/libavcodec/eatqi.c b/libavcodec/eatqi.c
index 59bba97..cbeee6e 100644
--- a/libavcodec/eatqi.c
+++ b/libavcodec/eatqi.c
@@ -35,42 +35,51 @@
 #include "idctdsp.h"
 #include "internal.h"
 #include "mpeg12.h"
-#include "mpegvideo.h"
 
 typedef struct TqiContext {
-MpegEncContext s;
+GetBitContext gb;
+BlockDSPContext bdsp;
 BswapDSPContext bsdsp;
+IDCTDSPContext idsp;
+ScanTable intra_scantable;
+
 void *bitstream_buf;
 unsigned int bitstream_buf_size;
+
+int mb_x, mb_y;
+uint16_t intra_matrix[64];
+int last_dc[3];
+
 DECLARE_ALIGNED(16, int16_t, block)[6][64];
 } TqiContext;
 
 static av_cold int tqi_decode_init(AVCodecContext *avctx)
 {
 TqiContext *t = avctx->priv_data;
-MpegEncContext *s = &t->s;
-s->avctx = avctx;
-ff_blockdsp_init(&s->bdsp, avctx);
+
+ff_blockdsp_init(&t->bdsp, avctx);
 ff_bswapdsp_init(&t->bsdsp);
-ff_idctdsp_init(&s->idsp, avctx);
-ff_init_scantable_permutation(s->idsp.idct_permutation, FF_IDCT_PERM_NONE);
-ff_init_scantable(s->idsp.idct_permutation, &s->intra_scantable, 
ff_zigzag_direct);
-s->qscale = 1;
+ff_idctdsp_init(&t->idsp, avctx);
+ff_init_scantable_permutation(t->idsp.idct_permutation, FF_IDCT_PERM_NONE);
+ff_init_scantable(t->idsp.idct_permutation, &t->intra_scantable, 
ff_zigzag_direct);
+
 avctx->framerate = (AVRational){ 15, 1 };
 avctx->pix_fmt = AV_PIX_FMT_YUV420P;
 ff_mpeg12_init_vlcs();
 return 0;
 }
 
-static int tqi_decode_mb(MpegEncContext *s, int16_t (*block)[64])
+static int tqi_decode_mb(AVCodecContext *avctx, int16_t (*block)[64])
 {
+TqiContext *t = avctx->priv_data;
 int n;
-s->bdsp.clear_blocks(block[0]);
+
+t->bdsp.clear_blocks(block[0]);
 for (n = 0; n < 6; n++) {
-int ret = ff_mpeg1_decode_block_intra(&s->gb,
-  s->intra_matrix,
-  s->intra_scantable.permutated,
-  s->last_dc, block[n], n, 1);
+int ret = ff_mpeg1_decode_block_intra(&t->gb,
+  t->intra_matrix,
+  t->intra_scantable.permutated,
+  t->last_dc, block[n], n, 1);
 if (ret < 0)
 return -1;
 }
@@ -78,31 +87,35 @@ static int tqi_decode_mb(MpegEncContext *s, int16_t 
(*block)[64])
 return 0;
 }
 
-static inline void tqi_idct_put(TqiContext *t, AVFrame *frame, int16_t 
(*block)[64])
+static inline void tqi_idct_put(AVCodecContext *avctx, AVFrame *frame,
+int16_t (*block)[64])
 {
-MpegEncContext *s = &t->s;
+TqiContext *t = avctx->priv_data;
 int linesize = frame->linesize[0];
-uint8_t *dest_y  = frame->data[0] + (s->mb_y * 16* linesize) + 
s->mb_x * 16;
-uint8_t *dest_cb = frame->data[1] + (s->mb_y * 8 * frame->linesize[1]) + 
s->mb_x * 8;
-uint8_t *dest_cr = frame->data[2] + (s->mb_y * 8 * frame->linesize[2]) + 
s->mb_x * 8;
+uint8_t *dest_y  = frame->data[0] + t->mb_y * 16 * linesize   + 
t->mb_x * 16;
+uint8_t *dest_cb = frame->data[1] + t->mb_y *  8 * frame->linesize[1] + 
t->mb_x *  8;
+uint8_t *dest_cr = frame->data[2] + t->mb_y *  8 * frame->linesize[2] + 
t->mb_x *  8;
 
 ff_ea_idct_put_c(dest_y , linesize, block[0]);
 ff_ea_idct_put_c(dest_y  + 8, linesize, block[1]);
 ff_ea_

[libav-devel] [PATCH 1/3] mpeg12dec: Always close reader on error

2016-01-25 Thread Vittorio Giovara
A reader left open may lead to hangs.

Signed-off-by: Vittorio Giovara 
---
 libavcodec/mpeg12dec.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 711b310..1e1e4a5 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -170,7 +170,8 @@ static inline int mpeg1_decode_block_intra(MpegEncContext 
*s,
 break;
 } else if (level != 0) {
 i += run;
-check_scantable_index(s, i);
+if (i > 63)
+break;
 j = scantable[i];
 level = (level * qscale * quant_matrix[j]) >> 4;
 level = (level - 1) | 1;
@@ -192,7 +193,8 @@ static inline int mpeg1_decode_block_intra(MpegEncContext 
*s,
 LAST_SKIP_BITS(re, &s->gb, 8);
 }
 i += run;
-check_scantable_index(s, i);
+if (i > 63)
+break;
 j = scantable[i];
 if (level < 0) {
 level = -level;
@@ -209,6 +211,9 @@ static inline int mpeg1_decode_block_intra(MpegEncContext 
*s,
 }
 CLOSE_READER(re, &s->gb);
 }
+
+check_scantable_index(s, i);
+
 s->block_last_index[n] = i;
 return 0;
 }
-- 
2.6.4

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/2] eatqi: Remove mpegvideo dependency

2016-01-25 Thread Vittorio Giovara
On Fri, Jan 22, 2016 at 1:36 AM, Diego Biurrun  wrote:
> On Thu, Jan 21, 2016 at 04:53:56PM -0500, Vittorio Giovara wrote:
>> --- a/libavcodec/eatqi.c
>> +++ b/libavcodec/eatqi.c
>> @@ -137,15 +149,15 @@ static int tqi_decode_frame(AVCodecContext *avctx,
>> -
>> -s->last_dc[0] = s->last_dc[1] = s->last_dc[2] = 0;
>> +t->last_dc[0] = t->last_dc[1] = t->last_dc[2] = 0;
>
> Please break these lines while you're at it for readability.

I'll change this line to memset(t->last_dc, 0,
sizeof(t->last_dc)); if that's all right
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/3] pixdesc: Use AV_CEIL_RSHIFT in documentation

2016-01-25 Thread Vittorio Giovara
On Thu, Jan 21, 2016 at 6:00 PM, Vittorio Giovara
 wrote:
> ---
>  libavutil/pixdesc.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
> index fe06d94..f52c434 100644
> --- a/libavutil/pixdesc.h
> +++ b/libavutil/pixdesc.h
> @@ -84,16 +84,16 @@ typedef struct AVPixFmtDescriptor {
>  /**
>   * Amount to shift the luma width right to find the chroma width.
>   * For YV12 this is 1 for example.
> - * chroma_width = -((-luma_width) >> log2_chroma_w)
> + * chroma_width = AV_CEIL_RSHIFT(luma_width, log2_chroma_w)
>   * The note above is needed to ensure rounding up.
>   * This value only refers to the chroma components.
>   */
> -uint8_t log2_chroma_w;  ///< chroma_width = -((-luma_width 
> )>>log2_chroma_w)
> +uint8_t log2_chroma_w;
>
>  /**
>   * Amount to shift the luma height right to find the chroma height.
>   * For YV12 this is 1 for example.
> - * chroma_height= -((-luma_height) >> log2_chroma_h)
> + * chroma_height= AV_CEIL_RSHIFT(luma_height, log2_chroma_h)
>   * The note above is needed to ensure rounding up.
>   * This value only refers to the chroma components.
>   */
> --
> 2.6.4
>

ok'd by Luca on IRC

-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCHv3] avcodec: Cineform HD Decoder

2016-01-25 Thread Vittorio Giovara
On Sun, Jan 24, 2016 at 7:34 PM, Kieran Kunhya  wrote:
> Decodes YUV 4:2:2 10-bit and RGB 12-bit files.
> Older files with more subbands, skips, Bayer, alpha not supported.
> Alpha requires addition of GBRAP12 pixel format.

Actually you could set AV_PIX_FMT_GBRAP16 and tweak the
bits_per_raw_sample, not sure about the repercussion on the users
though.

> ---
> +static av_cold int cfhd_decode_init(AVCodecContext *avctx)

is this function _decode or _init? or is it decoder_init? imho names
would be simpler with just _ scheme.

> +{
> +CFHDContext *s = avctx->priv_data;
> +
> +avctx->pix_fmt = AV_PIX_FMT_YUV422P10;

if the decoder supports multiple pixel formats it's better not to
initialize the pixel format here, and wait until decode(). Otherwise
it's going to cause a "parameters changed" warning and reinit any
previous filter chain.

> +avctx->bits_per_raw_sample = 10;
> +s->avctx   = avctx;
> +avctx->width   = 0;
> +avctx->height  = 0;

isn't the context mallocz anyway?

> +return ff_cfhd_init_vlcs(s);
> +}
> +

> +static inline void filter(int16_t *output, ptrdiff_t out_stride, int16_t 
> *low, ptrdiff_t low_stride,
> +  int16_t *high, ptrdiff_t high_stride, int len, 
> uint8_t clip)
> +{
> +int16_t tmp;
> +
> +int i;
> +for (i = 0; i < len; i++) {
> +if (i == 0) {
> +tmp = (11*low[0*low_stride] - 4*low[1*low_stride] + 
> low[2*low_stride] + 4) >> 3;
> +output[(2*i+0)*out_stride] = (tmp + high[0*high_stride]) >> 1;
> +if (clip)
> +output[(2*i+0)*out_stride] = 
> av_clip_uintp2(output[(2*i+0)*out_stride], clip);
> +
> +tmp = ( 5*low[0*low_stride] + 4*low[1*low_stride] - 
> low[2*low_stride] + 4) >> 3;
> +output[(2*i+1)*out_stride] = (tmp - high[0*high_stride]) >> 1;
> +if (clip)
> +output[(2*i+1)*out_stride] = 
> av_clip_uintp2(output[(2*i+1)*out_stride], clip);
> +}
> +else if (i == len-1){

nit, spacing and new line are still off

> +tmp = ( 5*low[i*low_stride] + 4*low[(i-1)*low_stride] - 
> low[(i-2)*low_stride] + 4) >> 3;
> +output[(2*i+0)*out_stride] = (tmp + high[i*high_stride]) >> 1;
> +if (clip)
> +output[(2*i+0)*out_stride] = 
> av_clip_uintp2(output[(2*i+0)*out_stride], clip);
> +
> +tmp = (11*low[i*low_stride] - 4*low[(i-1)*low_stride] + 
> low[(i-2)*low_stride] + 4) >> 3;
> +output[(2*i+1)*out_stride] = (tmp - high[i*high_stride]) >> 1;
> +if (clip)
> +output[(2*i+1)*out_stride] = 
> av_clip_uintp2(output[(2*i+1)*out_stride], clip);
> +}
> +else {
> +tmp = (low[(i-1)*low_stride] - low[(i+1)*low_stride] + 4) >> 3;
> +output[(2*i+0)*out_stride] = (tmp + low[i*low_stride] + 
> high[i*high_stride]) >> 1;
> +if (clip)
> +output[(2*i+0)*out_stride] = 
> av_clip_uintp2(output[(2*i+0)*out_stride], clip);
> +
> +tmp = (low[(i+1)*low_stride] - low[(i-1)*low_stride] + 4) >> 3;
> +output[(2*i+1)*out_stride] = (tmp + low[i*low_stride] - 
> high[i*high_stride]) >> 1;
> +if (clip)
> +output[(2*i+1)*out_stride] = 
> av_clip_uintp2(output[(2*i+1)*out_stride], clip);
> +}
> +}
> +}

+1 on the dsp suggestion

> +}
> +
> +static int alloc_buffers(AVCodecContext *avctx)
> +{
> +CFHDContext *s = avctx->priv_data;
> +int i, j, k;
> +
> +avctx->width = s->coded_width;
> +avctx->height = s->coded_height;

I think ff_set_dimensions is the function you're looking for (make
sure to check its return value)

> +avcodec_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_x_shift, 
> &s->chroma_y_shift);

this again? :)

> +for (i = 0; i < 3; i++) {
> +int width = i ? avctx->width >> s->chroma_x_shift : avctx->width;
> +int height = i ? avctx->height >> s->chroma_y_shift : avctx->height;

could these be candidates for AV_CEIL_RSHIFT?

> +int stride = FFALIGN(width / 8, 8) * 8;
> +int w8, h8, w4, h4, w2, h2;
> +height = FFALIGN(height / 8, 2) * 8;
> +s->plane[i].width = width;
> +s->plane[i].height = height;
> +s->plane[i].stride = stride;
> +
> +w8 = FFALIGN(s->plane[i].width / 8, 8);
> +h8 = FFALIGN(s->plane[i].height / 8, 2);
> +w4 = w8 * 2;
> +h4 = h8 * 2;
> +w2 = w4 * 2;
> +h2 = h4 * 2;
> +
> +s->plane[i].idwt_buf = av_malloc(height * stride * 
> sizeof(*s->plane[i].idwt_buf));
> +s->plane[i].idwt_tmp = av_malloc(height * stride * 
> sizeof(*s->plane[i].idwt_tmp));
> +if (!s->plane[i].idwt_buf || !s->plane[i].idwt_tmp) {
> +return AVERROR(ENOMEM);

need to av_freep both since you don't know which one failed

> +}

> +av_log(avctx, AV_LOG_DEBUG, "Start subband coef

Re: [libav-devel] [FFmpeg-devel] [PATCHv3] avcodec: Cineform HD Decoder

2016-01-25 Thread Ronald S. Bultje
Hi,

On Sun, Jan 24, 2016 at 7:34 PM, Kieran Kunhya  wrote:

> +static inline void filter(int16_t *output, ptrdiff_t out_stride, int16_t
> *low, ptrdiff_t low_stride,
> +  int16_t *high, ptrdiff_t high_stride, int len,
> uint8_t clip)


Should this be a DSP function? (That is, the functions calling this.)

They seem very SIMD'able.

Ronald
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/2] mpeg1: Make intra-block decoding independent of mpegvideoenc

2016-01-25 Thread Diego Biurrun
On Fri, Jan 22, 2016 at 10:24:09AM -0500, Vittorio Giovara wrote:
> On Fri, Jan 22, 2016 at 1:47 AM, Diego Biurrun  wrote:
> > On Thu, Jan 21, 2016 at 04:53:55PM -0500, Vittorio Giovara wrote:
> >> --- a/libavcodec/mpeg12.c
> >> +++ b/libavcodec/mpeg12.c
> >> @@ -236,3 +236,92 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const 
> >> uint8_t *buf, int buf_size,
> >> +
> >> +int ff_mpeg1_decode_block_intra(GetBitContext *gb,
> >> +const uint16_t *quant_matrix,
> >> +uint8_t *const scantable, int last_dc[3],
> >> +int16_t *block, int index, int qscale)
> >> +{
> >> +if (level == 127) {
> >> +break;
> >> +} else if (level != 0) {
> >> +i += run;
> >> +if (i > 63)
> >> +break;
> >> +
> >> --- a/libavcodec/mpeg12dec.c
> >> +++ b/libavcodec/mpeg12dec.c
> >> @@ -137,87 +137,6 @@ static int mpeg_decode_motion(MpegEncContext *s, int 
> >> fcode, int pred)
> >>
> >> -static inline int mpeg1_decode_block_intra(MpegEncContext *s,
> >> -   int16_t *block, int n)
> >> -{
> >> -
> >> -if (level == 127) {
> >> -break;
> >> -} else if (level != 0) {
> >> -i += run;
> >> -check_scantable_index(s, i);
> >
> > Before this seemed to return INVALIDDATA from check_scantable_index,
> > now it does something else.  Suspicious.
> 
> yes as part of simplification I dropped the avctx so all logs and
> return values are checked later, and what I did led to unwinding this
> macro: before returning there is a single (i > 63) check that
> overrides it to INVALIDDATA, so the same behaviour is kept

Could you please do such simplifications w/o moving the code around at
the same time?  It's hard to figure out what changed while the code is
on the move.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/2] configure: add missing avx2 support check

2016-01-25 Thread Luca Barbato
On 25/01/16 10:45, Diego Biurrun wrote:
> Those are 4-5 years old; bump requirements instead?

I already added the guards, it is more consistent with the rest of the
codebase and I guess whoever wants to build w/out a specific optional
component might like not having stray code even if he could build it =)

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/2] configure: add missing avx2 support check

2016-01-25 Thread Diego Biurrun
On Sun, Jan 24, 2016 at 02:10:44AM -0300, James Almer wrote:
> AVX2 support was introduced in Yasm 1.2.0 and NASM 2.10, and the
> oldest versions currently supported are Yasm 0.8.0 and NASM 2.03

Those are 4-5 years old; bump requirements instead?

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel