Re: [libav-devel] [PATCH] channel_layout: Add a 16channel default layout
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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 {}
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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.
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
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
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.
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.
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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()
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
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
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
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
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
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
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().
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.
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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