Re: [libav-devel] [PATCH] channel_layout: Add a 16channel default layout

2015-08-23 Thread Justin Ruggles

On 08/23/2015 04:57 AM, Luca Barbato wrote:

On 22/08/15 03:35, Justin Ruggles wrote:

On 08/21/2015 03:02 AM, Luca Barbato wrote:

---

Now octagonal has a buddy!

   libavutil/channel_layout.c | 2 ++
   libavutil/channel_layout.h | 1 +
   2 files changed, 3 insertions(+)


OK then..


Not sure if I understand correctly the questions.


sample?


blackmagic devices decklink can produce this kind of data, since it is
sort of big I can send it to you privately a sample file.


example?


my github has them, check bmdtools.


documentation?


Not sure which kind of documentation you have in mind.


That's enough. Looks fine.

-Justin


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


Re: [libav-devel] [PATCH] channel_layout: Add a 16channel default layout

2015-08-21 Thread Justin Ruggles

On 08/21/2015 03:02 AM, Luca Barbato wrote:

---

Now octagonal has a buddy!

  libavutil/channel_layout.c | 2 ++
  libavutil/channel_layout.h | 1 +
  2 files changed, 3 insertions(+)


OK then..

sample? example? documentation?

-Justin

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


Re: [libav-devel] [PATCH] filtergraph: Select the best pixel format

2015-08-09 Thread Justin Ruggles

On 08/07/2015 06:13 PM, Luca Barbato wrote:

Give priority to pixel formats closer to the input.
---

I kept it as simple as possible on purpose.

  libavfilter/avfiltergraph.c | 97 +
  1 file changed, 97 insertions(+)

diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index 0fc385c..91a9d71 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -31,6 +31,7 @@
  #include libavutil/internal.h
  #include libavutil/log.h
  #include libavutil/opt.h
+#include libavutil/pixdesc.h

  #include avfilter.h
  #include formats.h
@@ -742,6 +743,100 @@ static int pick_formats(AVFilterGraph *graph)
  return 0;
  }

+#define same_flag(d1, d2, flag) \
+!(!!(d1-flags  flag) ^ !!(d2-flags  flag))
+
+static int find_best_pix_fmt_match(AVFilterLink *outlink,
+   const AVPixFmtDescriptor *in_desc,
+   int in_format)
+{
+int j;
+int best_idx = -1, best_score = INT_MIN;
+
+for (j = 0; j  outlink-in_formats-nb_formats; j++) {
+int format = outlink-in_formats-formats[j];
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format);
+
+int score;
+
+int same_alpha = same_flag(desc, in_desc, AV_PIX_FMT_FLAG_ALPHA);
+int same_rgb   = same_flag(desc, in_desc, AV_PIX_FMT_FLAG_RGB);
+int bpc= desc-comp[0].depth_minus1;
+int in_bpc = in_desc-comp[0].depth_minus1;
+
+if (bpc  in_bpc)
+score = bpc - in_bpc;
+else
+score = 0;
+
+score += -10 * abs(desc-nb_components -
+   in_desc-nb_components);
+
+if (same_alpha  same_rgb) {
+score += 2;
+} else if (desc-flags  AV_PIX_FMT_FLAG_ALPHA) {
+if (same_alpha)
+score += 1;
+} else {
+if (same_rgb)
+score += 1;
+}
+
+if (score  best_score) {
+best_score = score;
+best_idx   = j;
+}
+}
+
+return best_idx;
+}
+
+static void swap_pix_fmts_on_filter(AVFilterContext *filter)
+{
+AVFilterLink *link = NULL;
+const AVPixFmtDescriptor *desc;
+int format;
+int i;
+
+for (i = 0; i  filter-nb_inputs; i++) {
+link = filter-inputs[i];
+
+if (link-type == AVMEDIA_TYPE_VIDEO 
+link-out_formats-nb_formats == 1)
+break;
+}
+if (i == filter-nb_inputs)
+return;
+
+format = link-out_formats-formats[0];
+desc   = av_pix_fmt_desc_get(format);
+
+for (i = 0; i  filter-nb_outputs; i++) {
+AVFilterLink *outlink = filter-outputs[i];
+int best_idx;
+
+if (outlink-type != AVMEDIA_TYPE_VIDEO ||
+outlink-in_formats-nb_formats  2)
+continue;
+
+best_idx = find_best_pix_fmt_match(outlink, desc, format);
+
+if (best_idx  -1)
+FFSWAP(int, outlink-in_formats-formats[0],
+   outlink-in_formats-formats[best_idx]);
+}
+}


I'm not so sure I like how the scoring is implemented here. The most 
logical to me would be a strict layered matching. Ideally, my preference 
would be, in order or priority:


- preserve alpha only if the input has alpha
- preserve average, non-alpha, bit depth
- preserve colorspace
- preserve individual component (including alpha) bit depths, if applicable
- preserve component ordering/packing

Something like this could still be done within a scoring system.

Cheers,
Justin

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


Re: [libav-devel] [PATCH] filtergraph: Select the best pixel format

2015-08-09 Thread Justin Ruggles

On 08/09/2015 07:35 PM, Luca Barbato wrote:

On 09/08/15 23:16, Justin Ruggles wrote:

On 08/07/2015 06:13 PM, Luca Barbato wrote:

Give priority to pixel formats closer to the input.
---

I kept it as simple as possible on purpose.

   libavfilter/avfiltergraph.c | 97
+
   1 file changed, 97 insertions(+)

diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index 0fc385c..91a9d71 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -31,6 +31,7 @@
   #include libavutil/internal.h
   #include libavutil/log.h
   #include libavutil/opt.h
+#include libavutil/pixdesc.h

   #include avfilter.h
   #include formats.h
@@ -742,6 +743,100 @@ static int pick_formats(AVFilterGraph *graph)
   return 0;
   }

+#define same_flag(d1, d2, flag) \
+!(!!(d1-flags  flag) ^ !!(d2-flags  flag))
+
+static int find_best_pix_fmt_match(AVFilterLink *outlink,
+   const AVPixFmtDescriptor *in_desc,
+   int in_format)
+{
+int j;
+int best_idx = -1, best_score = INT_MIN;
+
+for (j = 0; j  outlink-in_formats-nb_formats; j++) {
+int format =
outlink-in_formats-formats[j];
+const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format);
+
+int score;
+
+int same_alpha = same_flag(desc, in_desc,
AV_PIX_FMT_FLAG_ALPHA);
+int same_rgb   = same_flag(desc, in_desc, AV_PIX_FMT_FLAG_RGB);
+int bpc= desc-comp[0].depth_minus1;
+int in_bpc = in_desc-comp[0].depth_minus1;
+
+if (bpc  in_bpc)
+score = bpc - in_bpc;
+else
+score = 0;
+
+score += -10 * abs(desc-nb_components -
+   in_desc-nb_components);
+
+if (same_alpha  same_rgb) {
+score += 2;
+} else if (desc-flags  AV_PIX_FMT_FLAG_ALPHA) {
+if (same_alpha)
+score += 1;
+} else {
+if (same_rgb)
+score += 1;
+}
+
+if (score  best_score) {
+best_score = score;
+best_idx   = j;
+}
+}
+
+return best_idx;
+}
+
+static void swap_pix_fmts_on_filter(AVFilterContext *filter)
+{
+AVFilterLink *link = NULL;
+const AVPixFmtDescriptor *desc;
+int format;
+int i;
+
+for (i = 0; i  filter-nb_inputs; i++) {
+link = filter-inputs[i];
+
+if (link-type == AVMEDIA_TYPE_VIDEO 
+link-out_formats-nb_formats == 1)
+break;
+}
+if (i == filter-nb_inputs)
+return;
+
+format = link-out_formats-formats[0];
+desc   = av_pix_fmt_desc_get(format);
+
+for (i = 0; i  filter-nb_outputs; i++) {
+AVFilterLink *outlink = filter-outputs[i];
+int best_idx;
+
+if (outlink-type != AVMEDIA_TYPE_VIDEO ||
+outlink-in_formats-nb_formats  2)
+continue;
+
+best_idx = find_best_pix_fmt_match(outlink, desc, format);
+
+if (best_idx  -1)
+FFSWAP(int, outlink-in_formats-formats[0],
+   outlink-in_formats-formats[best_idx]);
+}
+}


I'm not so sure I like how the scoring is implemented here. The most
logical to me would be a strict layered matching. Ideally, my preference
would be, in order or priority:

- preserve alpha only if the input has alpha


Can be done.


- preserve average, non-alpha, bit depth


You end up with 16bits gray while 8bits gray is more than enough.


Yeah, I suppose preserving color if input has color could be the top 
priority.





- preserve colorspace


Ok


- preserve individual component (including alpha) bit depths, if applicable


Ok, I can extend what I put there, that part seems working quite well.


- preserve component ordering/packing

Something like this could still be done within a scoring system.


I'll update the rules soon.

lu




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



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


Re: [libav-devel] [PATCH 9/9] af_resample: do not touch the timestamps if we are not resampling

2015-07-16 Thread Justin Ruggles


On 07/16/2015 02:56 PM, Anton Khirnov wrote:

This filter currently assumes that the input audio is continuous and
does some timestamps manipulation based on this assumption.

This is unnecessary if we are only converting the channel layout or the
sample format, without resampling. In such a case, just leave the
timestamps as they are.
---
  libavfilter/af_resample.c | 50 +--
  1 file changed, 31 insertions(+), 19 deletions(-)


LGTM

-Justin

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


Re: [libav-devel] [PATCH] lavc/libwebpenc: use WebPMemoryWriterClear()

2015-06-16 Thread Justin Ruggles

On 06/16/2015 12:27 PM, Vittorio Giovara wrote:

From: James Almer jamr...@gmail.com

WebPMemoryWriterClear() must be used instead of free() when libwebp ABI version is 
 0x0203

Reviewed-by: Michael Niedermayer michae...@gmx.at
Signed-off-by: James Almer jamr...@gmail.com
---
  libavcodec/libwebpenc.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/libavcodec/libwebpenc.c b/libavcodec/libwebpenc.c
index 5283da5..4cb8dc3 100644
--- a/libavcodec/libwebpenc.c
+++ b/libavcodec/libwebpenc.c
@@ -231,7 +231,11 @@ static int libwebp_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
  *got_packet = 1;

  end:
+#if (WEBP_ENCODER_ABI_VERSION  0x0203)
+WebPMemoryWriterClear(mw);
+#else
  free(mw.mem); /* must use free() according to libwebp documentation */
+#endif
  WebPPictureFree(pic);
  av_freep(pic);
  av_frame_free(alt_frame);



LGTM

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


Re: [libav-devel] [PATCH] Introduce a TextureDSP module

2015-06-01 Thread Justin Ruggles

On 05/31/2015 07:41 AM, Vittorio Giovara wrote:

+/** Convert a premultiplied alpha pixel to a straigth alpha pixel. */
+static av_always_inline void premult2straight(uint8_t *src)
+{
+int r = src[0];
+int g = src[1];
+int b = src[2];
+int a = src[3];
+
+src[0] = (uint8_t) r * a / 255;
+src[1] = (uint8_t) g * a / 255;
+src[2] = (uint8_t) b * a / 255;
+src[3] = a;
+}


The last line is unnecessary.

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


Re: [libav-devel] [PATCH 09/10] ppc: vsx: Implement float_dsp

2015-05-13 Thread Justin Ruggles

On 05/13/2015 07:46 AM, Luca Barbato wrote:

---
  arch.mak  |  1 +
  libavutil/ppc/Makefile|  2 ++
  libavutil/ppc/float_dsp_altivec.c |  2 +-
  libavutil/ppc/float_dsp_init.c| 26 +-
  4 files changed, 21 insertions(+), 10 deletions(-)

...

+VSX-OBJS += ppc/float_dsp_vsx.o \


Did you forget a file?

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


Re: [libav-devel] [PATCH] error: Add ENOCARE to AVERROR types

2015-03-31 Thread Justin Ruggles

On 03/31/2015 07:38 PM, Vittorio Giovara wrote:

This value is to be used to reflect the real intentions of the developer
regarding a correct error reporting.
---
In the codebase there are a couple of places were this is evident, and this
error kind will come in handy for the future.
Cheers,
 Vittorio

  doc/APIchanges  | 3 +++
  libavutil/error.h   | 1 +
  libavutil/version.h | 2 +-
  3 files changed, 5 insertions(+), 1 deletion(-)


Can you please give an example?

-Justin

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


Re: [libav-devel] [PATCH] tiff: Return more meaningful error codes

2015-03-29 Thread Justin Ruggles

On 03/28/2015 03:39 PM, Luca Barbato wrote:

On 28/03/15 19:04, Himangi Saraogi wrote:

This really never should happen in practice, but at any rate the correct
error value is AVERROR(EINVAL) because it is an unsupported/invalid field
set by the user.



Since this is never to be  reached, can we have an assert instead of return?



No and I'd rather not have another discussion on why using reachable
asserts is wrong (in this case in particular even more).

I think I wrote a patch to wrap __builtin_unreachable() some time ago
for the cases in which could be useful.

Incidentally _this_ is not the case.

Please return AVERROR(EINVAL), that path is easily reachable by the
appropriate input even from avconv...


EIVAL is fine, but out of curiosity, how is this reachable with avconv? 
Doesn't avcodec_open2() check avctx-pix_fmt against the codec pix_fmts 
list? I know a user could theoretically change pix_fmt after opening the 
encoder, but I don't see how avconv could.


-Justin


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


Re: [libav-devel] [PATCH] tiff: Return more meaningful error codes

2015-03-29 Thread Justin Ruggles

On 03/28/2015 11:25 PM, Himangi Saraogi wrote:

---
  libavcodec/tiffenc.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)


LGTM

-Justin

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


Re: [libav-devel] [PATCH] tiff: Return more meaningful error codes

2015-03-29 Thread Justin Ruggles

On 03/29/2015 02:10 PM, James Almer wrote:

On 28/03/15 2:52 PM, Justin Ruggles wrote:

On 03/28/2015 01:42 PM, Himangi Saraogi wrote:

---
   libavcodec/tiffenc.c | 11 ++-
   1 file changed, 6 insertions(+), 5 deletions(-)
@@ -182,7 +183,7 @@ static int encode_strip(TiffEncoderContext *s, const int8_t 
*src,
   case TIFF_LZW:
   return ff_lzw_encode(s-lzws, src, n);
   default:
-return -1;
+return AVERROR_UNKNOWN;


Should be AVERROR_BUG since compression type is an AVOption that has defined 
bounds.


No, this should be AVERROR(EINVAL) because even inside the bounds there are 
several values for
compressions that are not currently supported.
i can do avconv -i INPUT -compression_algo 2 OUTPUT and it wouldn't be a bug, 
it would be an
invalid argument.



Ah, good catch. Then yes, AVERROR(EINVAL) is appropriate here.

-Justin

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


Re: [libav-devel] [PATCH] tiff: Return more meaningful error codes

2015-03-28 Thread Justin Ruggles

On 03/28/2015 01:42 PM, Himangi Saraogi wrote:

---
  libavcodec/tiffenc.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
index 169360f..46e4207 100644
--- a/libavcodec/tiffenc.c
+++ b/libavcodec/tiffenc.c
@@ -153,7 +153,8 @@ static int add_entry1(TiffEncoderContext *s,
   * @param dst Output buffer
   * @param n Size of input buffer
   * @param compr Compression method
- * @return Number of output bytes. If an output error is encountered, -1 
returned
+ * @return Number of output bytes. If an output error is encountered, a 
negative
+ * value corresponding to an AVERROR error code is returned.
   */
  static int encode_strip(TiffEncoderContext *s, const int8_t *src,
  uint8_t *dst, int n, int compr)
@@ -166,14 +167,14 @@ static int encode_strip(TiffEncoderContext *s, const 
int8_t *src,
  unsigned long zlen = s-buf_size - (*s-buf - s-buf_start);
  if (compress(dst, zlen, src, n) != Z_OK) {
  av_log(s-avctx, AV_LOG_ERROR, Compressing failed\n);
-return -1;
+return AVERROR_INVALIDDATA;


This is an unknown error from an external library, so AVERROR_UNKNOWN 
should be returned.



  }
  return zlen;
  }
  #endif
  case TIFF_RAW:
  if (check_size(s, n))
-return -1;
+return AVERROR(EINVAL);
  memcpy(dst, src, n);
  return n;
  case TIFF_PACKBITS:
@@ -182,7 +183,7 @@ static int encode_strip(TiffEncoderContext *s, const int8_t 
*src,
  case TIFF_LZW:
  return ff_lzw_encode(s-lzws, src, n);
  default:
-return -1;
+return AVERROR_UNKNOWN;


Should be AVERROR_BUG since compression type is an AVOption that has 
defined bounds.



  }
  }

@@ -291,7 +292,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
*pkt,
  default:
  av_log(s-avctx, AV_LOG_ERROR,
 This colors format is not supported\n);
-return -1;
+return AVERROR_INVALIDDATA;


This really never should happen in practice, but at any rate the correct 
error value is AVERROR(EINVAL) because it is an unsupported/invalid 
field set by the user.



  }

  if (s-compr == TIFF_DEFLATE   ||




Thanks,
Justin

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


Re: [libav-devel] [PATCH] webp: ensure that each transform is only used once

2015-03-05 Thread Justin Ruggles

On 03/05/2015 05:02 PM, Andreas Cadhalpun wrote:

Hi,

according to the WebP Lossless Bitstream Specification [1] each
transform is allowed to be used only once. Attached patch adds checks
for this to avoid crashes decoding broken files.

Best regards,
Andreas

1:
https://developers.google.com/speed/webp/docs/webp_lossless_bitstream_specification#3_transformations


0001-webp-ensure-that-each-transform-is-only-used-once.patch



From d80baa7f786ca326891e145a000fbecdde55da80 Mon Sep 17 00:00:00 2001

From: Andreas Cadhalpunandreas.cadhal...@googlemail.com
Date: Thu, 5 Mar 2015 22:48:28 +0100
Subject: [PATCH] webp: ensure that each transform is only used once

According to the WebP Lossless Bitstream Specification
each transform is allowed to be used only once.

If a transform is more than once this can lead to memory
corruption.


Can you give more details about the memory corruption?


Signed-off-by: Andreas Cadhalpunandreas.cadhal...@googlemail.com
---
  libavcodec/webp.c | 39 +++
  1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index 9549c0e..2cd976f 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -1104,7 +1104,7 @@ static int vp8_lossless_decode_frame(AVCodecContext 
*avctx, AVFrame *p,
   unsigned int data_size, int 
is_alpha_chunk)
  {
  WebPContext *s = avctx-priv_data;
-int w, h, ret, i;
+int w, h, ret, i, used;

  if (!is_alpha_chunk) {
  s-lossless = 1;
@@ -1154,18 +1154,49 @@ static int vp8_lossless_decode_frame(AVCodecContext 
*avctx, AVFrame *p,
  /* parse transformations */
  s-nb_transforms = 0;
  s-reduced_width = 0;
+used = 0;
  while (get_bits1(s-gb)) {
  enum TransformType transform = get_bits(s-gb, 2);
  s-transforms[s-nb_transforms++] = transform;
  switch (transform) {
  case PREDICTOR_TRANSFORM:
-ret = parse_transform_predictor(s);
+if (used  1) {


Use (1  transform) and move all of these checks to a single check 
before the switch.


Thanks,
Justin

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


Re: [libav-devel] [PATCH] avcodec/webp: validate the distance prefix code

2015-03-02 Thread Justin Ruggles

On 03/02/2015 02:58 PM, Andreas Cadhalpun wrote:

Hi,

according to the WebP Lossless Bitstream Specification [1] the highest
allowed value for the prefix code is 39. Attached patch adds a check for
this to avoid crashes decoding broken files.

Best regards,
Andreas


1:
https://developers.google.com/speed/webp/docs/webp_lossless_bitstream_specification#4_image_data


Looks ok

-Justin


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


Re: [libav-devel] [PATCH] libx264: Return more meaningful error codes

2015-03-01 Thread Justin Ruggles

On 02/26/2015 06:51 PM, Himangi Saraogi wrote:

On 27 February 2015 at 05:12, Janne Grunau janne-li...@jannau.net wrote:


On 2015-02-27 04:36:41 +0530, Himangi Saraogi wrote:

---
  libavcodec/libx264.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 6388b6c..71c38cc 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -235,11 +235,11 @@ static int X264_frame(AVCodecContext *ctx,

AVPacket *pkt, const AVFrame *frame,

  }
  do {
  if (x264_encoder_encode(x4-enc, nal, nnal, frame? x4-pic:

NULL, pic_out)  0)

-return -1;
+return AVERROR_UNKNOWN;

  ret = encode_nals(ctx, pkt, nal, nnal);
  if (ret  0)
-return -1;
+return ret;
  } while (!ret  !frame  x264_encoder_delayed_frames(x4-enc));

  pkt-pts = pic_out.i_pts;
@@ -518,7 +518,7 @@ static av_cold int X264_init(AVCodecContext *avctx)

  x4-enc = x264_encoder_open(x4-params);
  if (!x4-enc)
-return -1;
+return AVERROR_INVALIDDATA;


I think AVERROR_UNKNOWN or EINVAL is a better match here, I'd guess it's
mostly likely that one of the paramters is invalid. On the otherhand
that's just guessing and it could be anything but libx264 doesn't tell
us so UNKNOWN would be valid choice too.



In libavcodec/libx265.c, we have:

 ctx-encoder = x265_encoder_open(ctx-params);
 if (!ctx-encoder) {
 av_log(avctx, AV_LOG_ERROR, Cannot open libx265 encoder.\n);
 libx265_encode_close(avctx);
 return AVERROR_INVALIDDATA;
 }
I agree that UNKNOWN is a  valid choice, but think that having uniformity
is a good idea.


UNKNOWN is the correct choice when an external library does not give 
specific error information. Change it in both places if you're concerned 
about uniformity.


-Justin


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


Re: [libav-devel] [PATCH] vaapi: Return more meaningful error codes

2015-02-23 Thread Justin Ruggles

On 02/23/2015 07:47 AM, Diego Biurrun wrote:

On Mon, Feb 23, 2015 at 11:38:01AM +0100, Gwenole Beauchesne wrote:

2015-02-23 2:49 GMT+01:00 Himangi Saraogi himangi...@gmail.com:

--- a/libavcodec/vaapi_h264.c
+++ b/libavcodec/vaapi_h264.c
@@ -95,7 +95,7 @@ static int dpb_add(DPB *dpb, H264Picture *pic)
  if (dpb-size = dpb-max_size)
-return -1;
+return AVERROR_INVALIDDATA;

@@ -136,13 +136,13 @@ static int 
fill_vaapi_ReferenceFrames(VAPictureParameterBufferH264 *pic_param,
  for (i = 0; i  h-short_ref_count; i++) {
  H264Picture * const pic = h-short_ref[i];
  if (pic  pic-reference  dpb_add(dpb, pic)  0)
-return -1;
+return AVERROR_INVALIDDATA;
  }

  for (i = 0; i  16; i++) {
  H264Picture * const pic = h-long_ref[i];
  if (pic  pic-reference  dpb_add(dpb, pic)  0)
-return -1;
+return AVERROR_INVALIDDATA;
  }


Those three would be more: internal error, please report this bug to
XXX so that we could see how to fix it. Thanks. AVERROR_BUG?


You mean avpriv_request_sample() and AVERROR_PATCHWELCOME?


Those are for known unimplemented features or untested code paths due to 
lack of samples. I can't tell for sure, but from the context this does 
look more like a case for AVERROR_BUG.


-Justin

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


Re: [libav-devel] [PATCH 1/2] Refactor msmpeg4_decode_dc() to return only an error code

2015-02-21 Thread Justin Ruggles

On 02/20/2015 07:00 PM, Luca Barbato wrote:

On 21/02/15 00:55, Himangi Saraogi wrote:

---
  libavcodec/msmpeg4dec.c | 14 --
  1 file changed, 8 insertions(+), 6 deletions(-)


Level can't be negative so it is ok to return level directly.


Are you sure? It definitely isn't immediately obvious by the code...

-Justin

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


Re: [libav-devel] [PATCH 1/2] mpegvideo: Propagate function returns and return meaningful error codes

2015-02-15 Thread Justin Ruggles

On 02/15/2015 06:40 PM, Himangi Saraogi wrote:

@@ -646,19 +646,19 @@ int ff_msmpeg4_decode_block(MpegEncContext * s, int16_t * 
block,
  if (level  0){
  av_log(s-avctx, AV_LOG_ERROR, dc overflow- block: %d qscale: 
%d//\n, n, s-qscale);
  if(s-inter_intra_pred) level=0;
-elsereturn -1;
+elsereturn level;
  }


This looks like an overloaded return value. It could be overflow for the 
level value or an error code. I would recommend making 
msmpeg4_decode_dc() take a pointer to level as an output param and 
return only an error code.


-Justin

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


Re: [libav-devel] [PATCH] flacenc: Change 'return -1' lines to return proper error codes

2015-02-07 Thread Justin Ruggles

On 02/07/2015 08:15 PM, Timothy Gu wrote:

On Sat Feb 07 2015 at 4:45:55 PM Rohit Kumar Singh rohit91.2...@gmail.com
wrote:

-if (channels  1 || channels  FLAC_MAX_CHANNELS)
-return -1;
+if (channels  1 || channels  FLAC_MAX_CHANNELS) {
+av_log(avctx, AV_LOG_ERROR, invalid number of channels. \


+   Outside valid range of: 1 to %d\n, FLAC_MAX_CHANNELS);




Nope. Also remember to wrap long lines.

av_log(avctx, AV_LOG_ERROR, Invalid number of channels. 
 Only from 1 to %d-channel FLAC files are supported currently.\n,
 FLAC_MAX_CHANNELS);



+return AVERROR_INVALIDDATA;



AVERROR_PATCHWELCOME


Not quite. The FLAC format specification only allows for up to 8 
channels. This is not a limitation of the encoder but of the format itself.


-Justin

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


Re: [libav-devel] [PATCH] tta: fix framepos and start_offset types

2015-02-02 Thread Justin Ruggles

On 02/02/2015 01:21 AM, Vittorio Giovara wrote:

Also propagate errors.

CC: libav-sta...@libav.org
Bug-Id: CID 1238812
---
  libavformat/tta.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavformat/tta.c b/libavformat/tta.c
index e5e6e71..b7efe18 100644
--- a/libavformat/tta.c
+++ b/libavformat/tta.c
@@ -45,12 +45,14 @@ static int tta_read_header(AVFormatContext *s)
  TTAContext *c = s-priv_data;
  AVStream *st;
  int i, channels, bps, samplerate, datalen;
-uint64_t framepos, start_offset;
+int64_t framepos, start_offset;

  if (!av_dict_get(s-metadata, , NULL, AV_DICT_IGNORE_SUFFIX))
  ff_id3v1_read(s);

  start_offset = avio_tell(s-pb);
+if (start_offset  0)
+return start_offset;
  if (avio_rl32(s-pb) != AV_RL32(TTA1))
  return -1; // not tta file

@@ -91,7 +93,10 @@ static int tta_read_header(AVFormatContext *s)
  st-start_time = 0;
  st-duration = datalen;

-framepos = avio_tell(s-pb) + 4*c-totalframes + 4;
+framepos = avio_tell(s-pb);
+if (framepos  0)
+return framepos;
+framepos += 4 * c-totalframes + 4;

  for (i = 0; i  c-totalframes; i++) {
  uint32_t size = avio_rl32(s-pb);



Looks good.

-Justin

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


Re: [libav-devel] [PATCH] isom: add 'mp1v' fourcc

2015-01-06 Thread Justin Ruggles

On 01/06/2015 11:06 AM, Vittorio Giovara wrote:

From: Justin Ruggles justin.rugg...@gmail.com

As referenced in the CoreMedia API docs.

Signed-off-by: Vittorio Giovara vittorio.giov...@gmail.com
---
  libavformat/isom.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/libavformat/isom.c b/libavformat/isom.c
index 5d10dca..5570d26 100644
--- a/libavformat/isom.c
+++ b/libavformat/isom.c
@@ -169,6 +169,7 @@ const AVCodecTag ff_codec_movvideo_tags[] = {
  { AV_CODEC_ID_MPEG1VIDEO, MKTAG('m', '1', 'v', ' ') },
  { AV_CODEC_ID_MPEG1VIDEO, MKTAG('m', '1', 'v', '1') }, /* Apple MPEG-1 
Camcorder */
  { AV_CODEC_ID_MPEG1VIDEO, MKTAG('m', 'p', 'e', 'g') }, /* MPEG */
+{ AV_CODEC_ID_MPEG1VIDEO, MKTAG('m', 'p', '1', 'v') }, /* CoreMedia 
CMVideoCodecType */
  { AV_CODEC_ID_MPEG2VIDEO, MKTAG('m', '2', 'v', '1') }, /* Apple MPEG-2 
Camcorder */
  { AV_CODEC_ID_MPEG2VIDEO, MKTAG('h', 'd', 'v', '1') }, /* MPEG2 HDV 
720p30 */
  { AV_CODEC_ID_MPEG2VIDEO, MKTAG('h', 'd', 'v', '2') }, /* MPEG2 HDV 
1080i60 */



LGTM. Thanks.

-Justin

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


Re: [libav-devel] [PATCH] tiff: Check the check_size return value

2014-12-12 Thread Justin Ruggles

On 12/12/2014 12:49 PM, Vittorio Giovara wrote:

On Fri, Dec 12, 2014 at 12:47 PM, Luca Barbato lu_z...@gentoo.org wrote:

On 10/12/14 02:44, Luca Barbato wrote:


And forward it.

And use the same type for add_entry and check_size.



Ping.


Coverity is happy with this patch, Justin?



Yeah looks ok assuming 'goto fail' won't leak anything.

-Justin

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


Re: [libav-devel] [PATCH] mov: skip version and flags attributes in mov_read_chan()

2014-12-04 Thread Justin Ruggles

On 12/04/2014 02:13 PM, Vittorio Giovara wrote:

From: Matthieu Bouron matthieu.bou...@gmail.com

Fixes decting channel layout for files with uncommon audio, such as
FL and FR in two separate streams. Introduced in 3bab7cd.

CC: libav-devel@libav.org
Sample-Id: ticket1474.mov
Signed-off-by: Vittorio Giovara vittorio.giov...@gmail.com
---
  libavformat/mov.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index cc60ca4..7f288cc 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -647,6 +647,9 @@ static int mov_read_chan(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
  if (atom.size  16)
  return 0;

+/* skip version and flags */
+avio_skip(pb, 4);
+
  ff_mov_read_chan(c-fc, pb, st, atom.size - 4);

  return 0;



Looks correct.

-Justin

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


Re: [libav-devel] [PATCH 03/10] aac: Simplify decode_mid_side_stereo

2014-11-26 Thread Justin Ruggles

On 11/24/2014 12:10 PM, Vittorio Giovara wrote:

On Fri, Nov 21, 2014 at 12:57 PM, Vittorio Giovara
vittorio.giov...@gmail.com wrote:

From: Luca Barbato lu_z...@gentoo.org

Might spare few cycles if the compiler is naive and
makes the function more readable.
---
  libavcodec/aacdec.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavcodec/aacdec.c b/libavcodec/aacdec.c
index d2d51f5..6968102 100644
--- a/libavcodec/aacdec.c
+++ b/libavcodec/aacdec.c
@@ -1419,13 +1419,12 @@ static void decode_mid_side_stereo(ChannelElement *cpe, 
GetBitContext *gb,
 int ms_present)
  {
  int idx;
+int max_idx = cpe-ch[0].ics.num_window_groups * cpe-ch[0].ics.max_sfb;
  if (ms_present == 1) {
-for (idx = 0;
- idx  cpe-ch[0].ics.num_window_groups * cpe-ch[0].ics.max_sfb;
- idx++)
+for (idx = 0; idx  max_idx; idx++)
  cpe-ms_mask[idx] = get_bits1(gb);
  } else if (ms_present == 2) {
-memset(cpe-ms_mask, 1, cpe-ch[0].ics.num_window_groups * 
cpe-ch[0].ics.max_sfb * sizeof(cpe-ms_mask[0]));
+memset(cpe-ms_mask, 1, max_idx * sizeof(cpe-ms_mask[0]));
  }
  }

--
1.9.3 (Apple Git-50)



ping



Looks fine

-Justin

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


Re: [libav-devel] [PATCH] avcodec/vorbis_parser: Move vp check

2014-11-24 Thread Justin Ruggles

On 11/24/2014 10:48 AM, Vittorio Giovara wrote:

From: Michael Niedermayer michae...@gmx.at

Fixes null pointer dereference
Fixes CID1251347

Signed-off-by: Michael Niedermayer michae...@gmx.at
---
  libavcodec/vorbis_parser.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vorbis_parser.c b/libavcodec/vorbis_parser.c
index 0d72fb1..b99f115 100644
--- a/libavcodec/vorbis_parser.c
+++ b/libavcodec/vorbis_parser.c
@@ -330,9 +330,9 @@ static int vorbis_parse(AVCodecParserContext *s1, 
AVCodecContext *avctx,

  if (!s-vp  avctx-extradata  avctx-extradata_size) {
  s-vp = av_vorbis_parse_init(avctx-extradata, avctx-extradata_size);
-if (!s-vp)
-goto end;
  }
+if (!s-vp)
+goto end;

  if ((duration = av_vorbis_parse_frame(s-vp, buf, buf_size)) = 0)
  s1-duration = duration;



LGTM

-Justin

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


Re: [libav-devel] [PATCH] avcodec/lpc: remove unneeded {}

2014-11-24 Thread Justin Ruggles

On 11/24/2014 11:06 AM, Vittorio Giovara wrote:

From: Michael Niedermayer michae...@gmx.at

Signed-off-by: Michael Niedermayer michae...@gmx.at
---
  libavcodec/lpc.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/lpc.h b/libavcodec/lpc.h
index b36b19e..9e0b056 100644
--- a/libavcodec/lpc.h
+++ b/libavcodec/lpc.h
@@ -153,7 +153,7 @@ static inline int compute_lpc_coefs(const LPC_TYPE *autoc, 
int max_order,
  int normalize)
  {
  int i, j;
-LPC_TYPE err = { 0 };
+LPC_TYPE err = 0;
  LPC_TYPE *lpc_last = lpc;

  av_assert2(normalize || !fail);



OK

-Justin

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


Re: [libav-devel] [PATCH 06/10] tiff: Use the same type for add_entry and check_size

2014-11-24 Thread Justin Ruggles

On 11/21/2014 07:57 AM, Vittorio Giovara wrote:

From: Luca Barbato lu_z...@gentoo.org

Bug-Id: CID 700699
CC: libav-sta...@libav.org
---
  libavcodec/tiffenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
index b28f72b..0f9ac80 100644
--- a/libavcodec/tiffenc.c
+++ b/libavcodec/tiffenc.c
@@ -113,7 +113,7 @@ static void tnput(uint8_t **p, int n, const uint8_t *val, 
enum TiffTypes type,
   * @param ptr_val Pointer to values
   */
  static void add_entry(TiffEncoderContext *s, enum TiffTags tag,
-  enum TiffTypes type, int count, const void *ptr_val)
+  enum TiffTypes type, uint64_t count, const void *ptr_val)
  {
  uint8_t *entries_ptr = s-entries + 12 * s-num_entries;




Probably ok, but need to validate that the value is actually within 
uint32 because that is what is written to the bitstream.


-Justin

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


Re: [libav-devel] [PATCH 06/10] tiff: Use the same type for add_entry and check_size

2014-11-24 Thread Justin Ruggles

On 11/24/2014 12:45 PM, Luca Barbato wrote:

On 24/11/14 18:17, Justin Ruggles wrote:

On 11/21/2014 07:57 AM, Vittorio Giovara wrote:

From: Luca Barbato lu_z...@gentoo.org

Bug-Id: CID 700699
CC: libav-sta...@libav.org
---
  libavcodec/tiffenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
index b28f72b..0f9ac80 100644
--- a/libavcodec/tiffenc.c
+++ b/libavcodec/tiffenc.c
@@ -113,7 +113,7 @@ static void tnput(uint8_t **p, int n, const
uint8_t *val, enum TiffTypes type,
   * @param ptr_val Pointer to values
   */
  static void add_entry(TiffEncoderContext *s, enum TiffTags tag,
-  enum TiffTypes type, int count, const void
*ptr_val)
+  enum TiffTypes type, uint64_t count, const void
*ptr_val)
  {
  uint8_t *entries_ptr = s-entries + 12 * s-num_entries;




Probably ok, but need to validate that the value is actually within
uint32 because that is what is written to the bitstream.



check_size should do that already.


Yes, but that is called after writing the value. Plus the return value 
is not checked; it just sets the buffer to the end, which is also not 
checked when writing. So changing count to uint64_t doesn't really do 
much of anything to help the actual issue.


-Justin

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


Re: [libav-devel] [PATCH 05/10] flacenc: avoid integer overflow

2014-11-14 Thread Justin Ruggles

On 11/14/2014 02:31 PM, Luca Barbato wrote:

On 12/11/14 19:10, Vittorio Giovara wrote:

CC: libav-sta...@libav.org
Bug-Id: CID 743441
---
  libavcodec/flacenc.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/flacenc.c b/libavcodec/flacenc.c
index 1160da2..d5f7b35 100644
--- a/libavcodec/flacenc.c
+++ b/libavcodec/flacenc.c
@@ -663,7 +663,8 @@ static uint64_t find_subframe_rice_params(FlacEncodeContext 
*s,
  int pmax = get_max_p_order(s-options.max_partition_order,
 s-frame.blocksize, pred_order);

-uint64_t bits = 8 + pred_order * sub-obits + 2 + sub-rc.coding_mode;
+uint64_t bits = 8 + (uint64_t) pred_order * sub-obits +
+2 + sub-rc.coding_mode;
  if (sub-type == FLAC_SUBFRAME_LPC)
  bits += 4 + 5 + pred_order * s-options.lpc_coeff_precision;
  bits += calc_rice_params(sub-rc, pmin, pmax, sub-residual,



could we check the range of those variables


+1. I don't think there is any way for that to overflow.

-Justin


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


Re: [libav-devel] [PATCH 05/10] flacenc: avoid integer overflow

2014-11-14 Thread Justin Ruggles

On 11/14/2014 02:47 PM, Luca Barbato wrote:

On 12/11/14 19:10, Vittorio Giovara wrote:

CC: libav-sta...@libav.org
Bug-Id: CID 743441
---
  libavcodec/flacenc.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/flacenc.c b/libavcodec/flacenc.c
index 1160da2..d5f7b35 100644
--- a/libavcodec/flacenc.c
+++ b/libavcodec/flacenc.c
@@ -663,7 +663,8 @@ static uint64_t find_subframe_rice_params(FlacEncodeContext 
*s,
  int pmax = get_max_p_order(s-options.max_partition_order,
 s-frame.blocksize, pred_order);

-uint64_t bits = 8 + pred_order * sub-obits + 2 + sub-rc.coding_mode;
+uint64_t bits = 8 + (uint64_t) pred_order * sub-obits +
+2 + sub-rc.coding_mode;
  if (sub-type == FLAC_SUBFRAME_LPC)
  bits += 4 + 5 + pred_order * s-options.lpc_coeff_precision;
  bits += calc_rice_params(sub-rc, pmin, pmax, sub-residual,




pred_order range is 0-32 (from options.min/max_prediction_order)

obits is bits_per_raw_sample or something around it.

coding_mode is 4 or 5

why bits is uint64_t ?


Because it's a counter for all bits in the frame.

-Justin


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


Re: [libav-devel] [PATCH 02/10] libopusenc: prevent an out-of-bounds read by returning early

2014-11-11 Thread Justin Ruggles

On 11/11/2014 10:43 AM, Vittorio Giovara wrote:

On Tue, Nov 11, 2014 at 3:56 PM, Luca Barbato lu_z...@gentoo.org wrote:

On 11/11/14 13:26, Vittorio Giovara wrote:

CC: libav-sta...@libav.org
Bug-Id: CID 1244188
---
  libavcodec/libopusenc.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
index 8447206..9103677 100644
--- a/libavcodec/libopusenc.c
+++ b/libavcodec/libopusenc.c
@@ -163,10 +163,11 @@ static int av_cold libopus_encode_init(AVCodecContext 
*avctx)

  /* FIXME: Opus can handle up to 255 channels. However, the mapping for
   * anything greater than 8 is undefined. */
-if (avctx-channels  8)
-av_log(avctx, AV_LOG_WARNING,
+if (avctx-channels  8) {
+av_log(avctx, AV_LOG_ERROR,
 Channel layout undefined for %d channels.\n, 
avctx-channels);
-
+return AVERROR_INVALIDDATA;
+}
  if (!avctx-bit_rate) {
  /* Sane default copied from opusenc */
  avctx-bit_rate = 64000 * opus-stream_count +



return ENOSYS or PATCHWELCOME please.


locally amended

-return AVERROR_INVALIDDATA;
+return AVERROR(ENOSYS);

ok?


PATCHWELCOME is the better choice.

-Justin


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


Re: [libav-devel] [PATCH 09/13] tiffenc: initialize return value

2014-11-09 Thread Justin Ruggles

On 11/09/2014 02:48 AM, Vittorio Giovara wrote:

---
  libavcodec/tiffenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
index f794961..cde7c12 100644
--- a/libavcodec/tiffenc.c
+++ b/libavcodec/tiffenc.c
@@ -214,7 +214,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
*pkt,
  int bytes_per_row;
  uint32_t res[2]= { 72, 1 }; // image resolution (72/1)
  uint16_t bpp_tab[] = { 8, 8, 8, 8 };
-int ret;
+int ret = 0;
  int is_yuv = 0;
  uint8_t *yuv_line = NULL;
  int shift_h, shift_v;



'ret' can only be used without initialization if s-height = 0, which 
can only happen if avctx-height = 0, which is validated elsewhere, so 
I can see why this tripped up coverity. Doesn't hurt to still initialize 
it though.


-Justin

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


Re: [libav-devel] [PATCH 3/7] lavf: stop using avpriv_flac_parse_streaminfo()

2014-10-30 Thread Justin Ruggles

On 10/30/2014 03:00 AM, Anton Khirnov wrote:

The only parameters needed by the demuxers are the sample rate and sample
count, which can be trivially extracted manually, without resorting to
an avpriv function.
---
  libavcodec/Makefile|  4 +---
  libavformat/flacdec.c  | 19 +++
  libavformat/oggparseflac.c | 11 +++
  3 files changed, 19 insertions(+), 15 deletions(-)


I'm ok with it.

-Justin


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


Re: [libav-devel] [PATCH 4/7] lavc: make avpriv_flac_parse_streaminfo() private on the next bump

2014-10-30 Thread Justin Ruggles

On 10/30/2014 03:00 AM, Anton Khirnov wrote:

---
  libavcodec/flac.c| 10 +-
  libavcodec/flac.h|  5 +
  libavcodec/flacdec.c |  4 ++--
  3 files changed, 16 insertions(+), 3 deletions(-)


Ok

-Justin


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


Re: [libav-devel] [PATCH 2/7] riffenc: do not fall back on AVCodecContext.frame_size for MP3

2014-10-30 Thread Justin Ruggles

On 10/30/2014 03:00 AM, Anton Khirnov wrote:

It will not be set unless the codec context is used as the encoding
context, which is discouraged. For MP2, av_get_audio_frame_duration()
will already set the frame size properly. For MP3, set the frame size
explicitly.
---
  libavformat/riffenc.c | 10 +++---
  1 file changed, 3 insertions(+), 7 deletions(-)



Probably ok

-Justin


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


Re: [libav-devel] [PATCH 6/7] lavc: make avpriv_flac_is_extradata_valid() private on the next bump

2014-10-30 Thread Justin Ruggles

On 10/30/2014 03:01 AM, Anton Khirnov wrote:

---
  libavcodec/flac.c| 9 -
  libavcodec/flac.h| 9 ++---
  libavcodec/flacdec.c | 2 +-
  3 files changed, 15 insertions(+), 5 deletions(-)


Ok

-Justin


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


Re: [libav-devel] [PATCH 5/7] oggenc: accept only STREAMINFO extradata

2014-10-30 Thread Justin Ruggles

On 10/30/2014 03:01 AM, Anton Khirnov wrote:

The reasoning is the same as for
0097cbea695e534fce39958ccd103af2fbf65831.
---
  libavformat/oggenc.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)



Ok

-Justin


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


Re: [libav-devel] [PATCH 7/7] nutdec: do not set has_b_frames

2014-10-30 Thread Justin Ruggles

On 10/30/2014 03:01 AM, Anton Khirnov wrote:

It is not supposed to be set by demuxers.
---
  libavformat/nutdec.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index c3f5f4b..69057e9 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -399,7 +399,6 @@ static int decode_stream_header(NUTContext *nut)
  GET_V(stc-msb_pts_shift, tmp  16);
  stc-max_pts_distance = ffio_read_varlen(bc);
  GET_V(stc-decode_delay, tmp  1000); // sanity limit, raise this if 
Moore's law is true
-st-codec-has_b_frames = stc-decode_delay;
  ffio_read_varlen(bc); // stream flags

  GET_V(st-codec-extradata_size, tmp  (1  30));



Anything this would theoretically break? Not saying that line should 
stay there, but it would be good to know if something needs to be 
handled elsewhere to avoid breaking stuff.


-Justin

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


Re: [libav-devel] [PATCH 7/7] nutdec: do not set has_b_frames

2014-10-30 Thread Justin Ruggles

On 10/30/2014 10:32 AM, Anton Khirnov wrote:

Quoting Justin Ruggles (2014-10-30 15:24:45)

On 10/30/2014 03:01 AM, Anton Khirnov wrote:

It is not supposed to be set by demuxers.
---
   libavformat/nutdec.c | 1 -
   1 file changed, 1 deletion(-)

diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
index c3f5f4b..69057e9 100644
--- a/libavformat/nutdec.c
+++ b/libavformat/nutdec.c
@@ -399,7 +399,6 @@ static int decode_stream_header(NUTContext *nut)
   GET_V(stc-msb_pts_shift, tmp  16);
   stc-max_pts_distance = ffio_read_varlen(bc);
   GET_V(stc-decode_delay, tmp  1000); // sanity limit, raise this if 
Moore's law is true
-st-codec-has_b_frames = stc-decode_delay;
   ffio_read_varlen(bc); // stream flags

   GET_V(st-codec-extradata_size, tmp  (1  30));



Anything this would theoretically break? Not saying that line should
stay there, but it would be good to know if something needs to be
handled elsewhere to avoid breaking stuff.



In general, it could affect the way lavf makes up timestamps.
But specifically NUT stores both pts and dts, so this should not be a
problem.



Ok, fine with me then.

-Justin

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


Re: [libav-devel] [PATCH 5/9] avresample: Move the resample_out_buffer below

2014-10-17 Thread Justin Ruggles

On 10/17/2014 07:49 AM, Anton Khirnov wrote:

Quoting Vittorio Giovara (2014-10-15 18:32:56)

From: Luca Barbato lu_z...@gentoo.org

Spare a branch and make coverity less confused.

CC: libav-sta...@libav.org
Bug-Id: CID 73
---
  libavresample/utils.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)



Yet again -- I'm not a fan of randomly shuffling code around just to
make some specific tool shut up. There is a huge number of such tools
and each one complains about different perfectly valid code. We cannot
appease them all and we should not try.



I am also not in favor of this patch.

-Justin

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


Re: [libav-devel] [PATCH 1/2] libfdk-aacdec: Enable Decoder Downmix including Downmix Metadata Support

2014-10-17 Thread Justin Ruggles

On 10/17/2014 02:32 AM, Martin Storsjö wrote:

I'm not sure if it's kosher to request e.g. a 6 channel AVFrame and then
just flip it to 2 channels after allocation - if it is, then we could of
course simplify that, but that is unrelated to this patch, this is what
we do already.


It's maybe mostly safe with interleaved sample formats, but I think it's 
best to avoid such unexpected, undocumented behavior. Definitely safer 
to free and reallocate buffers for the proper channel layout.


-Justin

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


Re: [libav-devel] [PATCH 6/9] avresample: Make sure the even check does not overflow

2014-10-15 Thread Justin Ruggles

On 10/15/2014 12:32 PM, Vittorio Giovara wrote:

From: Luca Barbato lu_z...@gentoo.org

CC: libav-sta...@libav.org
Bug-Id: CID 732225
---
  libavresample/audio_mix_matrix.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavresample/audio_mix_matrix.c b/libavresample/audio_mix_matrix.c
index 487869b..5182ae1 100644
--- a/libavresample/audio_mix_matrix.c
+++ b/libavresample/audio_mix_matrix.c
@@ -60,7 +60,7 @@

  static av_always_inline int even(uint64_t layout)
  {
-return (!layout || (layout  (layout - 1)));
+return (!layout || !!(layout  (layout - 1)));
  }

  static int sane_layout(uint64_t layout)



How is overflow possible?

-Justin

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


Re: [libav-devel] [PATCH 9/9] avresample: prevent theoretical division by zero

2014-10-15 Thread Justin Ruggles

On 10/15/2014 12:33 PM, Vittorio Giovara wrote:

Make coverity less confused.

CC: libav-sta...@libav.org
Bug-Id: CID 1231986
---
  libavresample/utils.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavresample/utils.c b/libavresample/utils.c
index c5d30d7..0546c10 100644
--- a/libavresample/utils.c
+++ b/libavresample/utils.c
@@ -583,9 +583,12 @@ static inline int convert_frame(AVAudioResampleContext 
*avr,

  static inline int available_samples(AVFrame *out)
  {
+int samples;
  int bytes_per_sample = av_get_bytes_per_sample(out-format);
-int samples = out-linesize[0] / bytes_per_sample;
+if (!bytes_per_sample)
+return AVERROR_INVALIDDATA;


AVERROR(EINVAL)



+samples = out-linesize[0] / bytes_per_sample;
  if (av_sample_fmt_is_planar(out-format)) {
  return samples;
  } else {



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


Re: [libav-devel] [PATCH] resample: Avoid off-by-1 errors in PTS calcs.

2014-10-14 Thread Justin Ruggles

On 10/13/2014 09:01 PM, Timothy B. Terriberry wrote:

I also notice that the flag AVFMT_TS_NEGATIVE is set for all muxer
classes in libavformat/oggenc.c, but I don't believe this is actually
true for any of them (the granule position value -1 is reserved as
invalid, but granule positions are otherwise treated as unsigned,
leaving no way to represent a negative timestamp). I didn't change
anything there, however, as I don't really understand all the
implications, and I don't want to paper over other problems like this one.


This is definitely needed for the ogg muxer, specifically for Speex and 
Vorbis. There is special-case handling for Opus that offsets pts by the 
encoder delay when setting granulepos. If we want to be extra-safe and 
avoid odd corner cases like timestamps being less than -delay, that 
could be handled within the ogg muxer for Opus.


-Justin

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


Re: [libav-devel] [PATCH 1/2] Enable Decoder Downmix including Downmix Metadata Support

2014-09-29 Thread Justin Ruggles

On 09/29/2014 10:24 AM, Omer Osman wrote:

Am 25.09.2014 17:54, schrieb Justin Ruggles:

Right. My point is that it should not check for the number of
channels. The switch should be done on the requested layout itself and
should only support AV_CH_LAYOUT_MONO and AV_CH_LAYOUT_STEREO.


OK, will check for AV_CH_LAYOUT_STEREO, _STEREO_DOWNMIX, and _MONO.



Also, does aacDecoder_GetStreamInfo() report the channel layout info
for the downmixed output or the full coded channel layout? I don't
know the API well enough...


Yes, for example:

CStreamInfo::numChannels = 6
CStreamInfo::pChannelType = { ::ACT_FRONT, ::ACT_FRONT, ::ACT_FRONT,
::ACT_LFE, ::ACT_BACK, ::ACT_BACK }
CStreamInfo::pChannelIndices = { 1, 2, 0, 0, 0, 1 }

Why is this needed if the downmixed output is maximum stereo? On the
other hand, this can simplify the code in
libfdk-accdec.c::get_stream_info (). Should this be implemented instead?



It is needed because get_stream_info() must set AVCodecContext.channels 
and AVCodecContext.channel_layout to the downmixed values when 
appropriate. Alternatively, the downmixed channel_layout can be set for 
the AVFrame prior to ff_get_buffer(), but the former method is preferred 
due to the interaction between libavcodec and libavfilter.


-Justin

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


Re: [libav-devel] [PATCH 1/2] Enable Decoder Downmix including Downmix Metadata Support

2014-09-25 Thread Justin Ruggles

On 09/24/2014 11:22 AM, Omer Osman wrote:

Am 24.09.2014 17:07, schrieb Justin Ruggles:

On 09/24/2014 10:48 AM, Omer Osman wrote:

+switch (downmix_channels){
+  case 2:
+  case 1:
+  if(aacDecoder_SetParam(s-handle, 
AAC_PCM_OUTPUT_CHANNELS, downmix_channels) != AAC_DEC_OK){
+  av_log(avctx, AV_LOG_ERROR, Unable to set output 
channels in the decoder\n);

+  return AVERROR_UNKNOWN;
+  }
+  break;


If FDK-AAC actually downmixes to mono and stereo, those channel 
layouts should be checked, not the channel count.


 int downmix_channels = 
av_get_channel_layout_nb_channels(avctx-request_channel_layout); 


Therefore, the number of channels from the channel layout is being 
checked.




Right. My point is that it should not check for the number of channels. 
The switch should be done on the requested layout itself and should only 
support AV_CH_LAYOUT_MONO and AV_CH_LAYOUT_STEREO.


Also, does aacDecoder_GetStreamInfo() report the channel layout info for 
the downmixed output or the full coded channel layout? I don't know the 
API well enough...





+  default:
+  av_log(avctx, AV_LOG_ERROR, Invalid 
request_channel_layout\n);

+  return AVERROR_UNKNOWN;
+}


It's actually fine for the decoder to ignore the requested layout and 
not fail.

OK by me, however, I think it makes sense to at least throw a warning.


Sure, logging a warning message is fine.

-Justin

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


Re: [libav-devel] [PATCH 1/2] ac3enc: allow Dolby Pro Logic II as a preferred downmix mode.

2014-09-25 Thread Justin Ruggles

On 09/25/2014 12:48 PM, Tim Walker wrote:

Some encoders already use this value even
though it's reserved in the A/52 specification.
---
  libavcodec/ac3enc.h   | 1 +
  libavcodec/ac3enc_opts_template.c | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)



Fine with me.

-Justin

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


Re: [libav-devel] [PATCH 2/2] ac3enc: allow Dolby Pro Logic IIz as the Dolby Surround EX mode.

2014-09-25 Thread Justin Ruggles

On 09/25/2014 12:48 PM, Tim Walker wrote:

This is actually defined in the A/52 specification.
---
  libavcodec/ac3enc.h   | 1 +
  libavcodec/ac3enc_opts_template.c | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)



LGTM

-Justin


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


Re: [libav-devel] [PATCH 1/2] Enable Decoder Downmix including Downmix Metadata Support

2014-09-24 Thread Justin Ruggles

On 09/24/2014 10:48 AM, Omer Osman wrote:

+switch (downmix_channels){
+  case 2:
+  case 1:
+  if(aacDecoder_SetParam(s-handle, AAC_PCM_OUTPUT_CHANNELS, 
downmix_channels) != AAC_DEC_OK){
+  av_log(avctx, AV_LOG_ERROR, Unable to set output channels in the 
decoder\n);
+  return AVERROR_UNKNOWN;
+  }
+  break;


If FDK-AAC actually downmixes to mono and stereo, those channel layouts 
should be checked, not the channel count.



+  default:
+  av_log(avctx, AV_LOG_ERROR, Invalid request_channel_layout\n);
+  return AVERROR_UNKNOWN;
+}


It's actually fine for the decoder to ignore the requested layout and 
not fail.



+frame-nb_samples = avctx-frame_size;
+if ((ret = ff_get_buffer(avctx, frame, 0))  0)
+goto end;
+memcpy(frame-extended_data[0], s-decoder_buffer,
+   avctx-channels * avctx-frame_size *
+   av_get_bytes_per_sample(avctx-sample_fmt));


I'm not thrilled about making the memcpy() unconditional. Can this be 
avoided when not downmixing?


-Justin

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


Re: [libav-devel] [PATCH 9/9] Prefer https links over http where sensibly possible

2014-09-10 Thread Justin Ruggles

On 09/10/2014 03:49 PM, Diego Biurrun wrote:

  p Our master branch should be always in an almost stable condition, we
  provide a quite large regression testing framework and we monitor its
-a href=http://fate.libav.org;
-   title=FATE Automated Testing Environmentreports/a
+a href=//fate.libav.org title=FATE Automated Testing 
Environmentreports/a
  to make sure everything is working as expected on a wide variety of systems.
-We advise to check a href=http://fate.libav.org;  title=FATE Automated Testing 
EnvironmentFATE/a before distributing a snapshot or run it yourself./p
-p If you are not represented yet in a href=http://fate.libav.org;  title=FATE Automated 
Testing EnvironmentFATE/a and you have the resources to provide
+We advise to check a href=//fate.libav.org title=FATE Automated Testing 
EnvironmentFATE/a before distributing a snapshot or run it yourself./p
+p If you are not represented yet in a href=//fate.libav.org title=FATE Automated 
Testing EnvironmentFATE/a and you have the resources to provide
  reports please contact us. /p


href=//fate.libav.org

I don't think that works...

-Justin

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


Re: [libav-devel] [PATCH 1/3] alacenc: increase predictor buffer

2014-08-31 Thread Justin Ruggles

On 08/18/2014 11:12 AM, Christophe Gisquet wrote:

This change is almost cosmetical only, and reduces the changes needed to
fix the 24bps case.
---
  libavcodec/alacenc.c | 19 ++-
  1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/libavcodec/alacenc.c b/libavcodec/alacenc.c
index 401f26f..c8354d2 100644
--- a/libavcodec/alacenc.c
+++ b/libavcodec/alacenc.c
@@ -66,7 +66,7 @@ typedef struct AlacEncodeContext {
  int write_sample_size;
  int extra_bits;
  int32_t sample_buf[2][DEFAULT_FRAME_SIZE];
-int32_t predictor_buf[DEFAULT_FRAME_SIZE];
+int32_t predictor_buf[2][DEFAULT_FRAME_SIZE];
  int interlacing_shift;
  int interlacing_leftweight;
  PutBitContext pbctx;
@@ -253,13 +253,14 @@ static void alac_linear_predictor(AlacEncodeContext *s, 
int ch)
  {
  int i;
  AlacLPCContext lpc = s-lpc[ch];
+int32_t *residual = s-predictor_buf[ch];
  
  if (lpc.lpc_order == 31) {

-s-predictor_buf[0] = s-sample_buf[ch][0];
+residual[0] = s-sample_buf[ch][0];
  
  for (i = 1; i  s-frame_size; i++) {

-s-predictor_buf[i] = s-sample_buf[ch][i] -
-  s-sample_buf[ch][i - 1];
+residual[i] = s-sample_buf[ch][i] -
+  s-sample_buf[ch][i - 1];
  }
  
  return;

@@ -269,7 +270,6 @@ static void alac_linear_predictor(AlacEncodeContext *s, int 
ch)
  
  if (lpc.lpc_order  0) {

  int32_t *samples  = s-sample_buf[ch];
-int32_t *residual = s-predictor_buf;
  
  // generate warm-up samples

  residual[0] = samples[0];
@@ -313,11 +313,11 @@ static void alac_linear_predictor(AlacEncodeContext *s, 
int ch)
  }
  }
  
-static void alac_entropy_coder(AlacEncodeContext *s)

+static void alac_entropy_coder(AlacEncodeContext *s, int ch)
  {
  unsigned int history = s-rc.initial_history;
  int sign_modifier = 0, i, k;
-int32_t *samples = s-predictor_buf;
+int32_t *samples = s-predictor_buf[ch];
  
  for (i = 0; i  s-frame_size;) {

  int x;
@@ -432,10 +432,11 @@ static void write_element(AlacEncodeContext *s,
  // TODO: determine when this will actually help. for now it's not 
used.
  if (prediction_type == 15) {
  // 2nd pass 1st order filter
+int32_t *residual = s-predictor_buf[channels];


That seems wrong. shouldn't that be s-predictor_buf[i]


  for (j = s-frame_size - 1; j  0; j--)
-s-predictor_buf[j] -= s-predictor_buf[j - 1];
+residual[j] -= residual[j - 1];
  }
-alac_entropy_coder(s);
+alac_entropy_coder(s, i);
  }
  }
  }


Everything else seems fine.

-Justin

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


Re: [libav-devel] [PATCH 2/3] alacenc: fix extra bits extraction

2014-08-31 Thread Justin Ruggles

On 08/18/2014 11:12 AM, Christophe Gisquet wrote:

The raw coded bits are extracted prior to decorrelation, as is correctly
performed by the decoder, and not after.

Fixes ticket #2768.
---
  libavcodec/alacenc.c | 16 ++--
  1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/libavcodec/alacenc.c b/libavcodec/alacenc.c
index c8354d2..e62c015 100644
--- a/libavcodec/alacenc.c
+++ b/libavcodec/alacenc.c
@@ -394,6 +394,19 @@ static void write_element(AlacEncodeContext *s,
  init_sample_buffers(s, channels, samples);
  write_element_header(s, element, instance);
  
+// extract extra bits if needed

+if (s-extra_bits) {
+uint32_t mask = (1  s-extra_bits) - 1;
+for (j = 0; j  channels; j++) {
+int32_t *extra = s-predictor_buf[j];
+int32_t *smp   = s-sample_buf[j];
+for (i = 0; i  s-frame_size; i++) {
+extra[i] = smp[i]  mask;
+smp[i] = s-extra_bits;
+}
+}
+}
+
  if (channels == 2)
  alac_stereo_decorrelation(s);
  else
@@ -419,8 +432,7 @@ static void write_element(AlacEncodeContext *s,
  uint32_t mask = (1  s-extra_bits) - 1;
  for (i = 0; i  s-frame_size; i++) {
  for (j = 0; j  channels; j++) {
-put_bits(pb, s-extra_bits, s-sample_buf[j][i]  mask);
-s-sample_buf[j][i] = s-extra_bits;
+put_bits(pb, s-extra_bits, s-predictor_buf[j][i]  mask);
  }
  }
  }


Masking the value when writing should be unnecessary since it is already 
masked out above.


Thanks,
Justin

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


Re: [libav-devel] [PATCH 7/7] wmv2: cosmetics: Fix coding style inconsistencies

2014-08-30 Thread Justin Ruggles

On 08/29/2014 11:39 PM, Gabriel Dume wrote:

---
  libavcodec/wmv2.c|  95 +++--
  libavcodec/wmv2.h|   6 +-
  libavcodec/wmv2dec.c | 394 ++-
  libavcodec/wmv2enc.c | 114 ---
  4 files changed, 319 insertions(+), 290 deletions(-)

diff --git a/libavcodec/wmv2.c b/libavcodec/wmv2.c
index 7b1ea57..19ddc9a 100644
--- a/libavcodec/wmv2.c
+++ b/libavcodec/wmv2.c
@@ -26,8 +26,9 @@
  #include wmv2.h
  
  
-av_cold void ff_wmv2_common_init(Wmv2Context * w){

-MpegEncContext * const s= w-s;
+av_cold void ff_wmv2_common_init(Wmv2Context *w)
+{
+MpegEncContext *const s = w-s;
  
  ff_blockdsp_init(s-bdsp, s-avctx);

  ff_wmv2dsp_init(w-wdsp);
@@ -51,11 +52,13 @@ av_cold void ff_wmv2_common_init(Wmv2Context * w){
  s-idsp.idct = NULL;
  }
  
-static void wmv2_add_block(Wmv2Context *w, int16_t *block1, uint8_t *dst, int stride, int n){

-MpegEncContext * const s= w-s;
+static void wmv2_add_block(Wmv2Context *w, int16_t *block1,
+   uint8_t *dst, int stride, int n)
+{
+MpegEncContext *const s = w-s;
  
if (s-block_last_index[n] = 0) {

-switch(w-abt_type_table[n]){
+switch(w-abt_type_table[n]) {

Space between switch and (

Also would be nice to fix all the indentation to use 4 spaces.


  case 0:
  w-wdsp.idct_add(dst, stride, block1);
  break;
@@ -75,68 +78,70 @@ static void wmv2_add_block(Wmv2Context *w, int16_t *block1, 
uint8_t *dst, int st
}
  }
  
-void ff_wmv2_add_mb(MpegEncContext *s, int16_t block1[6][64], uint8_t *dest_y, uint8_t *dest_cb, uint8_t *dest_cr){

-Wmv2Context * const w= (Wmv2Context*)s;
+void ff_wmv2_add_mb(MpegEncContext *s, int16_t block1[6][64],
+uint8_t *dest_y, uint8_t *dest_cb, uint8_t *dest_cr)
+{
+Wmv2Context *const w = (Wmv2Context*) s;
  
-wmv2_add_block(w, block1[0], dest_y, s-linesize, 0);

-wmv2_add_block(w, block1[1], dest_y + 8, s-linesize, 1);
-wmv2_add_block(w, block1[2], dest_y + 8*s-linesize, s-linesize, 2);
-wmv2_add_block(w, block1[3], dest_y + 8 + 8*s-linesize, s-linesize, 3);
+wmv2_add_block(w, block1[0], dest_y  , s-linesize, 0);
+wmv2_add_block(w, block1[1], dest_y + 8  , s-linesize, 1);
+wmv2_add_block(w, block1[2], dest_y + 8 * s-linesize, s-linesize, 2);
+wmv2_add_block(w, block1[3], dest_y + 8 + 8 * s-linesize, s-linesize, 3);
  
-if(s-flagsCODEC_FLAG_GRAY) return;

+if (s-flagsCODEC_FLAG_GRAY)
+return;
  
-wmv2_add_block(w, block1[4], dest_cb   , s-uvlinesize, 4);

-wmv2_add_block(w, block1[5], dest_cr   , s-uvlinesize, 5);
+wmv2_add_block(w, block1[4], dest_cb , s-uvlinesize, 
4);
+wmv2_add_block(w, block1[5], dest_cr , s-uvlinesize, 
5);
  }
  
-void ff_mspel_motion(MpegEncContext *s,

-   uint8_t *dest_y, uint8_t *dest_cb, uint8_t 
*dest_cr,
-   uint8_t **ref_picture, op_pixels_func 
(*pix_op)[4],
-   int motion_x, int motion_y, int h)
+void ff_mspel_motion(MpegEncContext *s, uint8_t *dest_y,
+ uint8_t *dest_cb, uint8_t *dest_cr,
+ uint8_t **ref_picture, op_pixels_func (*pix_op) [4],

No space before [4] is slightly better


+ int motion_x, int motion_y, int h)
  {
-Wmv2Context * const w= (Wmv2Context*)s;
+Wmv2Context *const w = (Wmv2Context*) s;
  uint8_t *ptr;
  int dxy, offset, mx, my, src_x, src_y, v_edge_pos;
  ptrdiff_t linesize, uvlinesize;
-int emu=0;
+int emu = 0;
  
-dxy = ((motion_y  1)  1) | (motion_x  1);

-dxy = 2*dxy + w-hshift;
+dxy   = ((motion_y  1)  1) | (motion_x  1);
+dxy   = 2 * dxy + w-hshift;
  src_x = s-mb_x * 16 + (motion_x  1);
  src_y = s-mb_y * 16 + (motion_y  1);
  
  /* WARNING: do no forget half pels */

  v_edge_pos = s-v_edge_pos;
-src_x = av_clip(src_x, -16, s-width);
-src_y = av_clip(src_y, -16, s-height);
+src_x  = av_clip(src_x, -16, s-width);
+src_y  = av_clip(src_y, -16, s-height);
  
-if(src_x=-16 || src_x = s-width)

+if (src_x = -16 || src_x = s-width)
  dxy = ~3;
-if(src_y=-16 || src_y = s-height)
+if (src_y = -16 || src_y = s-height)
  dxy = ~4;
  
  linesize   = s-linesize;

  uvlinesize = s-uvlinesize;
-ptr = ref_picture[0] + (src_y * linesize) + src_x;
-
-if(src_x1 || src_y1 || src_x + 17  = s-h_edge_pos
-  || src_y + h+1 = v_edge_pos){
-s-vdsp.emulated_edge_mc(s-edge_emu_buffer,
- ptr - 1 - s-linesize,
- s-linesize, s-linesize,
- 19, 19,
+ptr= ref_picture[0] + (src_y * linesize) + src_x;
+
+if 

Re: [libav-devel] [PATCH] ogg: check malloc calls

2014-08-24 Thread Justin Ruggles

On 08/24/2014 12:12 AM, Nidhi Makhijani wrote:

---
  libavformat/oggparsespeex.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/libavformat/oggparsespeex.c b/libavformat/oggparsespeex.c
index b2779e7..43d687a 100644
--- a/libavformat/oggparsespeex.c
+++ b/libavformat/oggparsespeex.c
@@ -47,6 +47,8 @@ static int speex_header(AVFormatContext *s, int idx) {
  
  if (!spxp) {

  spxp = av_mallocz(sizeof(*spxp));
+if (!spxp)
+return AVERROR(ENOMEM);
  os-private = spxp;
  }
  
@@ -75,6 +77,10 @@ static int speex_header(AVFormatContext *s, int idx) {

  st-codec-extradata_size = os-psize;
  st-codec-extradata = av_malloc(st-codec-extradata_size
   + FF_INPUT_BUFFER_PADDING_SIZE);
+if (!st-codec-extradata) {
+st-codec-extradata_size = 0;
+return AVERROR(ENOMEM);
+}
  memcpy(st-codec-extradata, p, st-codec-extradata_size);
  
  avpriv_set_pts_info(st, 64, 1, st-codec-sample_rate);


The patch itself looks good. Unfortunately the ogg demuxer is not 
currently checking the return value of the header() function for error, 
so that needs to be handled as well.


-Justin

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


Re: [libav-devel] [PATCH] ogg: Provide aliases for Speex, Opus and audio-only ogg

2014-08-20 Thread Justin Ruggles

On 08/17/2014 02:56 PM, Luca Barbato wrote:

---
  configure|  5 +
  libavformat/Makefile |  4 ++--
  libavformat/allformats.c |  3 +++
  libavformat/oggenc.c | 53 +++-
  4 files changed, 62 insertions(+), 3 deletions(-)



Looks good

-Justin

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


Re: [libav-devel] [PATCH] frame: Remove some FF_API_AVFRAME_COLORSPACE leftovers

2014-08-13 Thread Justin Ruggles

On 08/13/2014 03:48 PM, Diego Biurrun wrote:

---

Anton, I'm charging you a Guinness ...

  libavutil/frame.c | 7 ---
  1 file changed, 7 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 4b154f3..765823b 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -385,13 +385,6 @@ int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
  dst-coded_picture_number   = src-coded_picture_number;
  dst-display_picture_number = src-display_picture_number;
  dst-flags  = src-flags;
-#if FF_API_AVFRAME_COLORSPACE
-dst-color_primaries= src-color_primaries;
-dst-color_trc  = src-color_trc;
-dst-colorspace = src-colorspace;
-dst-color_range= src-color_range;
-dst-chroma_location= src-chroma_location;
-#endif
  
  memcpy(dst-error, src-error, sizeof(dst-error));
  


Wait what? That doesn't look right.

-Justin

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


Re: [libav-devel] [PATCH] tiff: only use PAL8 if palette is present, else use GRAY8 for pixfmt.

2014-08-09 Thread Justin Ruggles

On 08/09/2014 09:19 AM, Diego Elio Pettenò wrote:

Instead of simulating a grayscale palette, use real grayscale pixels, if no
palette is actually defined.

Signed-off-by: Diego Elio Pettenò flamee...@flameeyes.eu
---
  libavcodec/tiff.c | 14 +++---
  1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 2aff45a..ca5ec75 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -246,15 +246,14 @@ static int tiff_unpack_strip(TiffContext *s, uint8_t 
*dst, int stride,
  
  static int init_image(TiffContext *s, AVFrame *frame)

  {
-int i, ret;
-uint32_t *pal;
+int ret;
  
  switch (s-bpp * 10 + s-bppcount) {

  case 11:
  s-avctx-pix_fmt = AV_PIX_FMT_MONOBLACK;
  break;
  case 81:
-s-avctx-pix_fmt = AV_PIX_FMT_PAL8;
+s-avctx-pix_fmt = s-palette_is_set ? AV_PIX_FMT_PAL8 : 
AV_PIX_FMT_GRAY8;
  break;
  case 243:
  s-avctx-pix_fmt = AV_PIX_FMT_RGB24;
@@ -290,14 +289,7 @@ static int init_image(TiffContext *s, AVFrame *frame)
  return ret;
  }
  if (s-avctx-pix_fmt == AV_PIX_FMT_PAL8) {
-if (s-palette_is_set) {
-memcpy(frame-data[1], s-palette, sizeof(s-palette));
-} else {
-/* make default grayscale pal */
-pal = (uint32_t *) frame-data[1];
-for (i = 0; i  256; i++)
-pal[i] = i * 0x010101;
-}
+memcpy(frame-data[1], s-palette, sizeof(s-palette));
  }
  return 0;
  }


This patch is definitely better than the current code, but there are a 
few situations that still aren't handled properly.


We should only use the palette in 2 situations, neither of which is 
checked for.


1. s-photometric == TIFF_PHOTOMETRIC_PALETTE
This is the normal TIFF6 palette type. If this type is set and 
there is no palette, I'm ok with outputting gray8, but there should at 
least be a warning. If the palette is set for other types (and component 
count doesn't match the type), I think we should either return an error 
or assume situation #2.


2. The Indexed tag (346) is set to 1
This is from an addendum by Adobe and allows colorspaces other than 
RGB in the palette. To actually support this though, we would have to 
change how the palette is read, as it can have different numbers of 
components and may require conversion to RGB. (See 
http://partners.adobe.com/public/developer/en/tiff/TIFFPM6.pdf)


Anyway, the patch is fine as-is, I just wanted to document what still 
needs to be done to improve the palette handling.


Thanks,
Justin

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

Re: [libav-devel] [PATCH 2/2] avresample: Introduce AVFrame-based API

2014-08-09 Thread Justin Ruggles

On 08/02/2014 10:56 AM, Luca Barbato wrote:

---
  doc/APIchanges |   4 ++
  libavresample/avresample.h |  71 
  libavresample/utils.c  | 134 +
  libavresample/version.h|   2 +-
  libavutil/error.h  |   2 +
  libavutil/version.h|   2 +-
  6 files changed, 213 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 261993b..6bf267b 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,10 @@ libavutil: 2013-12-xx
  
  API changes, most recent first:
  
+2014-04-xx - xxx - lavu 53.20.0 - error.h

+2014-04-xx - xxx - lavr 1.4.0 - avresample.h
+  Add avresample_convert_frame() and avresample_config().
+
  2014-07-xx - xxx - lavu 53.19.0 - avstring.h
Make name matching function from lavf public as av_match_name().
  
diff --git a/libavresample/avresample.h b/libavresample/avresample.h

index 6105759..165285c 100644
--- a/libavresample/avresample.h
+++ b/libavresample/avresample.h
@@ -95,6 +95,7 @@
  #include libavutil/avutil.h
  #include libavutil/channel_layout.h
  #include libavutil/dict.h
+#include libavutil/frame.h
  #include libavutil/log.h
  #include libavutil/mathematics.h
  
@@ -165,6 +166,10 @@ AVAudioResampleContext *avresample_alloc_context(void);
  
  /**

   * Initialize AVAudioResampleContext.
+ * @note The context must be configured using the AVOption api.


API


+ *
+ * @see av_opt_set_int()
+ * @see av_opt_set_dict()
   *
   * @param avr  audio resample context
   * @return 0 on success, negative AVERROR code on failure
@@ -423,6 +428,72 @@ int avresample_available(AVAudioResampleContext *avr);
  int avresample_read(AVAudioResampleContext *avr, uint8_t **output, int 
nb_samples);
  
  /**

+ * Convert the samples in the input AVFrame and write them to the output 
AVFrame.
+ *
+ * Input and output AVFrames must have channel_layout, sample_rate and format 
set.
+ *
+ * The upper bound on the number of output samples is obtained through
+ * avresample_get_out_samples().
+ *
+ * If the output AVFrame does not have the data pointers allocated a the 
nb_samples


a the


+ * will be set as described above and av_frame_get_buffer() will be called.
+ *
+ * The output AVFrame can be NULL or have fewer allocated samples than 
required.
+ *
+ * In this case, any remaining samples not written to the output will be added
+ * to an internal FIFO buffer, to be returned at the next call to this function
+ * or to avresample_convert() or to avresample_read().
+ *
+ * If converting sample rate, there may be data remaining in the internal
+ * resampling delay buffer. avresample_get_delay() tells the number of
+ * remaining samples. To get this data as output, call this function or
+ * avresample_convert() with NULL input.
+ *
+ * At the end of the conversion process, there may be data remaining in the
+ * internal FIFO buffer. avresample_available() tells the number of remaining
+ * samples. To get this data as output, either call this function or
+ * avresample_convert() with NULL input or call avresample_read().
+ *
+ * If the AVAudioResampleContext configuration does not match the output and
+ * input AVFrame settings the conversion does not take place and depending on
+ * which AVFrame is not matching AVERROR_OUTPUT_CHANGED, AVERROR_INPUT_CHANGED
+ * or AVERROR_OUTPUT_CHANGED|AVERROR_INPUT_CHANGED is returned.
+ *
+ * @see avresample_get_out_samples()
+ * @see avresample_available()
+ * @see avresample_convert()
+ * @see avresample_read()
+ * @see avresample_get_delay()
+ *
+ * @param avr audio resample context
+ * @param output  output AVFrame
+ * @param input   input AVFrame
+ * @return0 on success, AVERROR on failure or nonmatching
+ *configuration.
+ */
+int avresample_convert_frame(AVAudioResampleContext *avr,
+ AVFrame *output, AVFrame *input);
+
+/**
+ * Configure or reconfigure the AVAudioResampleContext using the information
+ * provided by the AVFrames and an optional AVDictionary containing additional
+ * resampler options.
+ *
+ * The original resampling context is reset even on failure.
+ * The function calls internally avresample_open().


calls avresample_open() internally


+ *
+ * @see avresample_open();
+ *
+ * @param avr audio resample context
+ * @param output  output AVFrame
+ * @param input   input AVFrame
+ * @param optsadditional options
+ * @return0 on success, AVERROR on failure.
+ */
+int avresample_config(AVAudioResampleContext *avr, AVFrame *out, AVFrame *in,
+  AVDictionary **opts);
+
+/**
   * @}
   */
  
diff --git a/libavresample/utils.c b/libavresample/utils.c

index 851cd35..1df3c48 100644
--- a/libavresample/utils.c
+++ b/libavresample/utils.c
@@ -21,6 +21,7 @@
  #include libavutil/common.h
  #include libavutil/dict.h
  #include 

Re: [libav-devel] [PATCH 2/2] avresample: Introduce AVFrame-based API

2014-08-09 Thread Justin Ruggles

On 08/09/2014 10:36 AM, wm4 wrote:

On Sat, 09 Aug 2014 10:26:16 -0400
Justin Ruggles justin.rugg...@gmail.com wrote:


On 08/02/2014 10:56 AM, Luca Barbato wrote:

---
+int avresample_config(AVAudioResampleContext *avr, AVFrame *out, AVFrame *in,
+  AVDictionary **opts)
+{
+int ret;
+
+if (avresample_is_open(avr)) {
+avresample_close(avr);
+}
+
+if (in) {
+avr-in_channel_layout  = in-channel_layout;
+avr-in_sample_rate = in-sample_rate;
+avr-in_sample_fmt  = in-format;
+}
+
+if (out) {
+avr-out_channel_layout = out-channel_layout;
+avr-out_sample_rate= out-sample_rate;
+avr-out_sample_fmt = out-format;
+}
+
+if (opts) {
+if ((ret = av_opt_set_dict(avr, opts))  0)
+return ret;
+}

Since input/output params can also be set through AVOption, it may be
beneficial to check for consistency.


I'm a bit worried about this part of API design in general: what if
features are added to AVFrame that overlap with what can be configured
with AVOption? On one hand, it'd be nice if configuration is as
automatic as possible, and settings are derived from AVFrame parameters.
On the other hand you'd have to detect whether an AVOption was set (or
set to a conflicting value), which all leads to chaos.


It doesn't necessarily lead to chaos. We could, for example, document 
clearly that the AVOptions take precedence over AVFrame values. In some 
cases we can print warnings for conflicts 
(channel_layout/sample_rate/sample_fmt), and in some other future cases 
it might be better not to.


-Justin

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


Re: [libav-devel] [PATCH] af_resample: Set the number of samples in the last frame

2014-08-09 Thread Justin Ruggles

On 08/09/2014 04:37 PM, Luca Barbato wrote:

On 02/08/14 01:58, Luca Barbato wrote:

Otherwise trailing zeroes would appear.
---

This patch requires to update the reference files.
Shall I upload a v3?

  libavfilter/af_resample.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/libavfilter/af_resample.c b/libavfilter/af_resample.c
index bc8fd8a..ce4273a 100644
--- a/libavfilter/af_resample.c
+++ b/libavfilter/af_resample.c
@@ -197,6 +197,7 @@ static int request_frame(AVFilterLink *outlink)
  return (ret == 0) ? AVERROR_EOF : ret;
  }

+frame-nb_samples = ret;
  frame-pts = s-next_pts;
  return ff_filter_frame(outlink, frame);
  }
--
1.9.0

Ping.


LGTM

-Justin

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


Re: [libav-devel] [PATCH 2/2] avresample: Introduce AVFrame-based API

2014-08-08 Thread Justin Ruggles

On 08/02/2014 10:56 AM, Luca Barbato wrote:

+2014-04-xx - xxx - lavu 53.20.0 - error.h
+2014-04-xx - xxx - lavr 1.4.0 - avresample.h
+  Add avresample_convert_frame() and avresample_config().


Mention what has been changed in lavu.


+ * The output AVFrame can be NULL or have fewer allocated samples than 
required.
+ *
+ * In this case, any remaining samples not written to the output will be added
+ * to an internal FIFO buffer, to be returned at the next call to this function
+ * or to avresample_convert() or to avresample_read().


Merge these 2 paragraphs.

I'll review the rest tomorrow.

-Justin

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


Re: [libav-devel] [PATCH] alac: use AV_WB instead of custom versions

2014-08-07 Thread Justin Ruggles

On 08/07/2014 02:00 PM, Nidhi Makhijani wrote:

---
  libavcodec/alacenc.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/alacenc.c b/libavcodec/alacenc.c
index 401f26f..19a640e 100644
--- a/libavcodec/alacenc.c
+++ b/libavcodec/alacenc.c
@@ -528,15 +528,15 @@ static av_cold int alac_encode_init(AVCodecContext *avctx)
  avctx-extradata_size = ALAC_EXTRADATA_SIZE;
  
  alac_extradata = avctx-extradata;

-AV_WB32(alac_extradata,ALAC_EXTRADATA_SIZE);
-AV_WB32(alac_extradata+4,  MKBETAG('a','l','a','c'));
-AV_WB32(alac_extradata+12, avctx-frame_size);
-AV_WB8 (alac_extradata+17, avctx-bits_per_raw_sample);
-AV_WB8 (alac_extradata+21, avctx-channels);
-AV_WB32(alac_extradata+24, s-max_coded_frame_size);
-AV_WB32(alac_extradata+28,
+AV_WB (32, alac_extradata,ALAC_EXTRADATA_SIZE);
+AV_WB (32, alac_extradata+4,  MKBETAG('a','l','a','c'));
+AV_WB (32, alac_extradata+12, avctx-frame_size);
+AV_WB8(alac_extradata+17, avctx-bits_per_raw_sample);
+AV_WB8(alac_extradata+21, avctx-channels);
+AV_WB (32, alac_extradata+24, s-max_coded_frame_size);
+AV_WB (32, alac_extradata+28,
  avctx-sample_rate * avctx-channels * 
avctx-bits_per_raw_sample); // average bitrate
-AV_WB32(alac_extradata+32, avctx-sample_rate);
+AV_WB (32, alac_extradata+32, avctx-sample_rate);
  
  // Set relevant extradata fields

  if (s-compression_level  0) {


Why?

-Justin

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


Re: [libav-devel] [PATCH] [9] cmdutil: Add image size check to codec_get_buffer()

2014-08-03 Thread Justin Ruggles

On 08/03/2014 11:32 AM, Diego Biurrun wrote:

From: Michael Niedermayer michae...@gmx.at

Bug-Id: CVE-2011-3935

Found-by: Mateusz j00ru Jurczyk and Gynvael Coldwind
Signed-off-by: Michael Niedermayer michae...@gmx.at
Signed-off-by: Diego Biurrun di...@biurrun.de
---

Applies to the 9 branch; again, no sample.

  cmdutils.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/cmdutils.c b/cmdutils.c
index b65326b..f072572 100644
--- a/cmdutils.c
+++ b/cmdutils.c
@@ -1598,6 +1598,9 @@ int codec_get_buffer(AVCodecContext *s, AVFrame *frame)
  FrameBuffer *buf;
  int ret, i;
  
+if (av_image_check_size(s-width, s-height, 0, s))

+return AVERROR_INVALIDDATA;
+
  if (!*pool  (ret = alloc_buffer(pool, s, pool))  0)
  return ret;
  


This seems like the lazy way out of making sure decoders validate width 
and height if they change them after init, but I suppose it doesn't hurt 
anything.


-Justin

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


Re: [libav-devel] [PATCH] af_join: Set the output frame format

2014-08-01 Thread Justin Ruggles

On 08/01/2014 04:27 PM, Luca Barbato wrote:

---
  libavfilter/af_join.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/libavfilter/af_join.c b/libavfilter/af_join.c
index ee87340..e684cb9 100644
--- a/libavfilter/af_join.c
+++ b/libavfilter/af_join.c
@@ -480,6 +480,7 @@ static int join_request_frame(AVFilterLink *outlink)
  frame-nb_samples = nb_samples;
  frame-channel_layout = outlink-channel_layout;
  frame-sample_rate= outlink-sample_rate;
+frame-format = outlink-format;
  frame-pts= s-input_frames[0]-pts;
  frame-linesize[0]= linesize;
  if (frame-data != frame-extended_data) {


LGTM

-Justin

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


Re: [libav-devel] [PATCH] af_channelmap: Set the frame channel layout

2014-08-01 Thread Justin Ruggles

On 08/01/2014 05:48 PM, Luca Barbato wrote:

Otherwise the frame would show the first layout matching the
channel count.
---
  libavfilter/af_channelmap.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
index 3e5cc3d..3035405 100644
--- a/libavfilter/af_channelmap.c
+++ b/libavfilter/af_channelmap.c
@@ -342,6 +342,8 @@ static int channelmap_filter_frame(AVFilterLink *inlink, 
AVFrame *buf)
  memcpy(buf-data, buf-extended_data,
 FFMIN(FF_ARRAY_ELEMS(buf-data), nch_out) * sizeof(buf-data[0]));
  
+buf-channel_layout = outlink-channel_layout;

+
  return ff_filter_frame(outlink, buf);
  }
  


LGTM

-Justin

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


Re: [libav-devel] Travis

2014-07-20 Thread Justin Ruggles

On 07/19/2014 01:22 PM, Reinhard Tartler wrote:

On Sat, Jul 19, 2014 at 12:26 PM, Diego Biurrun di...@biurrun.de wrote:

On Sat, Jul 19, 2014 at 12:23:58PM -0400, Reinhard Tartler wrote:

For release branches, we currently don't have any FATE Farm. It would
be great to have something like that, but as far as I understand, it
is a lot of effort to setup FATE for release branches, which is really
a shame.

Huh?  It's just a matter of changing a Git URL..

I wasn't aware that it's that easy, please go ahead and change fate to
also cover the release branches.



We also need to be careful about removing or changing existing tests in 
ways that will break release branches. An alternative would be to copy 
all of the reference samples to a separate dir when a release branch is 
made and use that instead.


-Justin

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


Re: [libav-devel] Travis

2014-07-20 Thread Justin Ruggles

On 07/20/2014 01:58 PM, Reinhard Tartler wrote:

On Sun, Jul 20, 2014 at 1:50 PM, Justin Ruggles
justin.rugg...@gmail.com wrote:

On 07/19/2014 01:22 PM, Reinhard Tartler wrote:

On Sat, Jul 19, 2014 at 12:26 PM, Diego Biurrun di...@biurrun.de wrote:

On Sat, Jul 19, 2014 at 12:23:58PM -0400, Reinhard Tartler wrote:

For release branches, we currently don't have any FATE Farm. It would
be great to have something like that, but as far as I understand, it
is a lot of effort to setup FATE for release branches, which is really
a shame.

Huh?  It's just a matter of changing a Git URL..

I wasn't aware that it's that easy, please go ahead and change fate to
also cover the release branches.


We also need to be careful about removing or changing existing tests in ways
that will break release branches. An alternative would be to copy all of the
reference samples to a separate dir when a release branch is made and use
that instead.

That's what we've been doing since release/0.7:


rsync -vaLW rsync://fate-suite.libav.org

samples r/o access to Libav's samples collection
fate-suite r/o access to Libav's fate-suite
fate-suite-10   r/o access to Libav's fate-suite for Libav 10
fate-suite-9   r/o access to Libav's fate-suite for Libav 9
fate-suite-0.8 r/o access to Libav's fate-suite for Libav 0.8
fate-suite-0.7 r/o access to Libav's fate-suite for Libav 0.7

We also adjust the target fate-rsync in tests/Makefile for release branches.





Oh, nice. :)

-Justin

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


Re: [libav-devel] [PATCH] adpcmenc: allow to force a certain blocksize when encoding, ADPCM

2014-07-09 Thread Justin Ruggles

On 07/09/2014 10:26 AM, Vladimir Pantelic wrote:

$subject



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


Fine with me as long as FATE still passes

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


Re: [libav-devel] [PATCH 3/8] oggparsecelt: do not set AVCodecContext.frame_size

2014-07-05 Thread Justin Ruggles

On 07/05/2014 05:51 AM, Martin Storsjö wrote:

On Sat, 5 Jul 2014, Anton Khirnov wrote:


It is supposed to be set by decoders only.
---
libavformat/oggparsecelt.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)


Sure, if you say so. Justin perhaps can confirm?



Yeah that is true. What is oggparsecelt used for anyway?

-Justin

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


[libav-devel] [PATCH 2/2] Check mp3 header before calling avpriv_mpegaudio_decode_header().

2014-06-22 Thread Justin Ruggles
As indicated in the function documentation, the header MUST be
checked prior to calling it because no consistency check is done
there.

CC:libav-sta...@libav.org
---
 libavcodec/libmp3lame.c |8 +++-
 libavformat/mp3enc.c|   17 ++---
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
index eebc65c..dee1909 100644
--- a/libavcodec/libmp3lame.c
+++ b/libavcodec/libmp3lame.c
@@ -182,6 +182,7 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, 
AVPacket *avpkt,
 MPADecodeHeader hdr;
 int len, ret, ch;
 int lame_result;
+uint32_t h;
 
 if (frame) {
 switch (avctx-sample_fmt) {
@@ -237,7 +238,12 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, 
AVPacket *avpkt,
determine the frame size. */
 if (s-buffer_index  4)
 return 0;
-if (avpriv_mpegaudio_decode_header(hdr, AV_RB32(s-buffer))) {
+h = AV_RB32(s-buffer);
+if (ff_mpa_check_header(h)  0) {
+av_log(avctx, AV_LOG_ERROR, Invalid mp3 header at start of buffer\n);
+return AVERROR_BUG;
+}
+if (avpriv_mpegaudio_decode_header(hdr, h)) {
 av_log(avctx, AV_LOG_ERROR, free format output not supported\n);
 return -1;
 }
diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index 9326258..476d7f7 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -252,13 +252,16 @@ static int mp3_write_audio_packet(AVFormatContext *s, 
AVPacket *pkt)
 
 if (mp3-xing_offset  pkt-size = 4) {
 MPADecodeHeader c;
-
-avpriv_mpegaudio_decode_header(c, AV_RB32(pkt-data));
-
-if (!mp3-initial_bitrate)
-mp3-initial_bitrate = c.bit_rate;
-if ((c.bit_rate == 0) || (mp3-initial_bitrate != c.bit_rate))
-mp3-has_variable_bitrate = 1;
+uint32_t h;
+
+h = AV_RB32(pkt-data);
+if (ff_mpa_check_header(h) == 0) {
+avpriv_mpegaudio_decode_header(c, h);
+if (!mp3-initial_bitrate)
+mp3-initial_bitrate = c.bit_rate;
+if ((c.bit_rate == 0) || (mp3-initial_bitrate != c.bit_rate))
+mp3-has_variable_bitrate = 1;
+}
 
 mp3_xing_add_frame(mp3, pkt);
 }
-- 
1.7.1

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


[libav-devel] [PATCH 1/2] Check if an mp3 header is using a reserved sample rate.

2014-06-22 Thread Justin Ruggles
Fixes an invalid read past the end of avpriv_mpa_freq_tab.
Fixes divide-by-zero due to sample_rate being set to 0.

Fixes Bug 705

CC:libav-sta...@libav.org
---
 libavcodec/mpegaudiodecheader.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/libavcodec/mpegaudiodecheader.c b/libavcodec/mpegaudiodecheader.c
index 69dda45..25e7319 100644
--- a/libavcodec/mpegaudiodecheader.c
+++ b/libavcodec/mpegaudiodecheader.c
@@ -24,6 +24,8 @@
  * MPEG Audio header decoder.
  */
 
+#include libavutil/common.h
+
 #include avcodec.h
 #include mpegaudio.h
 #include mpegaudiodata.h
@@ -45,6 +47,8 @@ int avpriv_mpegaudio_decode_header(MPADecodeHeader *s, 
uint32_t header)
 s-layer = 4 - ((header  17)  3);
 /* extract frequency */
 sample_rate_index = (header  10)  3;
+if (sample_rate_index = FF_ARRAY_ELEMS(avpriv_mpa_freq_tab))
+sample_rate_index = 0;
 sample_rate = avpriv_mpa_freq_tab[sample_rate_index]  (s-lsf + mpeg25);
 sample_rate_index += 3 * (s-lsf + mpeg25);
 s-sample_rate_index = sample_rate_index;
-- 
1.7.1

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


[libav-devel] [PATCH] Add av_image_check_sar() and use it to validate SAR

2014-06-19 Thread Justin Ruggles
---
 doc/APIchanges   |  3 +++
 libavcodec/dirac.c   |  2 ++
 libavcodec/dpx.c |  2 ++
 libavcodec/dvdec.c   | 17 +
 libavcodec/exr.c |  4 ++--
 libavcodec/ffv1dec.c |  8 
 libavcodec/h263dec.c |  2 ++
 libavcodec/h264_slice.c  |  3 +--
 libavcodec/hevc.c|  3 ++-
 libavcodec/internal.h|  6 ++
 libavcodec/mjpegdec.c|  1 +
 libavcodec/mpeg12dec.c   |  2 ++
 libavcodec/truemotion1.c |  2 ++
 libavcodec/utils.c   | 33 +
 libavcodec/vc1.c |  1 +
 libavcodec/vp3.c |  1 +
 libavutil/imgutils.c | 23 +++
 libavutil/imgutils.h | 15 +++
 libavutil/version.h  |  2 +-
 19 files changed, 116 insertions(+), 14 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 51a2ff5..1b32d0b 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@ libavutil: 2013-12-xx
 
 API changes, most recent first:
 
+2014-06-xx - xxx - lavu 53.17.0 - imgutils.h
+  Add av_image_check_sar().
+
 2014-xx-xx - xxx - lavf 55.20.0 - avformat.h
   The proper way for providing a hint about the desired timebase to the muxers
   is now setting AVStream.time_base, instead of AVStream.codec.time_base as was
diff --git a/libavcodec/dirac.c b/libavcodec/dirac.c
index f0fb85d..ed0ea9f 100644
--- a/libavcodec/dirac.c
+++ b/libavcodec/dirac.c
@@ -316,6 +316,8 @@ int avpriv_dirac_parse_sequence_header(AVCodecContext 
*avctx, GetBitContext *gb,
 if (ret  0)
 return ret;
 
+ff_set_sar(avctx, avctx-sample_aspect_ratio);
+
 /* [DIRAC_STD] picture_coding_mode shall be 0 for fields and 1 for frames
  * currently only used to signal field coding */
 picture_coding_mode = svq3_get_ue_golomb(gb);
diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c
index 0dfa538..c796387 100644
--- a/libavcodec/dpx.c
+++ b/libavcodec/dpx.c
@@ -150,6 +150,8 @@ static int decode_frame(AVCodecContext *avctx,
 if ((ret = ff_set_dimensions(avctx, w, h))  0)
 return ret;
 
+ff_set_sar(avctx, avctx-sample_aspect_ratio);
+
 if ((ret = ff_get_buffer(avctx, p, 0))  0) {
 av_log(avctx, AV_LOG_ERROR, get_buffer() failed\n);
 return ret;
diff --git a/libavcodec/dvdec.c b/libavcodec/dvdec.c
index ef9ba4c..77f8473 100644
--- a/libavcodec/dvdec.c
+++ b/libavcodec/dvdec.c
@@ -36,6 +36,7 @@
  */
 
 #include libavutil/internal.h
+#include libavutil/imgutils.h
 #include libavutil/pixdesc.h
 #include avcodec.h
 #include internal.h
@@ -337,6 +338,14 @@ static int dvvideo_decode_frame(AVCodecContext *avctx,
 if (ret  0)
 return ret;
 
+/* Determine the codec's sample_aspect ratio from the packet */
+vsc_pack = buf + 80*5 + 48 + 5;
+if ( *vsc_pack == dv_video_control ) {
+apt = buf[4]  0x07;
+is16_9 = (vsc_pack  ((vsc_pack[2]  0x07) == 0x02 || (!apt  
(vsc_pack[2]  0x07) == 0x07)));
+ff_set_sar(avctx, s-sys-sar[is16_9]);
+}
+
 if (ff_get_buffer(avctx, s-frame, 0)  0) {
 av_log(avctx, AV_LOG_ERROR, get_buffer() failed\n);
 return -1;
@@ -353,14 +362,6 @@ static int dvvideo_decode_frame(AVCodecContext *avctx,
 /* return image */
 *got_frame = 1;
 
-/* Determine the codec's sample_aspect ratio from the packet */
-vsc_pack = buf + 80*5 + 48 + 5;
-if ( *vsc_pack == dv_video_control ) {
-apt = buf[4]  0x07;
-is16_9 = (vsc_pack  ((vsc_pack[2]  0x07) == 0x02 || (!apt  
(vsc_pack[2]  0x07) == 0x07)));
-avctx-sample_aspect_ratio = s-sys-sar[is16_9];
-}
-
 return s-sys-frame_size;
 }
 
diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 9f60cc6..37a31ce 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1107,8 +1107,8 @@ static int decode_header(EXRContext *s)
 if (!var_size)
 return AVERROR_INVALIDDATA;
 
-s-avctx-sample_aspect_ratio =
-av_d2q(av_int2float(bytestream2_get_le32(s-gb)), 255);
+ff_set_sar(s-avctx,
+   av_d2q(av_int2float(bytestream2_get_le32(s-gb)), 
255));
 
 continue;
 } else if ((var_size = check_header_variable(s, compression,
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 8f7b2bf..703491e 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -328,6 +328,14 @@ static int decode_slice_header(FFV1Context *f, FFV1Context 
*fs)
 f-cur-sample_aspect_ratio.num = get_symbol(c, state, 0);
 f-cur-sample_aspect_ratio.den = get_symbol(c, state, 0);
 
+if (av_image_check_sar(f-width, f-height,
+   f-cur-sample_aspect_ratio)  0) {
+av_log(f-avctx, AV_LOG_WARNING, ignoring invalid SAR: %u/%u\n,
+   f-cur-sample_aspect_ratio.num,
+   f-cur-sample_aspect_ratio.den);
+f-cur-sample_aspect_ratio = (AVRational){ 0, 1 };
+}
+
 return 0;
 }
 
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 

Re: [libav-devel] [PATCH 1/2] dv: get rid of global non-const tables

2014-06-18 Thread Justin Ruggles

On 06/18/2014 01:03 PM, Anton Khirnov wrote:

Instead, store them in the context and compute on each parameter change.
---
Now eliminating idct_factor globals as well
---
  libavcodec/dv.c |   14 +-
  libavcodec/dv.h |9 -
  libavcodec/dv_profile.c |   34 --
  libavcodec/dv_profile.h |6 --
  libavcodec/dvdec.c  |   20 +++-
  libavcodec/dvenc.c  |4 ++--
  6 files changed, 30 insertions(+), 57 deletions(-)



LGTM

-Justin

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


Re: [libav-devel] [PATCH 2/2] dv: cosmetics, reindent

2014-06-18 Thread Justin Ruggles

On 06/18/2014 01:03 PM, Anton Khirnov wrote:

---
  libavcodec/dv.c |   74 +++
  1 file changed, 37 insertions(+), 37 deletions(-)



OK

-Justin

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


Re: [libav-devel] [PATCH] avconv: use the correct variable in comparison

2014-06-14 Thread Justin Ruggles
On 06/14/2014 03:46 PM, Anton Khirnov wrote:
 ---
  avconv.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/avconv.c b/avconv.c
 index 0fac5c6..b29f200 100644
 --- a/avconv.c
 +++ b/avconv.c
 @@ -2104,7 +2104,7 @@ static int transcode_init(void)
  if (out_codec) {
  encoder_name   = out_codec-name;
  out_codec_name = avcodec_descriptor_get(out_codec-id)-name;
 -if (!strcmp(encoder_name, in_codec_name))
 +if (!strcmp(encoder_name, out_codec_name))
  encoder_name = native;
  }
  

OK

-Justin

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


Re: [libav-devel] [PATCH] cutils: check malloc calls

2014-06-14 Thread Justin Ruggles
On 06/14/2014 03:38 PM, Nidhi Makhijani wrote:
 ---
 this will fail silently. what should be done to inform the user of a memory 
 allocation failure? 

I think the best thing would be to return 0 or AVERROR(ENOMEM) and
handle it in all the places dynarray_add is used. Unfortunately I think
the macro might make things difficult, but not impossible.

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


Re: [libav-devel] [PATCH] adpcm: Fix trellis encoding of IMA QT

2014-06-06 Thread Justin Ruggles

On 06/05/2014 04:56 AM, Martin Storsjö wrote:

This was broken in 095be4fb - samples+ch (for the previous
non-planar case) equals samples_p[ch][0]. The confusion
probably stemmed from the IMA WAV case where it originally
was samples[avctx-channels + ch], which was correctly
changed into samples_p[ch][1].

CC: libav-sta...@libav.org
---
  libavcodec/adpcmenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/adpcmenc.c b/libavcodec/adpcmenc.c
index fb3ce0d..2cf8d6f 100644
--- a/libavcodec/adpcmenc.c
+++ b/libavcodec/adpcmenc.c
@@ -549,7 +549,7 @@ static int adpcm_encode_frame(AVCodecContext *avctx, 
AVPacket *avpkt,
  put_bits(pb, 7,  status-step_index);
  if (avctx-trellis  0) {
  uint8_t buf[64];
-adpcm_compress_trellis(avctx, samples_p[ch][1], buf, status,
+adpcm_compress_trellis(avctx, samples_p[ch][0], buf, status,
 64, 1);
  for (i = 0; i  64; i++)
  put_bits(pb, 4, buf[i ^ 1]);


Thanks. Sorry for breaking it.

-Justin

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


Re: [libav-devel] [PATCH 103/132] dsputil: Split vector operations off into a separate context

2014-06-06 Thread Justin Ruggles

On 06/02/2014 12:55 PM, Diego Biurrun wrote:

On Mon, Jun 02, 2014 at 09:12:23AM -0400, Justin Ruggles wrote:

On 06/02/2014 08:53 AM, Diego Biurrun wrote:

On Mon, Jun 02, 2014 at 08:30:42AM -0400, Justin Ruggles wrote:

On 06/02/2014 07:34 AM, Diego Biurrun wrote:

-void ff_vector_clipf_neon(float *dst, const float *src, float min, float max,
-  int len);

Might be a better fit in floatdsp?

Hmm, maybe - do you refer to just vector_clipf or all the functions
I'm splitting off?

just that one. none of the others use floats...

I'm skeptical, the vector_clip* code is very similar, scattering it over
two places does not seem right to me.



I disagree. But whatever. I won't argue. It's more important to port it 
to use yasm.


-Justin

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


Re: [libav-devel] [PATCH] libfdk-aac: Relicense the library wrappers to the ISC license

2014-06-04 Thread Justin Ruggles

On 06/04/2014 12:23 PM, Martin Storsjö wrote:

This reduces the number of different licenses used within libav,
and is preferrable since it has less ambiguous wordings than
the BSD license with respect to the duties of the user of the code.

Fraunhofer have now indicated that they're allowed to contribute
code under this license as well.
---
The actual contributions they are meaning to make still haven't
showed up, but they keep promising that they'll come, but they
have other things delaying them right now.
---
  libavcodec/libfdk-aacdec.c | 32 ++--
  libavcodec/libfdk-aacenc.c | 32 ++--
  2 files changed, 20 insertions(+), 44 deletions(-)



Ok

-Justin

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


Re: [libav-devel] [PATCH 103/132] dsputil: Split vector operations off into a separate context

2014-06-02 Thread Justin Ruggles
On 06/02/2014 07:34 AM, Diego Biurrun wrote:
 -void ff_vector_clipf_neon(float *dst, const float *src, float min, float max,
 -  int len);

Might be a better fit in floatdsp?

-Justin

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


Re: [libav-devel] [PATCH 103/132] dsputil: Split vector operations off into a separate context

2014-06-02 Thread Justin Ruggles
On 06/02/2014 08:53 AM, Diego Biurrun wrote:
 On Mon, Jun 02, 2014 at 08:30:42AM -0400, Justin Ruggles wrote:
 On 06/02/2014 07:34 AM, Diego Biurrun wrote:
 -void ff_vector_clipf_neon(float *dst, const float *src, float min, float 
 max,
 -  int len);

 Might be a better fit in floatdsp?
 
 Hmm, maybe - do you refer to just vector_clipf or all the functions
 I'm splitting off?

just that one. none of the others use floats...

-Justin

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


Re: [libav-devel] [PATCH 1/2] lavu: add all color-related enums to AVFrame

2014-05-31 Thread Justin Ruggles
On 05/30/2014 04:14 PM, wm4 wrote:
 +#ifndef FF_API_AVFRAME_COLORSPACE
 +#define FF_API_AVFRAME_COLORSPACE   (LIBAVUTIL_VERSION_MAJOR = 54)
 +#endif

Wait, so this won't work until the next libavutil major bump?

-Justin

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


Re: [libav-devel] [PATCH 1/2] lavu: add all color-related enums to AVFrame

2014-05-31 Thread Justin Ruggles
On 05/31/2014 02:05 PM, wm4 wrote:
 On Sat, 31 May 2014 13:41:43 -0400
 Justin Ruggles justin.rugg...@gmail.com wrote:
 
 On 05/30/2014 04:14 PM, wm4 wrote:
 +#ifndef FF_API_AVFRAME_COLORSPACE
 +#define FF_API_AVFRAME_COLORSPACE   (LIBAVUTIL_VERSION_MAJOR = 54)
 +#endif

 Wait, so this won't work until the next libavutil major bump?
 
 Exactly. Since libavcodec depends on sizeof(AVFrame (h264 decoder),
 this change can't be done without changing ABI. And apparently
 bumping the ABI requires a major bump.

Then what is the point in bumping minor and adding it to APIchanges if
it is essentially dead code until major bump?

-Justin

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


Re: [libav-devel] [PATCH 1/2] lavu: add all color-related enums to AVFrame

2014-05-31 Thread Justin Ruggles
On 05/31/2014 04:38 PM, Anton Khirnov wrote:
 
 On Sat, 31 May 2014 14:38:43 -0400, Justin Ruggles justin.rugg...@gmail.com 
 wrote:
 On 05/31/2014 02:05 PM, wm4 wrote:
 On Sat, 31 May 2014 13:41:43 -0400
 Justin Ruggles justin.rugg...@gmail.com wrote:

 On 05/30/2014 04:14 PM, wm4 wrote:
 +#ifndef FF_API_AVFRAME_COLORSPACE
 +#define FF_API_AVFRAME_COLORSPACE   (LIBAVUTIL_VERSION_MAJOR = 54)
 +#endif

 Wait, so this won't work until the next libavutil major bump?

 Exactly. Since libavcodec depends on sizeof(AVFrame (h264 decoder),
 this change can't be done without changing ABI. And apparently
 bumping the ABI requires a major bump.

 Then what is the point in bumping minor and adding it to APIchanges if
 it is essentially dead code until major bump?

 
 The move of the enums happens immediately

Then a note should at least be made in the APIchanges entry that the
added fields to AVFrame will not be part of the API until lavu 54.

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


[libav-devel] [PATCH] Add av_image_check_sar() and use it to validate SAR

2014-05-30 Thread Justin Ruggles
---
 doc/APIchanges   |  3 +++
 libavcodec/dirac.c   |  2 ++
 libavcodec/dpx.c |  2 ++
 libavcodec/dvdec.c   | 17 +
 libavcodec/exr.c |  4 ++--
 libavcodec/ffv1dec.c |  8 
 libavcodec/h263dec.c |  2 ++
 libavcodec/h264_slice.c  |  3 +--
 libavcodec/hevc.c|  3 ++-
 libavcodec/internal.h|  6 ++
 libavcodec/mjpegdec.c|  1 +
 libavcodec/mpeg12dec.c   |  2 ++
 libavcodec/truemotion1.c |  2 ++
 libavcodec/utils.c   | 33 +
 libavcodec/vc1.c |  1 +
 libavcodec/vp3.c |  1 +
 libavutil/imgutils.c | 23 +++
 libavutil/imgutils.h | 15 +++
 libavutil/version.h  |  2 +-
 19 files changed, 116 insertions(+), 14 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 279cf66..070393e 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@ libavutil: 2013-12-xx
 
 API changes, most recent first:
 
+2014-05-xx - xxx - lavu 53.16.0 - imgutils.h
+  Add av_image_check_sar().
+
 2014-04-xx - xxx - lavr 1.3.0 - avresample.h
   Add avresample_max_output_samples
 
diff --git a/libavcodec/dirac.c b/libavcodec/dirac.c
index f0fb85d..ed0ea9f 100644
--- a/libavcodec/dirac.c
+++ b/libavcodec/dirac.c
@@ -316,6 +316,8 @@ int avpriv_dirac_parse_sequence_header(AVCodecContext 
*avctx, GetBitContext *gb,
 if (ret  0)
 return ret;
 
+ff_set_sar(avctx, avctx-sample_aspect_ratio);
+
 /* [DIRAC_STD] picture_coding_mode shall be 0 for fields and 1 for frames
  * currently only used to signal field coding */
 picture_coding_mode = svq3_get_ue_golomb(gb);
diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c
index 0dfa538..c796387 100644
--- a/libavcodec/dpx.c
+++ b/libavcodec/dpx.c
@@ -150,6 +150,8 @@ static int decode_frame(AVCodecContext *avctx,
 if ((ret = ff_set_dimensions(avctx, w, h))  0)
 return ret;
 
+ff_set_sar(avctx, avctx-sample_aspect_ratio);
+
 if ((ret = ff_get_buffer(avctx, p, 0))  0) {
 av_log(avctx, AV_LOG_ERROR, get_buffer() failed\n);
 return ret;
diff --git a/libavcodec/dvdec.c b/libavcodec/dvdec.c
index ef9ba4c..77f8473 100644
--- a/libavcodec/dvdec.c
+++ b/libavcodec/dvdec.c
@@ -36,6 +36,7 @@
  */
 
 #include libavutil/internal.h
+#include libavutil/imgutils.h
 #include libavutil/pixdesc.h
 #include avcodec.h
 #include internal.h
@@ -337,6 +338,14 @@ static int dvvideo_decode_frame(AVCodecContext *avctx,
 if (ret  0)
 return ret;
 
+/* Determine the codec's sample_aspect ratio from the packet */
+vsc_pack = buf + 80*5 + 48 + 5;
+if ( *vsc_pack == dv_video_control ) {
+apt = buf[4]  0x07;
+is16_9 = (vsc_pack  ((vsc_pack[2]  0x07) == 0x02 || (!apt  
(vsc_pack[2]  0x07) == 0x07)));
+ff_set_sar(avctx, s-sys-sar[is16_9]);
+}
+
 if (ff_get_buffer(avctx, s-frame, 0)  0) {
 av_log(avctx, AV_LOG_ERROR, get_buffer() failed\n);
 return -1;
@@ -353,14 +362,6 @@ static int dvvideo_decode_frame(AVCodecContext *avctx,
 /* return image */
 *got_frame = 1;
 
-/* Determine the codec's sample_aspect ratio from the packet */
-vsc_pack = buf + 80*5 + 48 + 5;
-if ( *vsc_pack == dv_video_control ) {
-apt = buf[4]  0x07;
-is16_9 = (vsc_pack  ((vsc_pack[2]  0x07) == 0x02 || (!apt  
(vsc_pack[2]  0x07) == 0x07)));
-avctx-sample_aspect_ratio = s-sys-sar[is16_9];
-}
-
 return s-sys-frame_size;
 }
 
diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 9f60cc6..37a31ce 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1107,8 +1107,8 @@ static int decode_header(EXRContext *s)
 if (!var_size)
 return AVERROR_INVALIDDATA;
 
-s-avctx-sample_aspect_ratio =
-av_d2q(av_int2float(bytestream2_get_le32(s-gb)), 255);
+ff_set_sar(s-avctx,
+   av_d2q(av_int2float(bytestream2_get_le32(s-gb)), 
255));
 
 continue;
 } else if ((var_size = check_header_variable(s, compression,
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 8f7b2bf..703491e 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -328,6 +328,14 @@ static int decode_slice_header(FFV1Context *f, FFV1Context 
*fs)
 f-cur-sample_aspect_ratio.num = get_symbol(c, state, 0);
 f-cur-sample_aspect_ratio.den = get_symbol(c, state, 0);
 
+if (av_image_check_sar(f-width, f-height,
+   f-cur-sample_aspect_ratio)  0) {
+av_log(f-avctx, AV_LOG_WARNING, ignoring invalid SAR: %u/%u\n,
+   f-cur-sample_aspect_ratio.num,
+   f-cur-sample_aspect_ratio.den);
+f-cur-sample_aspect_ratio = (AVRational){ 0, 1 };
+}
+
 return 0;
 }
 
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index f70feb9..b081e30 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -497,6 +497,8 @@ int 

Re: [libav-devel] [PATCH] Add av_image_check_sar() and use it to validate SAR

2014-05-30 Thread Justin Ruggles

On 05/30/2014 02:20 PM, Diego Biurrun wrote:

On Fri, May 30, 2014 at 01:20:36PM -0400, Justin Ruggles wrote:


--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -228,6 +230,27 @@ int av_image_check_size(unsigned int w, unsigned int h, 
int log_offset, void *lo
  
+int av_image_check_sar(unsigned int w, unsigned int h, AVRational sar)

+{
+int64_t scaled_dim;
+
+if (!sar.den)
+return AVERROR(EINVAL);
+
+if (!sar.num || sar.num == sar.den)
+return 0;
+
+if (sar.num  sar.den) {
+scaled_dim = av_rescale_rnd(w, sar.num, sar.den, AV_ROUND_ZERO);
+} else {
+scaled_dim = av_rescale_rnd(h, sar.den, sar.num, AV_ROUND_ZERO);
+}
+if (scaled_dim  0) {
+return 0;
+}

nit: extra parentheses


Where?

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


Re: [libav-devel] [PATCH] Add av_image_check_sar() and use it to validate SAR

2014-05-30 Thread Justin Ruggles

On 05/30/2014 03:23 PM, Diego Biurrun wrote:

On Fri, May 30, 2014 at 02:43:38PM -0400, Justin Ruggles wrote:

On 05/30/2014 02:20 PM, Diego Biurrun wrote:

On Fri, May 30, 2014 at 01:20:36PM -0400, Justin Ruggles wrote:

--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -228,6 +230,27 @@ int av_image_check_size(unsigned int w, unsigned int h, 
int log_offset, void *lo
+int av_image_check_sar(unsigned int w, unsigned int h, AVRational sar)
+{
+int64_t scaled_dim;
+
+if (!sar.den)
+return AVERROR(EINVAL);
+
+if (!sar.num || sar.num == sar.den)
+return 0;
+
+if (sar.num  sar.den) {
+scaled_dim = av_rescale_rnd(w, sar.num, sar.den, AV_ROUND_ZERO);
+} else {
+scaled_dim = av_rescale_rnd(h, sar.den, sar.num, AV_ROUND_ZERO);
+}
+if (scaled_dim  0) {
+return 0;
+}

nit: extra parentheses

Where?

.. extra {} for some, but not all if-blocks ..

Parentheses?  Brackets?  Braces?  They keep confusing me :)



Ah, ok. That's probably too much Go programming at work influencing my C 
coding style. :) I'll change it.


-Justin

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


Re: [libav-devel] [PATCH 1/4] lavc: add an option to enable side data-only packets during encoding

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:44 PM, Anton Khirnov wrote:
 Some encoders (e.g. flac) need to send side data when there is no more
 data to be output. This enables them to output a packet with no data in
 it, only side data.
 ---
  doc/APIchanges |5 +
  libavcodec/avcodec.h   |   15 +++
  libavcodec/options_table.h |1 +
  3 files changed, 21 insertions(+)
 
 diff --git a/doc/APIchanges b/doc/APIchanges
 index 9bc3f1a..4aae681 100644
 --- a/doc/APIchanges
 +++ b/doc/APIchanges
 @@ -13,6 +13,11 @@ libavutil: 2013-12-xx
  
  API changes, most recent first:
  
 +2014-04-xx - xxx - lavc 55.xx.0 - avcodec.h
 +  Add AVCodecContext.side_data_only_packets to allow encoders to output 
 packets
 +  with only side data. This option may become mandatory in the future, so all
 +  users are recommended to update their code and enable this option.

I assume you plan on sending a separate patch to enable this in avconv.

-Justin

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


Re: [libav-devel] [PATCH 2/4] flacenc: send final extradata in packet side data

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:44 PM, Anton Khirnov wrote:
 ---
  libavcodec/flacenc.c |   20 
  1 file changed, 20 insertions(+)

OK

-Justin

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


Re: [libav-devel] [PATCH 3/4] flac muxer: accept only STREAMINFO extradata

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:44 PM, Anton Khirnov wrote:
 The other format (full flac header blocks) should not be exported by any
 demuxers anymore.
 
 This allows to drop an avpriv_ function and also simplify the following
 commits.
 ---
  libavformat/flacenc_header.c |   11 +--
  1 file changed, 5 insertions(+), 6 deletions(-)

I'm ok with it. I just hope we don't break anyone's code. Maybe it
should be included in the next version migration document just to be safe.

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


Re: [libav-devel] [PATCH 4/4] flac muxer: support reading updated extradata from side data

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:44 PM, Anton Khirnov wrote:
 ---
  libavformat/flacenc.c|   37 +
  libavformat/flacenc.h|4 ++--
  libavformat/flacenc_header.c |8 
  libavformat/matroskaenc.c|3 ++-
  4 files changed, 37 insertions(+), 15 deletions(-)

Ok

-Justin

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


Re: [libav-devel] [PATCH] flac demuxer: parse the WAVEFORMATEXTENSIBLE_CHANNEL_MASK tag

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:27 PM, Anton Khirnov wrote:
 It is used to store the channel mask for non-standard layouts.
 ---
 added ULL to the constant so that it's properly inverted.
 ---
  libavformat/flacdec.c |   15 +++
  1 file changed, 15 insertions(+)
 
 diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
 index 11360a9..d02d8b5 100644
 --- a/libavformat/flacdec.c
 +++ b/libavformat/flacdec.c
 @@ -139,9 +139,24 @@ static int flac_read_header(AVFormatContext *s)
  }
  /* process supported blocks other than STREAMINFO */
  if (metadata_type == FLAC_METADATA_TYPE_VORBIS_COMMENT) {
 +AVDictionaryEntry *chmask;
 +
  if (ff_vorbis_comment(s, s-metadata, buffer, 
 metadata_size)) {
  av_log(s, AV_LOG_WARNING, error parsing VorbisComment 
 metadata\n);
  }
 +
 +/* parse the channels mask if present */
 +chmask = av_dict_get(s-metadata, 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK, NULL, 0);
 +if (chmask) {
 +uint64_t mask = strtol(chmask-value, NULL, 0);
 +if (!mask || mask  ~0x3ULL) {
 +av_log(s, AV_LOG_WARNING,
 +   Invalid value of 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK\n);
 +} else {
 +st-codec-channel_layout = mask;
 +av_dict_set(s-metadata, 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK, NULL, 0);
 +}
 +}
  }
  av_freep(buffer);
  }

Ok

-Justin

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


Re: [libav-devel] [PATCH] flac muxer: write WAVEFORMATEXTENSIBLE_CHANNEL_MASK tag for multichannel files

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:26 PM, Anton Khirnov wrote:
 ---
  libavformat/flacenc.c|   18 ++
  libavformat/flacenc.h|2 ++
  libavformat/flacenc_header.c |   16 
  3 files changed, 36 insertions(+)

Ok

-Justin

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


Re: [libav-devel] [PATCH] matroskadec: parse the channel layout mask for FLAC

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:29 PM, Anton Khirnov wrote:
 It is commonly stored in a vorbiscomment block in codec private data.
 ---
 Check that the mask is only the lower 18 bits
 ---
  libavformat/Makefile |3 ++-
  libavformat/flacdec.c|2 +-
  libavformat/matroskadec.c|   38 ++
  libavformat/oggdec.h |3 ++-
  libavformat/oggparsecelt.c   |2 +-
  libavformat/oggparseflac.c   |2 +-
  libavformat/oggparseogm.c|2 +-
  libavformat/oggparseopus.c   |2 +-
  libavformat/oggparsespeex.c  |2 +-
  libavformat/oggparsetheora.c |2 +-
  libavformat/oggparsevorbis.c |7 ---
  11 files changed, 53 insertions(+), 12 deletions(-)

Ok

-Justin

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


Re: [libav-devel] [PATCH] matroskaenc: write the channel mask for FLAC

2014-05-27 Thread Justin Ruggles
On 05/26/2014 11:34 PM, Anton Khirnov wrote:
 ---
  libavformat/Makefile  |2 +-
  libavformat/matroskaenc.c |   47 
 -
  2 files changed, 47 insertions(+), 2 deletions(-)

Ok

-Justin

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


Re: [libav-devel] [PATCH 5/9] matroskadec: split parsing tracks into a separate function

2014-05-27 Thread Justin Ruggles
On 05/26/2014 07:56 AM, Anton Khirnov wrote:
 ---
  libavformat/matroskadec.c |  127 
 -
  1 file changed, 69 insertions(+), 58 deletions(-)

probably ok

-Justin

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


Re: [libav-devel] [PATCH 1/9] flacdec: do not overwrite a channel layout set by the caller

2014-05-27 Thread Justin Ruggles
On 05/26/2014 07:56 AM, Anton Khirnov wrote:
 The channel layout mask for non-standard layouts is typically stored at
 the container level (as a vorbiscomment tag) for FLAC.
 ---
  libavcodec/flac.c |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/libavcodec/flac.c b/libavcodec/flac.c
 index b3e3847..e6a8ab8 100644
 --- a/libavcodec/flac.c
 +++ b/libavcodec/flac.c
 @@ -225,7 +225,10 @@ void avpriv_flac_parse_streaminfo(AVCodecContext *avctx, 
 struct FLACStreaminfo *
  avctx-channels = s-channels;
  avctx-sample_rate = s-samplerate;
  avctx-bits_per_raw_sample = s-bps;
 -ff_flac_set_channel_layout(avctx);
 +
 +if (!avctx-channel_layout ||
 +av_get_channel_layout_nb_channels(avctx-channel_layout) != 
 avctx-channels)
 +ff_flac_set_channel_layout(avctx);
  
  s-samples  = get_bits_long(gb, 32)  4;
  s-samples |= get_bits(gb, 4);

Ok

-Justin

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


Re: [libav-devel] [PATCH 2/9] flac demuxer: parse the WAVEFORMATEXTENSIBLE_CHANNEL_MASK tag

2014-05-26 Thread Justin Ruggles
On 05/26/2014 07:56 AM, Anton Khirnov wrote:
 It is used to store the channel mask for non-standard layouts.
 ---
  libavformat/flacdec.c |   15 +++
  1 file changed, 15 insertions(+)
 
 diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
 index 11360a9..479e5d3 100644
 --- a/libavformat/flacdec.c
 +++ b/libavformat/flacdec.c
 @@ -139,9 +139,24 @@ static int flac_read_header(AVFormatContext *s)
  }
  /* process supported blocks other than STREAMINFO */
  if (metadata_type == FLAC_METADATA_TYPE_VORBIS_COMMENT) {
 +AVDictionaryEntry *chmask;
 +
  if (ff_vorbis_comment(s, s-metadata, buffer, 
 metadata_size)) {
  av_log(s, AV_LOG_WARNING, error parsing VorbisComment 
 metadata\n);
  }
 +
 +/* parse the channels mask if present */
 +chmask = av_dict_get(s-metadata, 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK, NULL, 0);
 +if (chmask) {
 +uint64_t mask = strtol(chmask-value, NULL, 0);
 +if (!mask) {
 +av_log(s, AV_LOG_WARNING,
 +   Invalid value of 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK\n);
 +} else {
 +st-codec-channel_layout = mask;
 +av_dict_set(s-metadata, 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK, NULL, 0);
 +}
 +}
  }
  av_freep(buffer);
  }

Might be a good idea to check that the value doesn't have bits set that
do not match up with Libav channel layout. IIRC that's anything past the
first 18 bits.

-Justin

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


Re: [libav-devel] [PATCH 3/9] flac muxer: write WAVEFORMATEXTENSIBLE_CHANNEL_MASK tag for multichannel files

2014-05-26 Thread Justin Ruggles
On 05/26/2014 07:56 AM, Anton Khirnov wrote:
 ---
  libavformat/flacenc.c |   18 ++
  1 file changed, 18 insertions(+)
 
 diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
 index 8a23163..dc75133 100644
 --- a/libavformat/flacenc.c
 +++ b/libavformat/flacenc.c
 @@ -19,6 +19,7 @@
   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
 USA
   */
  
 +#include libavutil/channel_layout.h
  #include libavutil/opt.h
  #include libavcodec/flac.h
  #include avformat.h
 @@ -83,6 +84,23 @@ static int flac_write_header(struct AVFormatContext *s)
  if (ret)
  return ret;
  
 +/* add the channel layout tag */
 +if (codec-channel_layout 
 +codec-channel_layout != AV_CH_LAYOUT_MONO 
 +codec-channel_layout != AV_CH_LAYOUT_STEREO) {
 +AVDictionaryEntry *chmask = av_dict_get(s-metadata, 
 WAVEFORMATEXTENSIBLE_CHANNEL_MASK,
 +NULL, 0);
 +
 +if (chmask) {
 +av_log(s, AV_LOG_WARNING, A WAVEFORMATEXTENSIBLE_CHANNEL_MASK 
 is 
 +   already present, this muxer will not overwrite it.\n);
 +} else {
 +uint8_t buf[32];
 +snprintf(buf, sizeof(buf), 0x%PRIx64, codec-channel_layout);
 +av_dict_set(s-metadata, WAVEFORMATEXTENSIBLE_CHANNEL_MASK, 
 buf, 0);
 +}
 +}
 +
  ret = flac_write_block_comment(s-pb, s-metadata, 0,
 s-flags  AVFMT_FLAG_BITEXACT);
  if (ret)

FLAC does have a pre-defined set of assumed layouts for a given channel
count. I would check the layout against those rather than just mono and
stereo.

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


  1   2   3   4   5   6   7   8   9   10   >