Re: [libav-devel] [RFC PATCH 1/2] h264: set parameters from SPS whenever it changes
Hi, On Fri, Nov 16, 2012 at 8:43 AM, Janne Grunau wrote: > I'm not entirely convinced that is the the correct/best way to fix the > problem with this sample. It crashes in draw_edges_10_c because the > image format and parameters has just 8 bit per pixels. > > The main problem I see is that it does far too many context re-inits. > > Janne > ---8<--- > > Fixes a crash in the fuzzed sample sample_varPAR.avi_s26638 with > alternating bit depths. > --- > libavcodec/h264.c | 88 > ++- > 1 file changed, 48 insertions(+), 40 deletions(-) > So ... I originally added code to not support changing bitdepth at all, basically to prevent this and alike issues. E.g., what if the reference is 8bit but the next frame is 10bit? What if we're using frame threading at the same time? So I wouldn't mind just erroring out altogether, unless you think this can really be made to work? I mean, if we want to support reinits, with frame threading, then we should just rewrite the init code and allow complete reinits and be convinced it always works, like we are for ffvp8. Ronald ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] pixdesc: add PIX_FMT_ALPHA flag
--- Now with pixdesc updated. libavutil/pixdesc.c | 49 + libavutil/pixdesc.h | 3 +++ 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c index d0889b4..439c550 100644 --- a/libavutil/pixdesc.c +++ b/libavutil/pixdesc.c @@ -435,7 +435,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 0, 3, 3, 0, 7 },/* G */ { 0, 3, 4, 0, 7 },/* B */ }, -.flags = PIX_FMT_RGB, +.flags = PIX_FMT_RGB | PIX_FMT_ALPHA, }, [AV_PIX_FMT_RGBA] = { .name = "rgba", @@ -448,7 +448,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 0, 3, 3, 0, 7 },/* B */ { 0, 3, 4, 0, 7 },/* A */ }, -.flags = PIX_FMT_RGB, +.flags = PIX_FMT_RGB | PIX_FMT_ALPHA, }, [AV_PIX_FMT_ABGR] = { .name = "abgr", @@ -461,7 +461,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 0, 3, 3, 0, 7 },/* G */ { 0, 3, 4, 0, 7 },/* R */ }, -.flags = PIX_FMT_RGB, +.flags = PIX_FMT_RGB | PIX_FMT_ALPHA, }, [AV_PIX_FMT_BGRA] = { .name = "bgra", @@ -474,7 +474,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 0, 3, 3, 0, 7 },/* R */ { 0, 3, 4, 0, 7 },/* A */ }, -.flags = PIX_FMT_RGB, +.flags = PIX_FMT_RGB | PIX_FMT_ALPHA, }, [AV_PIX_FMT_GRAY16BE] = { .name = "gray16be", @@ -530,7 +530,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 0, 1, 0, 7 },/* V */ { 3, 0, 1, 0, 7 },/* A */ }, -.flags = PIX_FMT_PLANAR, +.flags = PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA422P] = { .name = "yuva422p", @@ -543,7 +543,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 0, 1, 0, 7 },/* V */ { 3, 0, 1, 0, 7 },/* A */ }, -.flags = PIX_FMT_PLANAR, +.flags = PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA444P] = { .name = "yuva444p", @@ -556,7 +556,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 0, 1, 0, 7 },/* V */ { 3, 0, 1, 0, 7 },/* A */ }, -.flags = PIX_FMT_PLANAR, +.flags = PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA420P9BE] = { .name = "yuva420p9be", @@ -582,7 +582,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 1, 1, 0, 8 },/* V */ { 3, 1, 1, 0, 8 },/* A */ }, -.flags = PIX_FMT_PLANAR, +.flags = PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA422P9BE] = { .name = "yuva422p9be", @@ -595,7 +595,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 1, 1, 0, 8 },/* V */ { 3, 1, 1, 0, 8 },/* A */ }, -.flags = PIX_FMT_BE | PIX_FMT_PLANAR, +.flags = PIX_FMT_BE | PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA422P9LE] = { .name = "yuva422p9le", @@ -608,7 +608,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 1, 1, 0, 8 },/* V */ { 3, 1, 1, 0, 8 },/* A */ }, -.flags = PIX_FMT_PLANAR, +.flags = PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA444P9BE] = { .name = "yuva444p9be", @@ -621,7 +621,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 1, 1, 0, 8 },/* V */ { 3, 1, 1, 0, 8 },/* A */ }, -.flags = PIX_FMT_BE | PIX_FMT_PLANAR, +.flags = PIX_FMT_BE | PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA444P9LE] = { .name = "yuva444p9le", @@ -634,7 +634,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 1, 1, 0, 8 },/* V */ { 3, 1, 1, 0, 8 },/* A */ }, -.flags = PIX_FMT_PLANAR, +.flags = PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA420P10BE] = { .name = "yuva420p10be", @@ -647,7 +647,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { { 2, 1, 1, 0, 9 },/* V */ { 3, 1, 1, 0, 9 },/* A */ }, -.flags = PIX_FMT_BE | PIX_FMT_PLANAR, +.flags = PIX_FMT_BE | PIX_FMT_PLANAR | PIX_FMT_ALPHA, }, [AV_PIX_FMT_YUVA420P10LE] = { .name = "yuva420p10le", @@ -660,7 +660,7 @@ const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
Re: [libav-devel] [PATCH 2/2] h264: reset has_b_frames after enabling low_delay from SPS
Hi, On Fri, Nov 16, 2012 at 8:43 AM, Janne Grunau wrote: > Fixes a crash in fuzzed file nasa-8s2.ts_s20033 caused by a too large > has_b_frames value. low_delay keeps getting re-enabled from the the > presumely broken SPS. > --- > libavcodec/h264.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/h264.c b/libavcodec/h264.c > index c30c478..7935fe6 100644 > --- a/libavcodec/h264.c > +++ b/libavcodec/h264.c > @@ -2346,8 +2346,10 @@ static int h264_set_parameter_from_sps(H264Context > *h) > > if (s->flags & CODEC_FLAG_LOW_DELAY || > (h->sps.bitstream_restriction_flag && > - !h->sps.num_reorder_frames)) > + !h->sps.num_reorder_frames)) { > s->low_delay = 1; > +s->avctx->has_b_frames = 0; > +} > So I'm going to have to wonder what happens to the delayed frames already cached in h->delayed_pics[]? Are they still output? What happens if we concatenate a delayed h264 file by a file that has this kind of sps header? Ronald ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 1/2] build: set -U__STRICT_ANSI__ for newlib
On 11/17/2012 12:19 AM, Mans Rullgard wrote: > This is (erroneously) required to enable various things in the > newlib headers. As cygwin uses newlib, it is covered by this. > > Signed-off-by: Mans Rullgard > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Both ok. lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 1/2] build: set -U__STRICT_ANSI__ for newlib
This is (erroneously) required to enable various things in the newlib headers. As cygwin uses newlib, it is covered by this. Signed-off-by: Mans Rullgard --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index af22b10..cfc7010 100755 --- a/configure +++ b/configure @@ -2863,7 +2863,6 @@ case $target_os in SHFLAGS='-shared -Wl,--out-implib,$(SUBDIR)lib$(FULLNAME).dll.a' objformat="win32" enable dos_paths -add_cppflags -U__STRICT_ANSI__ ;; *-dos|freedos|opendos) network_extralibs="-lsocket" @@ -2964,6 +2963,7 @@ elif check_header _mingw.h; then die "ERROR: MinGW runtime version must be >= 3.15." elif check_cpp_condition newlib.h "defined _NEWLIB_VERSION"; then libc_type=newlib +add_cppflags -U__STRICT_ANSI__ elif check_func_headers stdlib.h _get_doserrno; then libc_type=msvcrt add_compat strtod.o strtod=avpriv_strtod -- 1.8.0 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 2/2] configure: add basic support for ARM AArch64
Signed-off-by: Mans Rullgard --- configure | 15 +++ 1 file changed, 15 insertions(+) diff --git a/configure b/configure index cfc7010..fbc49dd 100755 --- a/configure +++ b/configure @@ -1068,6 +1068,7 @@ THREADS_LIST=' ' ARCH_LIST=' +aarch64 alpha arm avr32 @@ -2491,6 +2492,9 @@ fi # Deal with common $arch aliases case "$arch" in +aarch64|arm64) +arch="aarch64" +;; arm*) arch="arm" ;; @@ -2666,6 +2670,17 @@ elif enabled avr32; then ;; esac +elif enabled aarch64; then + +case $cpu in +armv*) +cpuflags="-march=$cpu" +;; +*) +cpuflags="-mcpu=$cpu" +;; +esac + fi add_cflags $cpuflags -- 1.8.0 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 41/45] x86: Rename SPLATD macro to SPLATD_OFFSET
On 07/31/2012 06:18 PM, Diego Biurrun wrote: > This will allow porting the optimized versions to cpuflags. > --- > libavutil/x86/x86util.asm |2 +- > libswscale/x86/output.asm |2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavutil/x86/x86util.asm b/libavutil/x86/x86util.asm > index 3c2ebce..52e5e58 100644 > --- a/libavutil/x86/x86util.asm > +++ b/libavutil/x86/x86util.asm > @@ -550,7 +550,7 @@ > %endif > %endmacro > > -%macro SPLATD 2-3 0 > +%macro SPLATD_OFFSET 2-3 0 > %if mmsize == 16 > pshufd %1, %2, (%3)*0x55 > %else > diff --git a/libswscale/x86/output.asm b/libswscale/x86/output.asm > index d137e6e..f822ca4 100644 > --- a/libswscale/x86/output.asm > +++ b/libswscale/x86/output.asm > @@ -187,7 +187,7 @@ cglobal yuv2planeX_%1, %3, 8, %2, filter, fltsize, src, > dst, w, dither, offset > %else ; %1 == 10/9/8 > punpcklwd m5, m3, m4 > punpckhwd m3, m4 > -SPLATD m0, m0 > +SPLATD_OFFSET m0, m0 > > pmaddwd m5, m0 > pmaddwd m3, m0 But that doesn't even use an offset... Why not just use the other cpuflag-specific SPLATD macro(s) there and remove the one that uses an offset? -Justin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 1/2] x86: PMINSD/PMAXSD: port to cpuflags
On 11/16/2012 06:31 AM, Diego Biurrun wrote: > On Wed, Nov 14, 2012 at 05:28:53PM -0500, Justin Ruggles wrote: >> On 11/14/2012 04:16 PM, Diego Biurrun wrote: >>> --- a/libavutil/x86/x86util.asm >>> +++ b/libavutil/x86/x86util.asm >>> @@ -599,25 +599,37 @@ >>> >>> -%macro PMINSD_MMX 3 ; dst, src, tmp >>> +%macro PMINSD 3 ; dst, src, tmp/unused >>> +%if mmsize == 8 >>> mova %3, %2 >>> pcmpgtd %3, %1 >>> pxor %1, %2 >>> pand %1, %3 >>> pxor %1, %2 >>> +%elif cpuflag(sse4) >>> +pminsd%1, %2 >>> +%elif cpuflag(sse2) >>> +cvtdq2ps %1, %1 >>> +minps %1, %2 >>> +cvtps2dq %1, %1 >>> +%endif >>> %endmacro >> >> It needs to be clearly documented that, for the sse2 versions, src is >> float not dword. > > Does it make sense to keep it in the same macro then? > > I also notice that you sent a patch port PMINSD/PMAXSD to cpuflags some > time ago, but it was never applied ... I thought it looked familiar... :) Yeah, having a separate macro would be fine. PMINSD_FLT or something. It would also work for YMM AVX, where-as the dword version would not. -Justin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] PGS subtitles: Set AVSubtitle pts value
On Fri, 2 Nov 2012 09:32:51 -0700, John Stebbins wrote: > On 11/02/2012 08:20 AM, Diego Biurrun wrote: > > On Sun, Oct 14, 2012 at 04:52:12PM +0200, John Stebbins wrote: > >> pts should be that of the packet containing the presentation segment. > >> --- > >> libavcodec/pgssubdec.c |9 +++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > > ping, anybody? > > > > > Patch updated to apply cleanly to head. > > -- > John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 > D0F7 > > From d4522c73106fe37686154ca924811bb895817782 Mon Sep 17 00:00:00 2001 > From: John Stebbins > Date: Fri, 2 Nov 2012 09:30:39 -0700 > Subject: [PATCH] PGS subtitles: Set AVSubtitle pts value > > pts should be that of the packet containing the presentation segment. > --- > libavcodec/pgssubdec.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c > index 0326ea8..f22088a 100644 > --- a/libavcodec/pgssubdec.c > +++ b/libavcodec/pgssubdec.c > @@ -46,6 +46,7 @@ typedef struct PGSSubPresentation { > int id_number; > int object_number; > uint8_t composition_flag; > +int64_t pts; > } PGSSubPresentation; > > typedef struct PGSSubPicture { > @@ -272,7 +273,8 @@ static void parse_palette_segment(AVCodecContext *avctx, > * @todo TODO: Implement forcing of subtitles > */ > static void parse_presentation_segment(AVCodecContext *avctx, > - const uint8_t *buf, int buf_size) > + const uint8_t *buf, int buf_size, > + int64_t pts) > { > PGSSubContext *ctx = avctx->priv_data; > > @@ -281,6 +283,8 @@ static void parse_presentation_segment(AVCodecContext > *avctx, > int w = bytestream_get_be16(&buf); > int h = bytestream_get_be16(&buf); > > +ctx->presentation.pts = pts; > + > av_dlog(avctx, "Video Dimensions %dx%d\n", > w, h); > if (av_image_check_size(w, h, 0, avctx) >= 0) > @@ -358,6 +362,8 @@ static int display_end_segment(AVCodecContext *avctx, > void *data, > */ > > memset(sub, 0, sizeof(*sub)); > +sub->pts = ctx->presentation.pts; The docs say pts is supposed to be in AV_TIME_BASE, while the packet pts is presumable in the decoder timebase. Not sure if the other subs decoders follow this though... Are you familiar with them? -- Anton Khirnov ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 2/2] h264: reset has_b_frames after enabling low_delay from SPS
Fixes a crash in fuzzed file nasa-8s2.ts_s20033 caused by a too large has_b_frames value. low_delay keeps getting re-enabled from the the presumely broken SPS. --- libavcodec/h264.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264.c b/libavcodec/h264.c index c30c478..7935fe6 100644 --- a/libavcodec/h264.c +++ b/libavcodec/h264.c @@ -2346,8 +2346,10 @@ static int h264_set_parameter_from_sps(H264Context *h) if (s->flags & CODEC_FLAG_LOW_DELAY || (h->sps.bitstream_restriction_flag && - !h->sps.num_reorder_frames)) + !h->sps.num_reorder_frames)) { s->low_delay = 1; +s->avctx->has_b_frames = 0; +} if (s->avctx->has_b_frames < 2) s->avctx->has_b_frames = !s->low_delay; -- 1.7.12.4 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [RFC PATCH 1/2] h264: set parameters from SPS whenever it changes
I'm not entirely convinced that is the the correct/best way to fix the problem with this sample. It crashes in draw_edges_10_c because the image format and parameters has just 8 bit per pixels. The main problem I see is that it does far too many context re-inits. Janne ---8<--- Fixes a crash in the fuzzed sample sample_varPAR.avi_s26638 with alternating bit depths. --- libavcodec/h264.c | 88 ++- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/libavcodec/h264.c b/libavcodec/h264.c index f45c572..c30c478 100644 --- a/libavcodec/h264.c +++ b/libavcodec/h264.c @@ -2340,6 +2340,47 @@ int ff_h264_get_profile(SPS *sps) return profile; } +static int h264_set_parameter_from_sps(H264Context *h) +{ +MpegEncContext *s = &h->s; + +if (s->flags & CODEC_FLAG_LOW_DELAY || +(h->sps.bitstream_restriction_flag && + !h->sps.num_reorder_frames)) +s->low_delay = 1; + +if (s->avctx->has_b_frames < 2) +s->avctx->has_b_frames = !s->low_delay; + +if (s->avctx->bits_per_raw_sample != h->sps.bit_depth_luma || +h->cur_chroma_format_idc != h->sps.chroma_format_idc) { +if (s->avctx->codec && +s->avctx->codec->capabilities & CODEC_CAP_HWACCEL_VDPAU +&& (h->sps.bit_depth_luma != 8 || h->sps.chroma_format_idc > 1)) { +av_log(s->avctx, AV_LOG_ERROR, + "VDPAU decoding does not support video colorspace\n"); +return AVERROR_INVALIDDATA; +} +if (h->sps.bit_depth_luma >= 8 && h->sps.bit_depth_luma <= 10) { +s->avctx->bits_per_raw_sample = h->sps.bit_depth_luma; +h->cur_chroma_format_idc = h->sps.chroma_format_idc; +h->pixel_shift= h->sps.bit_depth_luma > 8; + +ff_h264dsp_init(&h->h264dsp, h->sps.bit_depth_luma, +h->sps.chroma_format_idc); +ff_h264_pred_init(&h->hpc, s->codec_id, h->sps.bit_depth_luma, + h->sps.chroma_format_idc); +s->dsp.dct_bits = h->sps.bit_depth_luma > 8 ? 32 : 16; +ff_dsputil_init(&s->dsp, s->avctx); +} else { +av_log(s->avctx, AV_LOG_ERROR, "Unsupported bit depth: %d\n", + h->sps.bit_depth_luma); +return AVERROR_INVALIDDATA; +} +} +return 0; +} + /** * Decode a slice header. * This will also call ff_MPV_common_init() and frame_start() as needed. @@ -2356,7 +2397,7 @@ static int decode_slice_header(H264Context *h, H264Context *h0) MpegEncContext *const s0 = &h0->s; unsigned int first_mb_in_slice; unsigned int pps_id; -int num_ref_idx_active_override_flag, max_refs; +int num_ref_idx_active_override_flag, max_refs, ret; unsigned int slice_type, tmp, i, j; int default_ref_list_done = 0; int last_pic_structure, last_pic_dropable; @@ -2434,6 +2475,9 @@ static int decode_slice_header(H264Context *h, H264Context *h0) } h->sps = *h0->sps_buffers[h->pps.sps_id]; +if ((ret = h264_set_parameter_from_sps(h)) < 0) +return ret; + s->avctx->profile = ff_h264_get_profile(&h->sps); s->avctx->level = h->sps.level_idc; s->avctx->refs= h->sps.ref_frame_count; @@ -3865,45 +3909,9 @@ again: ff_h264_decode_seq_parameter_set(h); } -if (s->flags & CODEC_FLAG_LOW_DELAY || -(h->sps.bitstream_restriction_flag && - !h->sps.num_reorder_frames)) -s->low_delay = 1; - -if (avctx->has_b_frames < 2) -avctx->has_b_frames = !s->low_delay; - -if (avctx->bits_per_raw_sample != h->sps.bit_depth_luma || -h->cur_chroma_format_idc != h->sps.chroma_format_idc) { -if (s->avctx->codec && -s->avctx->codec->capabilities & CODEC_CAP_HWACCEL_VDPAU -&& (h->sps.bit_depth_luma != 8 || -h->sps.chroma_format_idc > 1)) { -av_log(avctx, AV_LOG_ERROR, - "VDPAU decoding does not support video " - "colorspace\n"); -buf_index = -1; -goto end; -} -if (h->sps.bit_depth_luma >= 8 && h->sps.bit_depth_luma <= 10) { -avctx->bits_per_raw_sample = h->sps.bit_depth_luma; -h->cur_chroma_format_idc = h->sps.chroma_format_idc; -h->pixel_shift = h->sps.bit_depth_luma > 8; - -ff_h264dsp_init(&h->h264dsp, h->sps.bit_depth_luma, -h->sps.chroma_format_idc); -ff_h264_pred_init(&h->hpc, s->codec_id, - h
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
On 11/16/2012 04:19 PM, Diego Biurrun wrote: > It does not disable everything, contrary to what the name suggests. As long it is documented (and it is) I see no problem. The result of this discussion ended with an useful option (--disable-programs) but I don't see why we should break people option. If you want to add --enable-components and --disable-all I'm not against. Please do not change the meaning of everything. lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
On Fri, Nov 16, 2012 at 11:55:34AM +, Måns Rullgård wrote: > Diego Biurrun writes: > > On Fri, Nov 16, 2012 at 10:33:38AM +, Måns Rullgård wrote: > >> Diego Biurrun writes: > >> > On Fri, Nov 16, 2012 at 10:06:16AM +, Måns Rullgård wrote: > >> >> Diego Biurrun writes: > >> >> > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: > >> >> >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: > >> >> >> > Now the difference between "all" and "everything" seems arbitrary. > >> >> >> > Why don't we just set clear semantics on what we call "parts" and > >> >> >> > "components" or "components" and "subcomponents"? > >> >> >> > >> >> >> From an usability point of view you are not going to change that > >> >> >> option, > >> >> >> the best is to clarify it making so from reading --help you would not > >> >> >> expect it to disable something it does not. > >> >> > > >> >> > The usability is what I am trying to fix here, among other things. > >> >> > Having > >> >> > both --disable-everything and --disable-all as options is a usability > >> >> > nightmare. I'd have to look up which option did what myself in a few > >> >> > weeks time after implementing them... > >> >> > > >> >> > So what's bad about --disable-components or --disable-subcomponents? > >> >> > More importantly, what about those names is worse than what we have > >> >> > right now: --disable-everything? > >> >> > >> >> For better or worse, we have --disable-everything now, and people are > >> >> using it. Changing it to an equally arbitrary name will only make those > >> >> people angry. > >> > > >> > I suspect that I am one of the main users of that option, but anyway.. > >> > Your remark misses the context of my proposal, which does not eliminate > >> > --disable-everything. > >> > >> It changes the meaning of it, which is even worse. > > > > The semantics of --disable-everything are broken to begin with, so this > > hardly counts. > > Broken how? It does not disable everything, contrary to what the name suggests. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 1/1] libswscale: remove unnecessary direct #if LIBSWSCALE_VERSION_MAJOR
On 11/16/2012 03:04 PM, Janne Grunau wrote: > SWS_CPU_CAPS are deprecated and slated to removed with libswscale major > version 3. No need to provide a SWS_CPU_CAPS_MMX2 as backward > compatibility define under the same explicit condition. > --- > libswscale/swscale.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h > index 598946d..8ba09e6 100644 > --- a/libswscale/swscale.h > +++ b/libswscale/swscale.h > @@ -85,9 +85,7 @@ const char *swscale_license(void); > */ > #define SWS_CPU_CAPS_MMX 0x8000 > #define SWS_CPU_CAPS_MMXEXT 0x2000 > -#if LIBSWSCALE_VERSION_MAJOR < 3 > #define SWS_CPU_CAPS_MMX2 0x2000 > -#endif > #define SWS_CPU_CAPS_3DNOW0x4000 > #define SWS_CPU_CAPS_ALTIVEC 0x1000 > #define SWS_CPU_CAPS_BFIN 0x0100 > ok. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 1/1] libswscale: remove unnecessary direct #if LIBSWSCALE_VERSION_MAJOR
SWS_CPU_CAPS are deprecated and slated to removed with libswscale major version 3. No need to provide a SWS_CPU_CAPS_MMX2 as backward compatibility define under the same explicit condition. --- libswscale/swscale.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/libswscale/swscale.h b/libswscale/swscale.h index 598946d..8ba09e6 100644 --- a/libswscale/swscale.h +++ b/libswscale/swscale.h @@ -85,9 +85,7 @@ const char *swscale_license(void); */ #define SWS_CPU_CAPS_MMX 0x8000 #define SWS_CPU_CAPS_MMXEXT 0x2000 -#if LIBSWSCALE_VERSION_MAJOR < 3 #define SWS_CPU_CAPS_MMX2 0x2000 -#endif #define SWS_CPU_CAPS_3DNOW0x4000 #define SWS_CPU_CAPS_ALTIVEC 0x1000 #define SWS_CPU_CAPS_BFIN 0x0100 -- 1.7.12.4 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 1/1] cpu: define AV_CPU_FLAG_MMX2 for libavutil major 52
On 11/16/2012 02:56 PM, Janne Grunau wrote: > --- > libavutil/cpu.h | 2 +- > libavutil/version.h | 3 +++ > 2 files changed, 4 insertions(+), 1 deletion(-) > Thanks, patch ok and unbreaks vlc-2.0.4 and other. lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 1/1] cpu: define AV_CPU_FLAG_MMX2 for libavutil major 52
--- libavutil/cpu.h | 2 +- libavutil/version.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libavutil/cpu.h b/libavutil/cpu.h index 01f7201..4929512 100644 --- a/libavutil/cpu.h +++ b/libavutil/cpu.h @@ -28,7 +28,7 @@ /* lower 16 bits - CPU features */ #define AV_CPU_FLAG_MMX 0x0001 ///< standard MMX #define AV_CPU_FLAG_MMXEXT 0x0002 ///< SSE integer functions or AMD MMX ext -#if LIBAVUTIL_VERSION_MAJOR < 52 +#if FF_API_CPU_FLAG_MMX2 #define AV_CPU_FLAG_MMX2 0x0002 ///< SSE integer functions or AMD MMX ext #endif #define AV_CPU_FLAG_3DNOW0x0004 ///< AMD 3DNOW diff --git a/libavutil/version.h b/libavutil/version.h index 6f79eeb..f69c73e 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -76,6 +76,9 @@ #ifndef FF_API_AUDIOCONVERT #define FF_API_AUDIOCONVERT (LIBAVUTIL_VERSION_MAJOR < 53) #endif +#ifndef FF_API_CPU_FLAG_MMX2 +#define FF_API_CPU_FLAG_MMX2(LIBAVUTIL_VERSION_MAJOR < 53) +#endif /** * @} -- 1.7.12.4 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
Diego Biurrun writes: > On Fri, Nov 16, 2012 at 10:33:38AM +, Måns Rullgård wrote: >> Diego Biurrun writes: >> > On Fri, Nov 16, 2012 at 10:06:16AM +, Måns Rullgård wrote: >> >> Diego Biurrun writes: >> >> > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: >> >> >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: >> >> >> > Now the difference between "all" and "everything" seems arbitrary. >> >> >> > Why don't we just set clear semantics on what we call "parts" and >> >> >> > "components" or "components" and "subcomponents"? >> >> >> >> >> >> From an usability point of view you are not going to change that >> >> >> option, >> >> >> the best is to clarify it making so from reading --help you would not >> >> >> expect it to disable something it does not. >> >> > >> >> > The usability is what I am trying to fix here, among other things. >> >> > Having >> >> > both --disable-everything and --disable-all as options is a usability >> >> > nightmare. I'd have to look up which option did what myself in a few >> >> > weeks time after implementing them... >> >> > >> >> > So what's bad about --disable-components or --disable-subcomponents? >> >> > More importantly, what about those names is worse than what we have >> >> > right now: --disable-everything? >> >> >> >> For better or worse, we have --disable-everything now, and people are >> >> using it. Changing it to an equally arbitrary name will only make those >> >> people angry. >> > >> > I suspect that I am one of the main users of that option, but anyway.. >> > Your remark misses the context of my proposal, which does not eliminate >> > --disable-everything. >> >> It changes the meaning of it, which is even worse. > > The semantics of --disable-everything are broken to begin with, so this > hardly counts. Broken how? -- Måns Rullgård m...@mansr.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 1/2] x86: PMINSD/PMAXSD: port to cpuflags
On Wed, Nov 14, 2012 at 05:28:53PM -0500, Justin Ruggles wrote: > On 11/14/2012 04:16 PM, Diego Biurrun wrote: > > --- a/libavutil/x86/x86util.asm > > +++ b/libavutil/x86/x86util.asm > > @@ -599,25 +599,37 @@ > > > > -%macro PMINSD_MMX 3 ; dst, src, tmp > > +%macro PMINSD 3 ; dst, src, tmp/unused > > +%if mmsize == 8 > > mova %3, %2 > > pcmpgtd %3, %1 > > pxor %1, %2 > > pand %1, %3 > > pxor %1, %2 > > +%elif cpuflag(sse4) > > +pminsd%1, %2 > > +%elif cpuflag(sse2) > > +cvtdq2ps %1, %1 > > +minps %1, %2 > > +cvtps2dq %1, %1 > > +%endif > > %endmacro > > It needs to be clearly documented that, for the sse2 versions, src is > float not dword. Does it make sense to keep it in the same macro then? I also notice that you sent a patch port PMINSD/PMAXSD to cpuflags some time ago, but it was never applied ... Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
On Fri, Nov 16, 2012 at 10:33:38AM +, Måns Rullgård wrote: > Diego Biurrun writes: > > On Fri, Nov 16, 2012 at 10:06:16AM +, Måns Rullgård wrote: > >> Diego Biurrun writes: > >> > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: > >> >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: > >> >> > Now the difference between "all" and "everything" seems arbitrary. > >> >> > Why don't we just set clear semantics on what we call "parts" and > >> >> > "components" or "components" and "subcomponents"? > >> >> > >> >> From an usability point of view you are not going to change that option, > >> >> the best is to clarify it making so from reading --help you would not > >> >> expect it to disable something it does not. > >> > > >> > The usability is what I am trying to fix here, among other things. > >> > Having > >> > both --disable-everything and --disable-all as options is a usability > >> > nightmare. I'd have to look up which option did what myself in a few > >> > weeks time after implementing them... > >> > > >> > So what's bad about --disable-components or --disable-subcomponents? > >> > More importantly, what about those names is worse than what we have > >> > right now: --disable-everything? > >> > >> For better or worse, we have --disable-everything now, and people are > >> using it. Changing it to an equally arbitrary name will only make those > >> people angry. > > > > I suspect that I am one of the main users of that option, but anyway.. > > Your remark misses the context of my proposal, which does not eliminate > > --disable-everything. > > It changes the meaning of it, which is even worse. The semantics of --disable-everything are broken to begin with, so this hardly counts. I'm missing your suggestion of what you want things to look like as opposed to what color of part of my proposal you dislike. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] configure: Add separate list for libraries and use where appropriate
On Fri, Nov 16, 2012 at 10:34:37AM +, Måns Rullgård wrote: > Diego Biurrun writes: > > On Fri, Nov 16, 2012 at 10:05:13AM +, Måns Rullgård wrote: > >> Diego Biurrun writes: > >> > --- a/configure > >> > +++ b/configure > >> > @@ -980,6 +980,15 @@ COMPONENT_LIST=" > >> > > >> > +LIBRARY_LIST=" > >> > +avcodec > >> > +avdevice > >> > +avfilter > >> > +avformat > >> > +avresample > >> > +swscale > >> > +" > >> > @@ -1782,13 +1786,8 @@ host_os=$target_os_default > >> > > >> > -enable avcodec > >> > -enable avdevice > >> > -enable avfilter > >> > -enable avformat > >> > -enable avresample > >> > +enable $LIBRARY_LIST > >> > enable avutil > >> > -enable swscale > >> > >> Why is avutil not in that list? > > > > Because libavutil is a necessary part of any build and can never be > > left out. > > It still leaves the code looking odd, with all uses of $LIBRARY_LIST > followed by an explicit avutil. Is the patch OK with avutil in that list? That would entail making avutil a normal config option as opposed to one not available on the command line, as it is currently. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
On 11/16/2012 10:14 AM, Diego Biurrun wrote: > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: >>> Now the difference between "all" and "everything" seems arbitrary. >>> Why don't we just set clear semantics on what we call "parts" and >>> "components" or "components" and "subcomponents"? >> >> From an usability point of view you are not going to change that option, >> the best is to clarify it making so from reading --help you would not >> expect it to disable something it does not. > > The usability is what I am trying to fix here, among other things. Having > both --disable-everything and --disable-all as options is a usability > nightmare. I'd have to look up which option did what myself in a few > weeks time after implementing them... As I said, sadly, we can do anything but change the name or the effect of --disable-everything =\ lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] configure: Add separate list for libraries and use where appropriate
Diego Biurrun writes: > On Fri, Nov 16, 2012 at 10:05:13AM +, Måns Rullgård wrote: >> Diego Biurrun writes: >> > --- a/configure >> > +++ b/configure >> > @@ -980,6 +980,15 @@ COMPONENT_LIST=" >> > >> > +LIBRARY_LIST=" >> > +avcodec >> > +avdevice >> > +avfilter >> > +avformat >> > +avresample >> > +swscale >> > +" >> > @@ -1782,13 +1786,8 @@ host_os=$target_os_default >> > >> > -enable avcodec >> > -enable avdevice >> > -enable avfilter >> > -enable avformat >> > -enable avresample >> > +enable $LIBRARY_LIST >> > enable avutil >> > -enable swscale >> >> Why is avutil not in that list? > > Because libavutil is a necessary part of any build and can never be > left out. It still leaves the code looking odd, with all uses of $LIBRARY_LIST followed by an explicit avutil. -- Måns Rullgård m...@mansr.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
Diego Biurrun writes: > On Fri, Nov 16, 2012 at 10:06:16AM +, Måns Rullgård wrote: >> Diego Biurrun writes: >> > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: >> >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: >> >> > Now the difference between "all" and "everything" seems arbitrary. >> >> > Why don't we just set clear semantics on what we call "parts" and >> >> > "components" or "components" and "subcomponents"? >> >> >> >> From an usability point of view you are not going to change that option, >> >> the best is to clarify it making so from reading --help you would not >> >> expect it to disable something it does not. >> > >> > The usability is what I am trying to fix here, among other things. Having >> > both --disable-everything and --disable-all as options is a usability >> > nightmare. I'd have to look up which option did what myself in a few >> > weeks time after implementing them... >> > >> > So what's bad about --disable-components or --disable-subcomponents? >> > More importantly, what about those names is worse than what we have >> > right now: --disable-everything? >> >> For better or worse, we have --disable-everything now, and people are >> using it. Changing it to an equally arbitrary name will only make those >> people angry. > > I suspect that I am one of the main users of that option, but anyway.. > Your remark misses the context of my proposal, which does not eliminate > --disable-everything. It changes the meaning of it, which is even worse. -- Måns Rullgård m...@mansr.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] configure: Add separate list for libraries and use where appropriate
On Fri, Nov 16, 2012 at 10:05:13AM +, Måns Rullgård wrote: > Diego Biurrun writes: > > --- a/configure > > +++ b/configure > > @@ -980,6 +980,15 @@ COMPONENT_LIST=" > > > > +LIBRARY_LIST=" > > +avcodec > > +avdevice > > +avfilter > > +avformat > > +avresample > > +swscale > > +" > > @@ -1782,13 +1786,8 @@ host_os=$target_os_default > > > > -enable avcodec > > -enable avdevice > > -enable avfilter > > -enable avformat > > -enable avresample > > +enable $LIBRARY_LIST > > enable avutil > > -enable swscale > > Why is avutil not in that list? Because libavutil is a necessary part of any build and can never be left out. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
On Fri, Nov 16, 2012 at 10:06:16AM +, Måns Rullgård wrote: > Diego Biurrun writes: > > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: > >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: > >> > Now the difference between "all" and "everything" seems arbitrary. > >> > Why don't we just set clear semantics on what we call "parts" and > >> > "components" or "components" and "subcomponents"? > >> > >> From an usability point of view you are not going to change that option, > >> the best is to clarify it making so from reading --help you would not > >> expect it to disable something it does not. > > > > The usability is what I am trying to fix here, among other things. Having > > both --disable-everything and --disable-all as options is a usability > > nightmare. I'd have to look up which option did what myself in a few > > weeks time after implementing them... > > > > So what's bad about --disable-components or --disable-subcomponents? > > More importantly, what about those names is worse than what we have > > right now: --disable-everything? > > For better or worse, we have --disable-everything now, and people are > using it. Changing it to an equally arbitrary name will only make those > people angry. I suspect that I am one of the main users of that option, but anyway.. Your remark misses the context of my proposal, which does not eliminate --disable-everything. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
Diego Biurrun writes: > On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: >> On 11/14/2012 02:38 PM, Diego Biurrun wrote: >> > Now the difference between "all" and "everything" seems arbitrary. >> > Why don't we just set clear semantics on what we call "parts" and >> > "components" or "components" and "subcomponents"? >> >> From an usability point of view you are not going to change that option, >> the best is to clarify it making so from reading --help you would not >> expect it to disable something it does not. > > The usability is what I am trying to fix here, among other things. Having > both --disable-everything and --disable-all as options is a usability > nightmare. I'd have to look up which option did what myself in a few > weeks time after implementing them... > > So what's bad about --disable-components or --disable-subcomponents? > More importantly, what about those names is worse than what we have > right now: --disable-everything? For better or worse, we have --disable-everything now, and people are using it. Changing it to an equally arbitrary name will only make those people angry. -- Måns Rullgård m...@mansr.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] configure: Add separate list for libraries and use where appropriate
Diego Biurrun writes: > --- > configure | 33 + > 1 files changed, 13 insertions(+), 20 deletions(-) > > diff --git a/configure b/configure > index 2894d6f..57a3891 100755 > --- a/configure > +++ b/configure > @@ -980,6 +980,15 @@ COMPONENT_LIST=" > protocols > " > > +LIBRARY_LIST=" > +avcodec > +avdevice > +avfilter > +avformat > +avresample > +swscale > +" > + > @@ -1782,13 +1786,8 @@ host_os=$target_os_default > # configurable options > enable $PROGRAM_LIST > > -enable avcodec > -enable avdevice > -enable avfilter > -enable avformat > -enable avresample > +enable $LIBRARY_LIST > enable avutil > -enable swscale Why is avutil not in that list? -- Måns Rullgård m...@mansr.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] configure: Rename --disable-everything --> --disable-components
On Wed, Nov 14, 2012 at 02:56:05PM +0100, Luca Barbato wrote: > On 11/14/2012 02:38 PM, Diego Biurrun wrote: > > Now the difference between "all" and "everything" seems arbitrary. > > Why don't we just set clear semantics on what we call "parts" and > > "components" or "components" and "subcomponents"? > > From an usability point of view you are not going to change that option, > the best is to clarify it making so from reading --help you would not > expect it to disable something it does not. The usability is what I am trying to fix here, among other things. Having both --disable-everything and --disable-all as options is a usability nightmare. I'd have to look up which option did what myself in a few weeks time after implementing them... So what's bad about --disable-components or --disable-subcomponents? More importantly, what about those names is worse than what we have right now: --disable-everything? Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/2] configure: Add separate list for libraries and use where appropriate
On Sun, Nov 11, 2012 at 02:59:06PM +0100, Diego Biurrun wrote: > --- > configure | 33 + > 1 files changed, 13 insertions(+), 20 deletions(-) ping Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 41/45] x86: Rename SPLATD macro to SPLATD_OFFSET
On Wed, Aug 01, 2012 at 12:18:05AM +0200, Diego Biurrun wrote: > This will allow porting the optimized versions to cpuflags. > --- > libavutil/x86/x86util.asm |2 +- > libswscale/x86/output.asm |2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) ping - trivial and blocks the porting of SPLATD to cpuflags .. Diego ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel