Re: [FFmpeg-devel] [PATCH] add hds demuxer
On Sat, 15 Oct 2016 15:52:48 +0200 Nicolas George wrote: > Le tridi 23 vendémiaire, an CCXXV, wm4 a écrit : > > XML is very complex > > This is the usual, and often only, argument raised in this kind of > situation. And unfortunately, I already addressed it in this very thread. > > XML is very complex, yes: processing instructions, entity definitions, DTSs > and schemas, namespaces, etc. > > Fortunately, we do not need that. Manifests only use the most basic XML > features: elements, attributes, text. Parsing that is easy. > > Regards, > Well, I thought the idea was already rejected by multiple people before. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] ogg kate text subtitles support (patch)
Hello,
Given the practical constraints I can not thoroughly fulfill all the
requirements for submitting a patch. I hope it can make it at least to
the list archive, for possible future perusal by someone.
The patch addresses the missing Kate subtitles support, reflected
in trac as
https://trac.ffmpeg.org/ticket/3039
It is based on the patch present in the kate libraries as of 2011-10-12
(https://git.xiph.org/?p=users/oggk/kate.git)
The attached version applies cleanly to ffmpeg-3.1.3 and does not introduce
any dependencies.
It makes ffmpeg and the programs using ffmpeg, like mplayer,
properly recognize and decode ogg kate text subtitles.
Thanks for the excellent ffmpeg software.
This is my attempt to make a small contribution, for a minor but
regrettably missing and inexpensive feature.
Regards,
Rune
--- a/libavformat/Makefile 2016-10-07 15:15:21.486265907 +0200
+++ b/libavformat/Makefile 2016-10-07 15:17:01.003589996 +0200
@@ -326,6 +326,7 @@
oggparsedaala.o \
oggparsedirac.o \
oggparseflac.o \
+oggparsekate.o \
oggparseogm.o\
oggparseopus.o \
oggparseskeleton.o \
--- a/libavformat/oggdec.c 2016-10-07 19:41:38.423750182 +0200
+++ b/libavformat/oggdec.c 2016-10-07 20:25:41.479354827 +0200
@@ -50,6 +50,7 @@
&ff_celt_codec,
&ff_opus_codec,
&ff_vp8_codec,
+&ff_kate_codec,
&ff_old_dirac_codec,
&ff_old_flac_codec,
&ff_ogm_video_codec,
@@ -146,6 +147,7 @@
os->bufpos = 0;
os->pstart = 0;
os->psize = 0;
+os->skip = 0;
os->granule= -1;
os->lastpts= AV_NOPTS_VALUE;
os->lastdts= AV_NOPTS_VALUE;
@@ -568,7 +570,7 @@
*dsize = os->psize;
if (fpos)
*fpos = os->sync_pos;
-os->pstart += os->psize;
+os->pstart += os->psize+os->skip;
os->psize= 0;
if(os->pstart == os->bufpos)
os->bufpos = os->pstart = 0;
@@ -607,7 +609,7 @@
size = avio_size(s->pb);
if (size < 0)
return 0;
-end = size > MAX_PAGE_SIZE ? size - MAX_PAGE_SIZE : 0;
+end = size > MAX_PAGE_SIZE ? size - MAX_PAGE_SIZE : size;
ret = ogg_save(s);
if (ret < 0)
@@ -797,6 +799,9 @@
return ret;
} while (idx < 0 || !s->streams[idx]);
+if (psize == 0)
+return 0;
+
ogg = s->priv_data;
os = ogg->streams + idx;
--- a/libavformat/oggdec.h 2016-10-07 19:55:24.486255449 +0200
+++ b/libavformat/oggdec.h 2016-10-07 20:32:56.265793913 +0200
@@ -65,7 +65,8 @@
unsigned int pstart;
unsigned int psize;
unsigned int pflags;
-unsigned int pduration;
+uint64_t pduration;
+unsigned int skip;
uint32_t serial;
uint64_t granule;
uint64_t start_granule;
@@ -129,6 +130,7 @@
extern const struct ogg_codec ff_theora_codec;
extern const struct ogg_codec ff_vorbis_codec;
extern const struct ogg_codec ff_vp8_codec;
+extern const struct ogg_codec ff_kate_codec;
int ff_vorbis_comment(AVFormatContext *ms, AVDictionary **m,
const uint8_t *buf, int size, int parse_picture);
--- /dev/null 2015-11-25 12:33:48.239947234 +0100
+++ b/libavformat/oggparsekate.c2016-10-08 14:18:28.251729834 +0200
@@ -0,0 +1,252 @@
+/*
+ *Copyright (C) 2009 ogg.k.og...@googlemail.com
+ *adjustments 2016 by Rl @ Aetey Global Technologies AB
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include
+#include
+#include "libavutil/intreadwrite.h"
+#include "libavcodec/bytestream.h"
+#include "avlanguage.h"
+#include "avformat.h"
+#include "internal.h"
+#include "oggdec.h"
+
+struct kate_params {
+int major, minor;
+uint32_t gps_num;
+uint32_t gps_den;
+int granule_shift;
+
+int canvas_width;
+int canvas_height;
+
+int num_headers;
+int text_encoding;
+int directionality;
+
+char language[16];
+char category[16];
+};
+
+
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Sun, 23 Oct 2016 13:02:01 -0400 "Ronald S. Bultje" wrote: > Hi, > > On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer < > mich...@niedermayer.cc> wrote: > > > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote: > > > Hi, > > > > > > general comment about all other dec patches. > > > > > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer > > > > > wrote: > > > > > > > Signed-off-by: Michael Niedermayer > > > > --- > > > > libavcodec/svq1dec.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c > > > > index 2b72e08..0fe222e 100644 > > > > --- a/libavcodec/svq1dec.c > > > > +++ b/libavcodec/svq1dec.c > > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecContext > > *avctx, > > > > void *data, > > > > } > > > > } > > > > } > > > > +emms_c(); > > > > > > > > > This is hideous, you're sprinkling emms_c in various places to make some > > > stupid test pass. The test is morbidly stupid and there is no general > > > consensus on patterns to be followed as for where to place emms_c. > > Someone > > > who doesn't know any better will litter each new decoder with 10-20 calls > > > to emms_c just because he found that other decoders do it in > > undocumented, > > > unexplained and unclear locations also. > > > > > > If you want this to be a "thing", you need to design and document > > carefully > > > where emms_c is necessary. Then come up with some system that makes this > > > work by itself. > > > > Your mail sounds quite aggressive to me, did i say something to anger > > you ? It was certainly not intended > > > > About this patchset > > FFmpeg is broken ATM (both in theory and practice with some libcs), > > the way MMX code is used is not correct, emms_c() > > calls are missing and required. The obvious way to fix that > > (in practice) is adding emms_c() calls where they are missing. > > I can document why each call is needed, if thats wanted? > > > Your representation of facts is strange, to say the least. Let's explore > two related claims: > > - (A) ffmpeg is broken in practice when calling musl malloc/free functions > after calling MMX SIMD functions on x86-32 (which crashes). > - (B) ffmpeg is broken in theory because we don't clear FPU state (as > required) at the end of MMX SIMD functions. I was under the impression that it is UB to have the FPU in MMX state at any time while in C, not just while e.g. calling the stdlib. Maybe I got that wrong (how would MMX intrinsics even work?) - can anyone shed light on the exact requirements? (Possibly again, sorry.) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Sun, 23 Oct 2016 05:37:25 +0200
Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer
> ---
> libavutil/x86/emms.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> index 6fda6e2..42c18e2 100644
> --- a/libavutil/x86/emms.h
> +++ b/libavutil/x86/emms.h
> @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> * Empty mmx state.
> * this must be called between any dsp function and float/double code.
> * for example sin(); dsp->idct_put(); emms_c(); cos()
> + * Note, *alloc() and *free() also use float code in some libc
> implementations
> + * thus this also applies to them or any function using them.
> */
> static av_always_inline void emms_c(void)
> {
Overly specific and useless information. It's an implementation detail
of 1 specific libc. It could happen to any libc function for any libc
at any time. This just adds noise because of one specific bug.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/13] avcodec: Clear MMX state in ff_thread_report_progress(INT_MAX)
On Sat, 22 Oct 2016 22:06:22 -0400 "Ronald S. Bultje" wrote: > Hi, > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer > wrote: > > > This decreases the number of FPU state violations in fate on x86-64 from > > 309 to 79 > > > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/pthread_frame.c | 3 +++ > > libavcodec/utils.c | 2 ++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > > index 7ef5e9f..b6c6027 100644 > > --- a/libavcodec/pthread_frame.c > > +++ b/libavcodec/pthread_frame.c > > @@ -473,6 +473,9 @@ void ff_thread_report_progress(ThreadFrame *f, int n, > > int field) > > PerThreadContext *p; > > volatile int *progress = f->progress ? (int*)f->progress->data : NULL; > > > > +if (n >= INT_MAX) > > +emms_c(); > > > I don't like how INT_MAX is becoming more and more like a magic number. > This should probably be a new function that is called upon frame (or field) > decoding completion (or abort), where part of what it does is to set > progress to INT_MAX. Indeed, this looks very strange. Also the ">" in the ">=" is redundant, why isn't it "=="? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] lavfi: add FFFrameQueue API.
On Sun, 23 Oct 2016 12:27:41 +0200
Nicolas George wrote:
> Signed-off-by: Nicolas George
> ---
> libavfilter/Makefile | 1 +
> libavfilter/framequeue.c | 123 +
> libavfilter/framequeue.h | 173
> +++
> 3 files changed, 297 insertions(+)
> create mode 100644 libavfilter/framequeue.c
> create mode 100644 libavfilter/framequeue.h
>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 7ed4696..623dd8e 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -18,6 +18,7 @@ OBJS = allfilters.o
> \
> fifo.o \
> formats.o\
> framepool.o \
> + framequeue.o \
> graphdump.o \
> graphparser.o\
> opencl_allkernels.o \
> diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c
> new file mode 100644
> index 000..debeab2
> --- /dev/null
> +++ b/libavfilter/framequeue.c
> @@ -0,0 +1,123 @@
> +/*
> + * Generic frame queue
> + * Copyright (c) 2016 Nicolas George
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "framequeue.h"
> +
> +static inline FFFrameBucket *bucket(FFFrameQueue *fq, size_t idx)
> +{
> +return &fq->queue[(fq->tail + idx) & (fq->allocated - 1)];
> +}
> +
> +void ff_framequeue_global_init(FFFrameQueueGlobal *fqg)
> +{
> +}
> +
> +static void check_consistency(FFFrameQueue *fq)
> +{
> +#if ASSERT_LEVEL >= 2
> +uint64_t nb_samples = 0;
> +size_t i;
> +
> +av_assert0(fq->queued == fq->total_frames_head - fq->total_frames_tail);
> +for (i = 0; i < fq->queued; i++)
> +nb_samples += bucket(fq, i)->frame->nb_samples;
> +av_assert0(nb_samples == fq->total_samples_head -
> fq->total_samples_tail);
> +#endif
> +}
> +
> +void ff_framequeue_init(FFFrameQueue *fq, FFFrameQueueGlobal *fqg)
> +{
> +fq->queue = &fq->first_bucket;
> +fq->allocated = 1;
> +}
> +
> +void ff_framequeue_free(FFFrameQueue *fq)
> +{
> +while (fq->queued) {
> +AVFrame *frame = ff_framequeue_take(fq);
> +av_frame_free(&frame);
> +}
> +if (fq->queue != &fq->first_bucket)
> +av_freep(&fq->queue);
> +}
> +
> +int ff_framequeue_add(FFFrameQueue *fq, AVFrame *frame)
> +{
> +FFFrameBucket *b;
> +
> +check_consistency(fq);
> +if (fq->queued == fq->allocated) {
> +if (fq->allocated == 1) {
> +size_t na = 8;
> +FFFrameBucket *nq = av_realloc_array(NULL, na, sizeof(*nq));
> +if (!nq)
> +return AVERROR(ENOMEM);
> +nq[0] = fq->queue[0];
> +fq->queue = nq;
> +fq->allocated = na;
> +} else {
> +size_t na = fq->allocated << 1;
> +FFFrameBucket *nq = av_realloc_array(fq->queue, na, sizeof(*nq));
> +if (!nq)
> +return AVERROR(ENOMEM);
> +if (fq->tail + fq->queued > fq->allocated)
> +memmove(nq + fq->allocated, nq,
> +(fq->tail + fq->queued - fq->allocated) *
> sizeof(*nq));
> +fq->queue = nq;
> +fq->allocated = na;
> +}
> +}
> +b = bucket(fq, fq->queued);
> +b->frame = frame;
> +fq->queued++;
> +fq->total_frames_head++;
> +fq->total_samples_head += frame->nb_samples;
> +check_consistency(fq);
> +return 0;
> +}
> +
> +AVFrame *ff_framequeue_take(FFFrameQueue *fq)
> +{
> +FFFrameBucket *b;
> +
> +check_consistency(fq);
> +av_assert1(fq->queued);
> +b = bucket(fq, 0);
> +fq->queued--;
> +fq->tail++;
> +fq->tail &= fq->allocated - 1;
> +fq->total_frames_tail++;
> +fq->total_samples_tail += b->frame->nb_samples;
> +check_consistency(fq);
> +return b->frame;
> +}
> +
> +AVFrame *
Re: [FFmpeg-devel] [PATCH 02/12] bfi: validate sample_rate
On Sun, 23 Oct 2016 18:27:02 +0200
Andreas Cadhalpun wrote:
> A negative sample rate doesn't make sense and triggers assertions in
> av_rescale_rnd.
>
> Signed-off-by: Andreas Cadhalpun
> ---
> libavformat/bfi.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/libavformat/bfi.c b/libavformat/bfi.c
> index 568363d..ef4c17d 100644
> --- a/libavformat/bfi.c
> +++ b/libavformat/bfi.c
> @@ -88,6 +88,10 @@ static int bfi_read_header(AVFormatContext * s)
> vstream->codecpar->extradata_size);
>
> astream->codecpar->sample_rate = avio_rl32(pb);
> +if (astream->codecpar->sample_rate <= 0) {
> +av_log(s, AV_LOG_ERROR, "Invalid sample rate %d\n",
> astream->codecpar->sample_rate);
> +return AVERROR_INVALIDDATA;
> +}
>
> /* Set up the video codec... */
> avpriv_set_pts_info(vstream, 32, 1, fps);
Would it make sense to validate codecpars in the generic code (utils.c)?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, 24 Oct 2016 09:31:25 +0200 u-9...@aetey.se wrote: > Hello, > > Given the practical constraints I can not thoroughly fulfill all the > requirements for submitting a patch. I hope it can make it at least to > the list archive, for possible future perusal by someone. > > The patch addresses the missing Kate subtitles support, reflected > in trac as >https://trac.ffmpeg.org/ticket/3039 > > It is based on the patch present in the kate libraries as of 2011-10-12 > (https://git.xiph.org/?p=users/oggk/kate.git) > > The attached version applies cleanly to ffmpeg-3.1.3 and does not introduce > any dependencies. > > It makes ffmpeg and the programs using ffmpeg, like mplayer, > properly recognize and decode ogg kate text subtitles. > > Thanks for the excellent ffmpeg software. > > This is my attempt to make a small contribution, for a minor but > regrettably missing and inexpensive feature. > > Regards, > Rune I don't quite get it. Doesn't this just demux kate subtitles as text without stripping whatever formatting tags kate might support? (I was under the impression that kate does more than just plain text.) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 10:00:38AM +0200, wm4 wrote: > > The patch addresses the missing Kate subtitles support, reflected > > in trac as > >https://trac.ffmpeg.org/ticket/3039 > I don't quite get it. Doesn't this just demux kate subtitles as text > without stripping whatever formatting tags kate might support? (I was > under the impression that kate does more than just plain text.) Kate can of course do much more than plain text, but this patch only adds the recognition of ogg-kate-text, nothing else. That's what makes it a low hanging fruit. At the same time we get a remarkable improvement, compared to the complete inability to handle subtitles in ogg. Regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] add hds demuxer
2016-10-24 15:22 GMT+08:00 wm4 : > On Sat, 15 Oct 2016 15:52:48 +0200 > Nicolas George wrote: > > > Le tridi 23 vendémiaire, an CCXXV, wm4 a écrit : > > > XML is very complex > > > > This is the usual, and often only, argument raised in this kind of > > situation. And unfortunately, I already addressed it in this very thread. > > > > XML is very complex, yes: processing instructions, entity definitions, > DTSs > > and schemas, namespaces, etc. > > > > Fortunately, we do not need that. Manifests only use the most basic XML > > features: elements, attributes, text. Parsing that is easy. > > > > Regards, > > > > Well, I thought the idea was already rejected by multiple people before. > But i think Nicolas George's idea is useful now, for example used in hds mkv and some format depend on simple xml format. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
worth to mention: On Mon, Oct 24, 2016 at 10:25:44AM +0200, u-9...@aetey.se wrote: > > >https://trac.ffmpeg.org/ticket/3039 The ticket refers to a sample with graphical subtitles. This is _not_ being solved by the proposed patch. A sample with text subtitles can be seen e.g. at http://people.xiph.org/~oggk/elephants_dream/elephantsdream-with-subtitles.ogg It is well decodable with the patch applied. Text subtitles in ogg kate are a straightforward way to put srt-like data into ogg. Multiplexing kate into ogg is not addressed by the proposed patch, but is otherwise easily done by external tools. Such tools are not applicable as much during playback, that's why demuxing support is most important. Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, 24 Oct 2016 10:52:14 +0200 u-9...@aetey.se wrote: > worth to mention: > > On Mon, Oct 24, 2016 at 10:25:44AM +0200, u-9...@aetey.se wrote: > > > >https://trac.ffmpeg.org/ticket/3039 > > The ticket refers to a sample with graphical subtitles. This is _not_ > being solved by the proposed patch. > > A sample with text subtitles can be seen e.g. at > > http://people.xiph.org/~oggk/elephants_dream/elephantsdream-with-subtitles.ogg > It is well decodable with the patch applied. > > Text subtitles in ogg kate are a straightforward way to put srt-like data Note that srt supports simple HTML-like tags. > into ogg. Multiplexing kate into ogg is not addressed by the proposed > patch, but is otherwise easily done by external tools. Such tools are > not applicable as much during playback, that's why demuxing support > is most important. > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 11:09:43AM +0200, wm4 wrote: > > Text subtitles in ogg kate are a straightforward way to put srt-like data > > Note that srt supports simple HTML-like tags. Yes, you are right, the patch just ignores the possible presence of Kate markup (does not try to interpret it, nor removes). This is probably not too bad, for the minimal support. Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
2016-10-24 10:52 GMT+02:00 : > worth to mention: > > On Mon, Oct 24, 2016 at 10:25:44AM +0200, u-9...@aetey.se wrote: >> > >https://trac.ffmpeg.org/ticket/3039 > > The ticket refers to a sample with graphical subtitles. This is _not_ > being solved by the proposed patch. > > A sample with text subtitles can be seen e.g. at > > http://people.xiph.org/~oggk/elephants_dream/elephantsdream-with-subtitles.ogg > It is well decodable with the patch applied. Thank you for explaining this! > Text subtitles in ogg kate are a straightforward way to put srt-like data > into ogg. Multiplexing kate into ogg is not addressed by the proposed > patch, but is otherwise easily done by external tools. Such tools are > not applicable as much during playback, that's why demuxing support > is most important. +1 Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 11:39:40AM +0200, u-9...@aetey.se wrote: > On Mon, Oct 24, 2016 at 11:09:43AM +0200, wm4 wrote: > > > Text subtitles in ogg kate are a straightforward way to put srt-like data > > > > Note that srt supports simple HTML-like tags. > > Yes, you are right, the patch just ignores the possible presence > of Kate markup (does not try to interpret it, nor removes). > This is probably not too bad, for the minimal support. > It is bad if you don't strip the markup in the decoder. -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 11:46:07AM +0200, Clément Bœsch wrote: > On Mon, Oct 24, 2016 at 11:39:40AM +0200, u-9...@aetey.se wrote: > > On Mon, Oct 24, 2016 at 11:09:43AM +0200, wm4 wrote: > > > > Text subtitles in ogg kate are a straightforward way to put srt-like > > > > data > > > > > > Note that srt supports simple HTML-like tags. > > > > Yes, you are right, the patch just ignores the possible presence > > of Kate markup (does not try to interpret it, nor removes). > > This is probably not too bad, for the minimal support. > > > > It is bad if you don't strip the markup in the decoder. > To expand on this: it is extremely worrisome that ffmpeg could be the source of yet another wave of insane .srt files with kate markup in it because someone decided to run ffmpeg -i in.ogg out.srt in order to extract the subtitles and share them. You absolutely want to strip all markup in the decoder for minimal support, or FFmpeg will generate broken files, which is not tolerable. -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 11:46:07AM +0200, Clément Bœsch wrote: > On Mon, Oct 24, 2016 at 11:39:40AM +0200, u-9...@aetey.se wrote: > > Yes, you are right, the patch just ignores the possible presence > > of Kate markup (does not try to interpret it, nor removes). > > This is probably not too bad, for the minimal support. > > It is bad if you don't strip the markup in the decoder. It shouldn't be hard (the kate_text_remove_markup() function in libkate is just 49 lines) but was not included in the original patch. Could be added later if desired, either as an incremental patch or by introducing an optional dependency on libkate (which would open for full Kate functionality). Regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 11:58:41AM +0200, u-9...@aetey.se wrote: [...] > Could be added later if desired, either as an incremental patch or by > introducing an optional dependency on libkate (which would open for full > Kate functionality). No, it's a blocking requirement (see my next mail), it needs to be included in the initial support. Regards, -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 11:49:25AM +0200, Clément Bœsch wrote: > > It is bad if you don't strip the markup in the decoder. > To expand on this: it is extremely worrisome that ffmpeg could be the > source of yet another wave of insane .srt files with kate markup in it > because someone decided to run ffmpeg -i in.ogg out.srt in order to > extract the subtitles and share them. > > You absolutely want to strip all markup in the decoder for minimal > support, or FFmpeg will generate broken files, which is not tolerable. I see and have to agree. (even though I would not expect any remarkable harm to happen in practice) Regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 12:12:23PM +0200, u-9...@aetey.se wrote: > On Mon, Oct 24, 2016 at 11:49:25AM +0200, Clément Bœsch wrote: > > > It is bad if you don't strip the markup in the decoder. > > > To expand on this: it is extremely worrisome that ffmpeg could be the > > source of yet another wave of insane .srt files with kate markup in it > > because someone decided to run ffmpeg -i in.ogg out.srt in order to > > extract the subtitles and share them. > > > > You absolutely want to strip all markup in the decoder for minimal > > support, or FFmpeg will generate broken files, which is not tolerable. > > I see and have to agree. > Thanks. > (even though I would not expect any remarkable harm to happen in practice) > Today, in practice, we have to support all kind of mix of subtitles markup in various formats because of this. It would be great not to make things worst :) -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> On Sun, 23 Oct 2016 05:37:25 +0200
> Michael Niedermayer wrote:
>
> > Signed-off-by: Michael Niedermayer
> > ---
> > libavutil/x86/emms.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > index 6fda6e2..42c18e2 100644
> > --- a/libavutil/x86/emms.h
> > +++ b/libavutil/x86/emms.h
> > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > * Empty mmx state.
> > * this must be called between any dsp function and float/double code.
> > * for example sin(); dsp->idct_put(); emms_c(); cos()
> > + * Note, *alloc() and *free() also use float code in some libc
> > implementations
> > + * thus this also applies to them or any function using them.
> > */
> > static av_always_inline void emms_c(void)
> > {
>
> Overly specific and useless information. It's an implementation detail
If we place emms_c() so as to ensure that the FPU state is clean
before all calls to *alloc() and *free() (as is done in the posted
patchset) then we need to document this
so others working on the code are aware of it and wont mistakly break
it.
If/when a different or more complete solution is implemented then this
note needs to be adjusted accordingly.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 12:18:23PM +0200, Clément Bœsch wrote: > Today, in practice, we have to support all kind of mix of subtitles markup > in various formats because of this. It would be great not to make things > worst :) Let's see. Looking at kateenc code I believe that the intention was to keep Kate "compatible with SRT markup" whatever it means. The encoder does not interpret/recode markup between srt and kate. What it does is to handle the metainformation about the presence of markup (which presence or absence is explicit in Kate). So we will quite certainly do the users a favor by _not_ cleaning the possibly present markup tags, when converting from kate to srt. Does this sound good enough? Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
Hi,
On Mon, Oct 24, 2016 at 7:24 AM, Michael Niedermayer wrote:
> On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > On Sun, 23 Oct 2016 05:37:25 +0200
> > Michael Niedermayer wrote:
> >
> > > Signed-off-by: Michael Niedermayer
> > > ---
> > > libavutil/x86/emms.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > index 6fda6e2..42c18e2 100644
> > > --- a/libavutil/x86/emms.h
> > > +++ b/libavutil/x86/emms.h
> > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > > * Empty mmx state.
> > > * this must be called between any dsp function and float/double code.
> > > * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > + * Note, *alloc() and *free() also use float code in some libc
> implementations
> > > + * thus this also applies to them or any function using them.
> > > */
> > > static av_always_inline void emms_c(void)
> > > {
> >
> > Overly specific and useless information. It's an implementation detail
>
> If we place emms_c() so as to ensure that the FPU state is clean
> before all calls to *alloc() and *free() (as is done in the posted
> patchset) then we need to document this
> so others working on the code are aware of it and wont mistakly break
> it.
>
> If/when a different or more complete solution is implemented then this
> note needs to be adjusted accordingly.
I am doubtful that the patches that implement the partial solution
("hack"?) should be pushed, or that we should (by documenting it) advocate
partial solutions ("hacks") in general.
Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
Hi, On Mon, Oct 24, 2016 at 3:36 AM, wm4 wrote: > On Sun, 23 Oct 2016 13:02:01 -0400 > "Ronald S. Bultje" wrote: > > > Hi, > > > > On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer < > > mich...@niedermayer.cc> wrote: > > > > > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote: > > > > Hi, > > > > > > > > general comment about all other dec patches. > > > > > > > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer > > > > > > > wrote: > > > > > > > > > Signed-off-by: Michael Niedermayer > > > > > --- > > > > > libavcodec/svq1dec.c | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c > > > > > index 2b72e08..0fe222e 100644 > > > > > --- a/libavcodec/svq1dec.c > > > > > +++ b/libavcodec/svq1dec.c > > > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecContext > > > *avctx, > > > > > void *data, > > > > > } > > > > > } > > > > > } > > > > > +emms_c(); > > > > > > > > > > > > This is hideous, you're sprinkling emms_c in various places to make > some > > > > stupid test pass. The test is morbidly stupid and there is no general > > > > consensus on patterns to be followed as for where to place emms_c. > > > Someone > > > > who doesn't know any better will litter each new decoder with 10-20 > calls > > > > to emms_c just because he found that other decoders do it in > > > undocumented, > > > > unexplained and unclear locations also. > > > > > > > > If you want this to be a "thing", you need to design and document > > > carefully > > > > where emms_c is necessary. Then come up with some system that makes > this > > > > work by itself. > > > > > > Your mail sounds quite aggressive to me, did i say something to anger > > > you ? It was certainly not intended > > > > > > About this patchset > > > FFmpeg is broken ATM (both in theory and practice with some libcs), > > > the way MMX code is used is not correct, emms_c() > > > calls are missing and required. The obvious way to fix that > > > (in practice) is adding emms_c() calls where they are missing. > > > I can document why each call is needed, if thats wanted? > > > > > > Your representation of facts is strange, to say the least. Let's explore > > two related claims: > > > > - (A) ffmpeg is broken in practice when calling musl malloc/free > functions > > after calling MMX SIMD functions on x86-32 (which crashes). > > - (B) ffmpeg is broken in theory because we don't clear FPU state (as > > required) at the end of MMX SIMD functions. > > I was under the impression that it is UB to have the FPU in MMX state > at any time while in C, not just while e.g. calling the stdlib. Maybe I > got that wrong (how would MMX intrinsics even work?) - can anyone shed > light on the exact requirements? (Possibly again, sorry.) I'm under the impression that it's part of the calling convention. That is, any code anywhere (including mmx intrinsics, indeed) can - when called - expect the state to be cleared by the caller, just like you'd expect eax/edx to be caller-save (whereas esi/edi are callee-save). However, if you never call external code (including intrinsics), you can ignore this, just as you can ignore / create your own calling convention (remember fastcall etc.?). However, when calling any external code, this could (in theory) crash; it's just that right now it only crashes with musl when calling malloc/free. So basically, ffmpeg has its own calling convention, and manually calling emms_c() fixes "ffmpeg" calling convention to be compatible with "standard" calling convention...? Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] ogg kate text subtitles support (patch)
On Mon, Oct 24, 2016 at 01:31:10PM +0200, u-9...@aetey.se wrote: > On Mon, Oct 24, 2016 at 12:18:23PM +0200, Clément Bœsch wrote: > > Today, in practice, we have to support all kind of mix of subtitles markup > > in various formats because of this. It would be great not to make things > > worst :) > > Let's see. > > Looking at kateenc code I believe that the intention was to keep > Kate "compatible with SRT markup" whatever it means. > > The encoder does not interpret/recode markup between srt and kate. What > it does is to handle the metainformation about the presence of markup > (which presence or absence is explicit in Kate). > > So we will quite certainly do the users a favor by _not_ cleaning the > possibly present markup tags, when converting from kate to srt. > > Does this sound good enough? I could replace srt with any other subtitles format. Whether it's Kate or SubRip markup, I don't want it in ASS or MicroDVD. If the Kate markup is the same as SRT, then you should output SubRip packets in the OGG demuxer, and you do not need to write a Kate decoder. Otherwise, you need to do just like the SRT decoder: handle the markup (that is, decode it as the internal representation), or just strip it for a start. -- Clément B. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Mon, 24 Oct 2016 13:24:38 +0200
Michael Niedermayer wrote:
> On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > On Sun, 23 Oct 2016 05:37:25 +0200
> > Michael Niedermayer wrote:
> >
> > > Signed-off-by: Michael Niedermayer
> > > ---
> > > libavutil/x86/emms.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > index 6fda6e2..42c18e2 100644
> > > --- a/libavutil/x86/emms.h
> > > +++ b/libavutil/x86/emms.h
> > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > > * Empty mmx state.
> > > * this must be called between any dsp function and float/double code.
> > > * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > + * Note, *alloc() and *free() also use float code in some libc
> > > implementations
> > > + * thus this also applies to them or any function using them.
> > > */
> > > static av_always_inline void emms_c(void)
> > > {
> >
> > Overly specific and useless information. It's an implementation detail
>
> If we place emms_c() so as to ensure that the FPU state is clean
> before all calls to *alloc() and *free() (as is done in the posted
> patchset) then we need to document this
> so others working on the code are aware of it and wont mistakly break
> it.
>
> If/when a different or more complete solution is implemented then this
> note needs to be adjusted accordingly.
>
> [...]
That fixes only the musl-specific issue. It could happen any time again
with a set of different callers. Unless you want to put special effort
into fixing operation with musl (and keeping it working) it won't help
much to fix the correctness issues.
If anything, it should probably say "all external" calls or such.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Mon, 24 Oct 2016 07:54:47 -0400 "Ronald S. Bultje" wrote: > Hi, > > On Mon, Oct 24, 2016 at 3:36 AM, wm4 wrote: > > > On Sun, 23 Oct 2016 13:02:01 -0400 > > "Ronald S. Bultje" wrote: > > > > > Hi, > > > > > > On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer < > > > mich...@niedermayer.cc> wrote: > > > > > > > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote: > > > > > Hi, > > > > > > > > > > general comment about all other dec patches. > > > > > > > > > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer > > > > > > > > > wrote: > > > > > > > > > > > Signed-off-by: Michael Niedermayer > > > > > > --- > > > > > > libavcodec/svq1dec.c | 2 ++ > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c > > > > > > index 2b72e08..0fe222e 100644 > > > > > > --- a/libavcodec/svq1dec.c > > > > > > +++ b/libavcodec/svq1dec.c > > > > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecContext > > > > *avctx, > > > > > > void *data, > > > > > > } > > > > > > } > > > > > > } > > > > > > +emms_c(); > > > > > > > > > > > > > > > This is hideous, you're sprinkling emms_c in various places to make > > some > > > > > stupid test pass. The test is morbidly stupid and there is no general > > > > > consensus on patterns to be followed as for where to place emms_c. > > > > Someone > > > > > who doesn't know any better will litter each new decoder with 10-20 > > calls > > > > > to emms_c just because he found that other decoders do it in > > > > undocumented, > > > > > unexplained and unclear locations also. > > > > > > > > > > If you want this to be a "thing", you need to design and document > > > > carefully > > > > > where emms_c is necessary. Then come up with some system that makes > > this > > > > > work by itself. > > > > > > > > Your mail sounds quite aggressive to me, did i say something to anger > > > > you ? It was certainly not intended > > > > > > > > About this patchset > > > > FFmpeg is broken ATM (both in theory and practice with some libcs), > > > > the way MMX code is used is not correct, emms_c() > > > > calls are missing and required. The obvious way to fix that > > > > (in practice) is adding emms_c() calls where they are missing. > > > > I can document why each call is needed, if thats wanted? > > > > > > > > > Your representation of facts is strange, to say the least. Let's explore > > > two related claims: > > > > > > - (A) ffmpeg is broken in practice when calling musl malloc/free > > functions > > > after calling MMX SIMD functions on x86-32 (which crashes). > > > - (B) ffmpeg is broken in theory because we don't clear FPU state (as > > > required) at the end of MMX SIMD functions. > > > > I was under the impression that it is UB to have the FPU in MMX state > > at any time while in C, not just while e.g. calling the stdlib. Maybe I > > got that wrong (how would MMX intrinsics even work?) - can anyone shed > > light on the exact requirements? (Possibly again, sorry.) > > > I'm under the impression that it's part of the calling convention. That is, > any code anywhere (including mmx intrinsics, indeed) can - when called - > expect the state to be cleared by the caller, just like you'd expect > eax/edx to be caller-save (whereas esi/edi are callee-save). > > However, if you never call external code (including intrinsics), you can > ignore this, just as you can ignore / create your own calling > convention (remember fastcall etc.?). However, when calling any external > code, this could (in theory) crash; it's just that right now it only > crashes with musl when calling malloc/free. So basically, ffmpeg has its > own calling convention, and manually calling emms_c() fixes "ffmpeg" > calling convention to be compatible with "standard" calling convention...? It can't really be a calling convention unless the compiler is aware of it? We're doing things behind the compiler's back here. The safest assumption would be that leaving the FPU state invalid while in C is not allowed, period. The next safest assumption is that it's fine as long as we explicitly don't use floating point or call external functions that aren't MMX-aware. This would mean calling any libc functions or user callbacks (including indirectly through e.g. av_log) is not fine. Of course this is not as easy to verify as adding a hack-assert to av_malloc/av_free. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
Hi, On Mon, Oct 24, 2016 at 8:47 AM, wm4 wrote: > On Mon, 24 Oct 2016 07:54:47 -0400 > "Ronald S. Bultje" wrote: > > > Hi, > > > > On Mon, Oct 24, 2016 at 3:36 AM, wm4 wrote: > > > > > On Sun, 23 Oct 2016 13:02:01 -0400 > > > "Ronald S. Bultje" wrote: > > > > > > > Hi, > > > > > > > > On Sat, Oct 22, 2016 at 11:36 PM, Michael Niedermayer < > > > > mich...@niedermayer.cc> wrote: > > > > > > > > > On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote: > > > > > > Hi, > > > > > > > > > > > > general comment about all other dec patches. > > > > > > > > > > > > On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer > > > > > > > > > > > wrote: > > > > > > > > > > > > > Signed-off-by: Michael Niedermayer > > > > > > > --- > > > > > > > libavcodec/svq1dec.c | 2 ++ > > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c > > > > > > > index 2b72e08..0fe222e 100644 > > > > > > > --- a/libavcodec/svq1dec.c > > > > > > > +++ b/libavcodec/svq1dec.c > > > > > > > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecConte > xt > > > > > *avctx, > > > > > > > void *data, > > > > > > > } > > > > > > > } > > > > > > > } > > > > > > > +emms_c(); > > > > > > > > > > > > > > > > > > This is hideous, you're sprinkling emms_c in various places to > make > > > some > > > > > > stupid test pass. The test is morbidly stupid and there is no > general > > > > > > consensus on patterns to be followed as for where to place > emms_c. > > > > > Someone > > > > > > who doesn't know any better will litter each new decoder with > 10-20 > > > calls > > > > > > to emms_c just because he found that other decoders do it in > > > > > undocumented, > > > > > > unexplained and unclear locations also. > > > > > > > > > > > > If you want this to be a "thing", you need to design and document > > > > > carefully > > > > > > where emms_c is necessary. Then come up with some system that > makes > > > this > > > > > > work by itself. > > > > > > > > > > Your mail sounds quite aggressive to me, did i say something to > anger > > > > > you ? It was certainly not intended > > > > > > > > > > About this patchset > > > > > FFmpeg is broken ATM (both in theory and practice with some libcs), > > > > > the way MMX code is used is not correct, emms_c() > > > > > calls are missing and required. The obvious way to fix that > > > > > (in practice) is adding emms_c() calls where they are missing. > > > > > I can document why each call is needed, if thats wanted? > > > > > > > > > > > > Your representation of facts is strange, to say the least. Let's > explore > > > > two related claims: > > > > > > > > - (A) ffmpeg is broken in practice when calling musl malloc/free > > > functions > > > > after calling MMX SIMD functions on x86-32 (which crashes). > > > > - (B) ffmpeg is broken in theory because we don't clear FPU state (as > > > > required) at the end of MMX SIMD functions. > > > > > > I was under the impression that it is UB to have the FPU in MMX state > > > at any time while in C, not just while e.g. calling the stdlib. Maybe I > > > got that wrong (how would MMX intrinsics even work?) - can anyone shed > > > light on the exact requirements? (Possibly again, sorry.) > > > > > > I'm under the impression that it's part of the calling convention. That > is, > > any code anywhere (including mmx intrinsics, indeed) can - when called - > > expect the state to be cleared by the caller, just like you'd expect > > eax/edx to be caller-save (whereas esi/edi are callee-save). > > > > However, if you never call external code (including intrinsics), you can > > ignore this, just as you can ignore / create your own calling > > convention (remember fastcall etc.?). However, when calling any external > > code, this could (in theory) crash; it's just that right now it only > > crashes with musl when calling malloc/free. So basically, ffmpeg has its > > own calling convention, and manually calling emms_c() fixes "ffmpeg" > > calling convention to be compatible with "standard" calling > convention...? > > It can't really be a calling convention unless the compiler is aware of > it? > > We're doing things behind the compiler's back here. The safest > assumption would be that leaving the FPU state invalid while in C is > not allowed, period. > > The next safest assumption is that it's fine as long as we explicitly > don't use floating point or call external functions that aren't > MMX-aware. This would mean calling any libc functions or user callbacks > (including indirectly through e.g. av_log) is not fine. > Yes. And while I don't agree with it, I feel we should also mention Carl Eugen's alternative proposal, which is to document that ffmpeg doesn't formally adhere to this calling convention requirement [1]. Of course this is not as easy to verify as adding a hack-assert to > av_malloc/av_free. I agree. (Which is why I insist that the design of this
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Mon, Oct 24, 2016 at 02:36:23PM +0200, wm4 wrote:
> On Mon, 24 Oct 2016 13:24:38 +0200
> Michael Niedermayer wrote:
>
> > On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > > On Sun, 23 Oct 2016 05:37:25 +0200
> > > Michael Niedermayer wrote:
> > >
> > > > Signed-off-by: Michael Niedermayer
> > > > ---
> > > > libavutil/x86/emms.h | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > > index 6fda6e2..42c18e2 100644
> > > > --- a/libavutil/x86/emms.h
> > > > +++ b/libavutil/x86/emms.h
> > > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > > > * Empty mmx state.
> > > > * this must be called between any dsp function and float/double code.
> > > > * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > > + * Note, *alloc() and *free() also use float code in some libc
> > > > implementations
> > > > + * thus this also applies to them or any function using them.
> > > > */
> > > > static av_always_inline void emms_c(void)
> > > > {
> > >
> > > Overly specific and useless information. It's an implementation detail
> >
> > If we place emms_c() so as to ensure that the FPU state is clean
> > before all calls to *alloc() and *free() (as is done in the posted
> > patchset) then we need to document this
> > so others working on the code are aware of it and wont mistakly break
> > it.
> >
> > If/when a different or more complete solution is implemented then this
> > note needs to be adjusted accordingly.
> >
> > [...]
>
> That fixes only the musl-specific issue. It could happen any time again
> with a set of different callers.
yes
> Unless you want to put special effort
> into fixing operation with musl (and keeping it working) it won't help
> much to fix the correctness issues.
i did want to do that, but ive a headache atm and the discussion
about emms yesterday makes me not want to touch emms anymore
>
> If anything, it should probably say "all external" calls or such.
I see this as the long term goal, yes i agree it should be
be documented if theres consensus about it
The way i imagined it was to fix the practically relevant issues and
anything else thats easy or attached (like x86-64 for what is the same
on x86-32) before 3.2 and have 3.2 work with musl on all archs
then aim at fixing the rest over the following months unless some
roadblocks like performance issues are hit.
This incremental approuch has been hit with quite some opposition so
ill leave this to whoever likes to work on this
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 07/11] lavf/movenc+dashenc: add automatic bitstream filtering
On Fri, Sep 09, 2016 at 11:37:21PM -0500, Rodger Combs wrote:
> This is disabled by default when the empty_moov flag is enabled
> ---
> libavformat/dashenc.c | 43 +++-
> libavformat/movenc.c | 107
> +++---
> 2 files changed, 124 insertions(+), 26 deletions(-)
[...]
> +static int mov_write_header(AVFormatContext *s)
> +{
> +AVIOContext *pb = s->pb;
> +MOVMuxContext *mov = s->priv_data;
> +AVDictionaryEntry *t, *global_tcr = av_dict_get(s->metadata, "timecode",
> NULL, 0);
> +int i, ret, hint_track = 0, tmcd_track = 0, nb_tracks = s->nb_streams;
> +
> +if (mov->mode & (MODE_MP4|MODE_MOV|MODE_IPOD) && s->nb_chapters)
> +nb_tracks++;
> +
> +if (mov->flags & FF_MOV_FLAG_RTP_HINT) {
> +/* Add hint tracks for each audio and video stream */
> +hint_track = nb_tracks;
> +for (i = 0; i < s->nb_streams; i++) {
> +AVStream *st = s->streams[i];
> +if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO ||
> +st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> +nb_tracks++;
> +}
> +}
> +}
> +
> +if (mov->mode == MODE_MOV || mov->mode == MODE_MP4)
> +tmcd_track = nb_tracks;
tmcd_track and hint_track are set but unused
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: fix avcc creation for h264 streams missing extradata
On Sunday, October 23, 2016, Richard Kern wrote:
>
> > On Oct 19, 2016, at 8:45 PM, Aman Gupta >
> wrote:
> >
> > From: Aman Gupta >
> >
> > ff_videotoolbox_avcc_extradata_create() was never being called if
> > avctx->extradata_size==0, even though the function does not need or use
> > the avctx->extradata.
>
> Could this be a bug in another part of the code? It seems like extradata
> should be set.
Yes it definitely could be. I can verify if extradata is present on macOS
where the same sample and ffmpeg version worked.
>
> >
> > This manifested itself only on h264 streams in specific containers, and
> > only on iOS. I guess the macOS version of VideoToolbox is more
> > forgiving, atleast on my specific combination of OS version and hardware.
>
> Which container has this issue?
I saw it with an mpegts file, but only on iOS.
>
> >
> > I also added an error log message when VTDecompressionSessionCreate()
> > fails, to help the next poor soul who runs into a bug in this area of the
> > code. The native OSStatus error codes are much easier to google than
> > their AVERROR counterparts (especially in this case, with
> AVERROR_UNKNOWN).
> > ---
> > libavcodec/videotoolbox.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
> > index 1288aa5..b21eccb 100644
> > --- a/libavcodec/videotoolbox.c
> > +++ b/libavcodec/videotoolbox.c
> > @@ -413,7 +413,7 @@ static CFDictionaryRef
> > videotoolbox_decoder_config_create(CMVideoCodecType
> codec
> > kVTVideoDecoderSpecification_
> RequireHardwareAcceleratedVideoDecoder,
> > kCFBooleanTrue);
> >
> > -if (avctx->extradata_size) {
> > +if (avctx->extradata_size || codec_type == kCMVideoCodecType_H264) {
>
> This is somewhat confusing. The extradata_size check is only needed for
> videotoolbox_esds_extradata_create(). It should check the sps and pps
> sizes are valid before creating the avcc atom.
I can remove this outer if statement altogether, and add a check either
around or inside the esds_create if that makes more sense.
>
> > CFMutableDictionaryRef avc_info;
> > CFDataRef data = NULL;
> >
> > @@ -572,13 +572,16 @@ static int videotoolbox_default_init(AVCodecContext
> *avctx)
> > if (buf_attr)
> > CFRelease(buf_attr);
> >
> > +if (status != 0)
> > +av_log(avctx, AV_LOG_ERROR, "Error creating videotoolbox
> decompression session: %d\n", status);
> > +
> > switch (status) {
> > case kVTVideoDecoderNotAvailableNowErr:
> > case kVTVideoDecoderUnsupportedDataFormatErr:
> > return AVERROR(ENOSYS);
> > case kVTVideoDecoderMalfunctionErr:
> > return AVERROR(EINVAL);
> > -case kVTVideoDecoderBadDataErr :
> > +case kVTVideoDecoderBadDataErr:
> > return AVERROR_INVALIDDATA;
> > case 0:
> > return 0;
> > --
> > 2.8.1
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Mon, 24 Oct 2016 18:08:11 +0200
Michael Niedermayer wrote:
> On Mon, Oct 24, 2016 at 02:36:23PM +0200, wm4 wrote:
> > On Mon, 24 Oct 2016 13:24:38 +0200
> > Michael Niedermayer wrote:
> >
> > > On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > > > On Sun, 23 Oct 2016 05:37:25 +0200
> > > > Michael Niedermayer wrote:
> > > >
> > > > > Signed-off-by: Michael Niedermayer
> > > > > ---
> > > > > libavutil/x86/emms.h | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > > > index 6fda6e2..42c18e2 100644
> > > > > --- a/libavutil/x86/emms.h
> > > > > +++ b/libavutil/x86/emms.h
> > > > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > > > > * Empty mmx state.
> > > > > * this must be called between any dsp function and float/double
> > > > > code.
> > > > > * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > > > + * Note, *alloc() and *free() also use float code in some libc
> > > > > implementations
> > > > > + * thus this also applies to them or any function using them.
> > > > > */
> > > > > static av_always_inline void emms_c(void)
> > > > > {
> > > >
> > > > Overly specific and useless information. It's an implementation detail
> > > >
> > >
> > > If we place emms_c() so as to ensure that the FPU state is clean
> > > before all calls to *alloc() and *free() (as is done in the posted
> > > patchset) then we need to document this
> > > so others working on the code are aware of it and wont mistakly break
> > > it.
> > >
> > > If/when a different or more complete solution is implemented then this
> > > note needs to be adjusted accordingly.
> > >
> > > [...]
> >
> > That fixes only the musl-specific issue. It could happen any time again
> > with a set of different callers.
>
> yes
>
>
> > Unless you want to put special effort
> > into fixing operation with musl (and keeping it working) it won't help
> > much to fix the correctness issues.
>
> i did want to do that, but ive a headache atm and the discussion
> about emms yesterday makes me not want to touch emms anymore
>
>
> >
> > If anything, it should probably say "all external" calls or such.
>
> I see this as the long term goal, yes i agree it should be
> be documented if theres consensus about it
>
> The way i imagined it was to fix the practically relevant issues and
> anything else thats easy or attached (like x86-64 for what is the same
> on x86-32) before 3.2 and have 3.2 work with musl on all archs
> then aim at fixing the rest over the following months unless some
> roadblocks like performance issues are hit.
>
> This incremental approuch has been hit with quite some opposition so
> ill leave this to whoever likes to work on this
Nobody said it's going to be simple. Though personally I wouldn't mind
spamming emms() everywhere to make it somewhat more correct - just
don't pretend it actually fixes the problem.
There are some other "brutal" solutions:
A) require musl users to build with MMX disabled or have them set an
environment variable that disables MMX at runtime as part of the CPU
detection
B) add a compile time option that runs emms() unconditionally at the end
of each mmx asm block
Since musl intentionally evades detection, neither can be enabled
automatically, probably. It would be interesting to see what the speed
impact of B) actually is.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Mon, Oct 24, 2016 at 8:09 PM, wm4 wrote: > B) add a compile time option that runs emms() unconditionally at the end >of each mmx asm block > > Since musl intentionally evades detection, neither can be enabled > automatically, probably. It would be interesting to see what the speed > impact of B) actually is. Depends on the function, but in small/simple DSP functions (which most relevant MMX functions are) adding emms at the end will make the entire function twice as slow on most Intel CPUs. So it's certainly not negligible. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.
On Mon, Oct 24, 2016 at 08:09:09PM +0200, wm4 wrote: > B) add a compile time option that runs emms() unconditionally at the end >of each mmx asm block > It would be interesting to see what the speed > impact of B) actually is. +1 Regards, Rune ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 02/12] bfi: validate sample_rate
On 24.10.2016 09:55, wm4 wrote:
> On Sun, 23 Oct 2016 18:27:02 +0200
> Andreas Cadhalpun wrote:
>
>> A negative sample rate doesn't make sense and triggers assertions in
>> av_rescale_rnd.
>>
>> Signed-off-by: Andreas Cadhalpun
>> ---
>> libavformat/bfi.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/libavformat/bfi.c b/libavformat/bfi.c
>> index 568363d..ef4c17d 100644
>> --- a/libavformat/bfi.c
>> +++ b/libavformat/bfi.c
>> @@ -88,6 +88,10 @@ static int bfi_read_header(AVFormatContext * s)
>> vstream->codecpar->extradata_size);
>>
>> astream->codecpar->sample_rate = avio_rl32(pb);
>> +if (astream->codecpar->sample_rate <= 0) {
>> +av_log(s, AV_LOG_ERROR, "Invalid sample rate %d\n",
>> astream->codecpar->sample_rate);
>> +return AVERROR_INVALIDDATA;
>> +}
>>
>> /* Set up the video codec... */
>> avpriv_set_pts_info(vstream, 32, 1, fps);
>
> Would it make sense to validate codecpars in the generic code (utils.c)?
I think that's a good idea. Still, checking the value where it is actually
set is good in any case.
Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On 23.10.2016 20:18, Carl Eugen Hoyos wrote: > 2016-10-23 12:14 GMT+02:00 Andreas Cadhalpun > : > >> I also don't like adding emms_c in various places, but I don't see a >> realistic alternative > > (+1!) > >> other than not fixing the problem. And I think that would be worse >> than this patch set. > > I don't (strongly) disagree but could you elaborate on why this would > be such a bad alternative? For one thing FFmpeg shouldn't rely on undefined behavior (a theoretical issue) and for another thing it should work with musl libc (a practical problem). I think not fixing the latter is bad for our users. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: fix avcc creation for h264 streams missing extradata
On Sun, Oct 23, 2016 at 11:20 PM, Aman Gupta wrote:
>
>
> On Sunday, October 23, 2016, Richard Kern wrote:
>
>>
>> > On Oct 19, 2016, at 8:45 PM, Aman Gupta wrote:
>> >
>> > From: Aman Gupta
>> >
>> > ff_videotoolbox_avcc_extradata_create() was never being called if
>> > avctx->extradata_size==0, even though the function does not need or use
>> > the avctx->extradata.
>>
>> Could this be a bug in another part of the code? It seems like extradata
>> should be set.
>
>
> Yes it definitely could be. I can verify if extradata is present on macOS
> where the same sample and ffmpeg version worked.
>
Looks like missing extradata has nothing to do with the platform, but
rather only happens when you skip calling avformat_find_stream_info().
I am skipping the find_stream_info() call to reduce time spent probing and
begin playback more quickly, since the initial avformat_open_input() is
enough to discover available streams on mpegts containers.
That means this issue is not as widespread as I initially thought, but I
still think it's an optimization worth making.
>
>
>>
>> >
>> > This manifested itself only on h264 streams in specific containers, and
>> > only on iOS. I guess the macOS version of VideoToolbox is more
>> > forgiving, atleast on my specific combination of OS version and
>> hardware.
>>
>> Which container has this issue?
>
>
> I saw it with an mpegts file, but only on iOS.
>
>
>>
>> >
>> > I also added an error log message when VTDecompressionSessionCreate()
>> > fails, to help the next poor soul who runs into a bug in this area of
>> the
>> > code. The native OSStatus error codes are much easier to google than
>> > their AVERROR counterparts (especially in this case, with
>> AVERROR_UNKNOWN).
>> > ---
>> > libavcodec/videotoolbox.c | 7 +--
>> > 1 file changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c
>> > index 1288aa5..b21eccb 100644
>> > --- a/libavcodec/videotoolbox.c
>> > +++ b/libavcodec/videotoolbox.c
>> > @@ -413,7 +413,7 @@ static CFDictionaryRef
>> videotoolbox_decoder_config_create(CMVideoCodecType codec
>> > kVTVideoDecoderSpecification_R
>> equireHardwareAcceleratedVideoDecoder,
>> > kCFBooleanTrue);
>> >
>> > -if (avctx->extradata_size) {
>> > +if (avctx->extradata_size || codec_type == kCMVideoCodecType_H264)
>> {
>>
>> This is somewhat confusing. The extradata_size check is only needed for
>> videotoolbox_esds_extradata_create(). It should check the sps and pps
>> sizes are valid before creating the avcc atom.
>
>
> I can remove this outer if statement altogether, and add a check either
> around or inside the esds_create if that makes more sense.
>
Let me know if you'd like me to submit the alternative version of this
patch.
>
>
>>
>> > CFMutableDictionaryRef avc_info;
>> > CFDataRef data = NULL;
>> >
>> > @@ -572,13 +572,16 @@ static int videotoolbox_default_init(AVCodecContext
>> *avctx)
>> > if (buf_attr)
>> > CFRelease(buf_attr);
>> >
>> > +if (status != 0)
>> > +av_log(avctx, AV_LOG_ERROR, "Error creating videotoolbox
>> decompression session: %d\n", status);
>> > +
>> > switch (status) {
>> > case kVTVideoDecoderNotAvailableNowErr:
>> > case kVTVideoDecoderUnsupportedDataFormatErr:
>> > return AVERROR(ENOSYS);
>> > case kVTVideoDecoderMalfunctionErr:
>> > return AVERROR(EINVAL);
>> > -case kVTVideoDecoderBadDataErr :
>> > +case kVTVideoDecoderBadDataErr:
>> > return AVERROR_INVALIDDATA;
>> > case 0:
>> > return 0;
>> > --
>> > 2.8.1
>> >
>> > ___
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On 24.10.2016 16:14, Ronald S. Bultje wrote: > On Mon, Oct 24, 2016 at 8:47 AM, wm4 wrote: >> On Mon, 24 Oct 2016 07:54:47 -0400 >> "Ronald S. Bultje" wrote: >>> On Mon, Oct 24, 2016 at 3:36 AM, wm4 wrote: I was under the impression that it is UB to have the FPU in MMX state at any time while in C, not just while e.g. calling the stdlib. Maybe I got that wrong (how would MMX intrinsics even work?) - can anyone shed light on the exact requirements? (Possibly again, sorry.) >>> >>> I'm under the impression that it's part of the calling convention. That >> is, >>> any code anywhere (including mmx intrinsics, indeed) can - when called - >>> expect the state to be cleared by the caller, just like you'd expect >>> eax/edx to be caller-save (whereas esi/edi are callee-save). >>> >>> However, if you never call external code (including intrinsics), you can >>> ignore this, just as you can ignore / create your own calling >>> convention (remember fastcall etc.?). However, when calling any external >>> code, this could (in theory) crash; it's just that right now it only >>> crashes with musl when calling malloc/free. So basically, ffmpeg has its >>> own calling convention, and manually calling emms_c() fixes "ffmpeg" >>> calling convention to be compatible with "standard" calling >> convention...? >> >> It can't really be a calling convention unless the compiler is aware of >> it? It is defined as part of the System V Application Binary Interface [1]: "The CPU shall be in x87 mode upon entry to a function. Therefore, every function that uses the MMX registers is required to issue an emms or femms instruction after using MMX registers, before returning or calling another function." >> We're doing things behind the compiler's back here. The safest >> assumption would be that leaving the FPU state invalid while in C is >> not allowed, period. That's what the standard says. >> The next safest assumption is that it's fine as long as we explicitly >> don't use floating point or call external functions that aren't >> MMX-aware. This would mean calling any libc functions or user callbacks >> (including indirectly through e.g. av_log) is not fine. This is probably OK in practice and likely has a significant performance benefit. Best regards, Andreas 1: https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/videotoolbox: fix avcc creation for h264 streams missing extradata
On Mon, 24 Oct 2016 12:18:07 -0700 Aman Gupta wrote: > On Sun, Oct 23, 2016 at 11:20 PM, Aman Gupta wrote: > > > > > > > On Sunday, October 23, 2016, Richard Kern wrote: > > > >> > >> > On Oct 19, 2016, at 8:45 PM, Aman Gupta wrote: > >> > > >> > From: Aman Gupta > >> > > >> > ff_videotoolbox_avcc_extradata_create() was never being called if > >> > avctx->extradata_size==0, even though the function does not need or use > >> > the avctx->extradata. > >> > >> Could this be a bug in another part of the code? It seems like extradata > >> should be set. > > > > > > Yes it definitely could be. I can verify if extradata is present on macOS > > where the same sample and ffmpeg version worked. > > > > Looks like missing extradata has nothing to do with the platform, but > rather only happens when you skip calling avformat_find_stream_info(). > > I am skipping the find_stream_info() call to reduce time spent probing and > begin playback more quickly, since the initial avformat_open_input() is > enough to discover available streams on mpegts containers. > > That means this issue is not as widespread as I initially thought, but I > still think it's an optimization worth making. That makes sense. I think a decoder shouldn't really rely on this pseudo extradata that libavformat constructs. That this code is run only with extradata_size set makes no sense at least in the h264 code and was an oversight. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Mon, 24 Oct 2016 21:19:46 +0200 Andreas Cadhalpun wrote: > On 24.10.2016 16:14, Ronald S. Bultje wrote: > > On Mon, Oct 24, 2016 at 8:47 AM, wm4 wrote: > >> On Mon, 24 Oct 2016 07:54:47 -0400 > >> "Ronald S. Bultje" wrote: > >>> On Mon, Oct 24, 2016 at 3:36 AM, wm4 wrote: > I was under the impression that it is UB to have the FPU in MMX state > at any time while in C, not just while e.g. calling the stdlib. Maybe I > got that wrong (how would MMX intrinsics even work?) - can anyone shed > light on the exact requirements? (Possibly again, sorry.) > >>> > >>> I'm under the impression that it's part of the calling convention. That > >> is, > >>> any code anywhere (including mmx intrinsics, indeed) can - when called - > >>> expect the state to be cleared by the caller, just like you'd expect > >>> eax/edx to be caller-save (whereas esi/edi are callee-save). > >>> > >>> However, if you never call external code (including intrinsics), you can > >>> ignore this, just as you can ignore / create your own calling > >>> convention (remember fastcall etc.?). However, when calling any external > >>> code, this could (in theory) crash; it's just that right now it only > >>> crashes with musl when calling malloc/free. So basically, ffmpeg has its > >>> own calling convention, and manually calling emms_c() fixes "ffmpeg" > >>> calling convention to be compatible with "standard" calling > >> convention...? > >> > >> It can't really be a calling convention unless the compiler is aware of > >> it? > > It is defined as part of the System V Application Binary Interface [1]: > "The CPU shall be in x87 mode upon entry to a function. Therefore, every > function that uses the MMX registers is required to issue an emms or femms > instruction after using MMX registers, before returning or calling another > function." I mean FFmpeg can't make up its own calling convention without the compiler's knowledge. But thanks for reminding me about this but of the sysv ABI. The paragraph you quoted is actually very clear about the requirements. It means FFmpeg can barely do anything and remain standard compliant: a ASM function must, according to the calling convention, reset the MMX state when returning. What FFmpeg does here was misdesigned from the very start. > >> We're doing things behind the compiler's back here. The safest > >> assumption would be that leaving the FPU state invalid while in C is > >> not allowed, period. > > That's what the standard says. > > >> The next safest assumption is that it's fine as long as we explicitly > >> don't use floating point or call external functions that aren't > >> MMX-aware. This would mean calling any libc functions or user callbacks > >> (including indirectly through e.g. av_log) is not fine. > > This is probably OK in practice and likely has a significant performance > benefit. Seems like it. The compiler still could generate code using the FPU state at any point, though, unless maybe there is an inline asm block. No function can put the FPU into MMX mode, because any MMX using function must have called emms before returning. Consequently, only compiler-known intrinsics or opaque inline asm block could clobber the FPU state. The latter because the compiler doesn't inspect asm blocks. > > Best regards, > Andreas > > > 1: > https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On 24.10.2016 21:34, wm4 wrote: > On Mon, 24 Oct 2016 21:19:46 +0200 > Andreas Cadhalpun wrote: >> On 24.10.2016 16:14, Ronald S. Bultje wrote: >>> On Mon, Oct 24, 2016 at 8:47 AM, wm4 wrote: The next safest assumption is that it's fine as long as we explicitly don't use floating point or call external functions that aren't MMX-aware. This would mean calling any libc functions or user callbacks (including indirectly through e.g. av_log) is not fine. >> >> This is probably OK in practice and likely has a significant performance >> benefit. > > Seems like it. > > The compiler still could generate code using the FPU state at any > point, though, unless maybe there is an inline asm block. No function > can put the FPU into MMX mode, because any MMX using function must have > called emms before returning. Consequently, only compiler-known > intrinsics or opaque inline asm block could clobber the FPU state. The > latter because the compiler doesn't inspect asm blocks. I agree. However, I think that this is currently only a theoretical issue. At least I'm not aware of this causing problems with a real compiler. Best regards, Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Mon, Oct 24, 2016 at 9:34 PM, wm4 wrote: > a ASM function must, according to the calling convention, reset the > MMX state when returning. > > What FFmpeg does here was misdesigned from the very start. The decision to issue emms manually instead of after every MMX function was a deliberate decision. I'd hardly call it "misdesigned" to make SIMD code twice as fast at the cost of technically abusing the ABI, which has worked flawlessly for years until very recently. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
Hi, On Mon, Oct 24, 2016 at 3:34 PM, wm4 wrote: > On Mon, 24 Oct 2016 21:19:46 +0200 > Andreas Cadhalpun wrote: > > > On 24.10.2016 16:14, Ronald S. Bultje wrote: > > > On Mon, Oct 24, 2016 at 8:47 AM, wm4 wrote: > > >> On Mon, 24 Oct 2016 07:54:47 -0400 > > >> "Ronald S. Bultje" wrote: > > >>> On Mon, Oct 24, 2016 at 3:36 AM, wm4 wrote: > > I was under the impression that it is UB to have the FPU in MMX > state > > at any time while in C, not just while e.g. calling the stdlib. > Maybe I > > got that wrong (how would MMX intrinsics even work?) - can anyone > shed > > light on the exact requirements? (Possibly again, sorry.) > > >>> > > >>> I'm under the impression that it's part of the calling convention. > That > > >> is, > > >>> any code anywhere (including mmx intrinsics, indeed) can - when > called - > > >>> expect the state to be cleared by the caller, just like you'd expect > > >>> eax/edx to be caller-save (whereas esi/edi are callee-save). > > >>> > > >>> However, if you never call external code (including intrinsics), you > can > > >>> ignore this, just as you can ignore / create your own calling > > >>> convention (remember fastcall etc.?). However, when calling any > external > > >>> code, this could (in theory) crash; it's just that right now it only > > >>> crashes with musl when calling malloc/free. So basically, ffmpeg has > its > > >>> own calling convention, and manually calling emms_c() fixes "ffmpeg" > > >>> calling convention to be compatible with "standard" calling > > >> convention...? > > >> > > >> It can't really be a calling convention unless the compiler is aware > of > > >> it? > > > > It is defined as part of the System V Application Binary Interface [1]: > > "The CPU shall be in x87 mode upon entry to a function. Therefore, every > > function that uses the MMX registers is required to issue an emms or > femms > > instruction after using MMX registers, before returning or calling > another > > function." > > I mean FFmpeg can't make up its own calling convention without the > compiler's knowledge. > > But thanks for reminding me about this but of the sysv ABI. The > paragraph you quoted is actually very clear about the requirements. It > means FFmpeg can barely do anything and remain standard compliant: a > ASM function must, according to the calling convention, reset the MMX > state when returning. > > What FFmpeg does here was misdesigned from the very start. Good idea to reference Hendrik Gramner here, who keeps insisting we get rid of all MMX code in ffmpeg (at least as an option) for future Intel CPUs in which MMX will be deprecated. Food for thought, indeed. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Mon, 24 Oct 2016 21:53:22 +0200 Henrik Gramner wrote: > On Mon, Oct 24, 2016 at 9:34 PM, wm4 wrote: > > a ASM function must, according to the calling convention, reset the > > MMX state when returning. > > > > What FFmpeg does here was misdesigned from the very start. > > The decision to issue emms manually instead of after every MMX > function was a deliberate decision. I'd hardly call it "misdesigned" > to make SIMD code twice as fast at the cost of technically abusing the > ABI, which has worked flawlessly for years until very recently. It's still clearly not a good idea because it violates the ABI directly. OK let's get back to the "practical" argument that we have not much of a choice but to violate the ABI. Then at least the "weaker" assumption, that emms should be called before calling (or returning to) functions that don't explicitly support MMX, should be followed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vf_colorspace: Add support for iec61966-2.1 (sRGB) transfer
Hi, On Thu, Oct 20, 2016 at 12:37 PM, Michael Niedermayer < mich...@niedermayer.cc> wrote: > On Tue, Oct 18, 2016 at 03:17:26PM -0400, Vittorio Giovara wrote: > > Signed-off-by: Vittorio Giovara > > --- > > Special thanks to Andrew Roland for helping me figure out these numbers. > > Please keep me in CC. > > Vittorio > > > > libavfilter/vf_colorspace.c | 3 +++ > > 1 file changed, 3 insertions(+) > > LGTM Pushed. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vp9: change order of operations in adapt_prob().
Hi, On Fri, Oct 14, 2016 at 2:09 PM, James Zern wrote: > Ronald, > > On Fri, Oct 14, 2016 at 10:01 AM, Ronald S. Bultje > wrote: > > This is intended to workaround bug "665 Integer Divide Instruction May > > Cause Unpredictable Behavior" on some early AMD CPUs, which causes a > > div-by-zero in this codepath, such as reported in Mozilla bug #1293996. > > > > Note that this isn't guaranteed to fix the bug, since a compiler is free > > to reorder instructions that don't depend on each other. However, it > > appears to fix the bug in Firefox, and a similar patch was applied to > > libvpx also (see Chrome bug #599899). > > > > I recently made a few additional changes as this regressed in chrome > [1][2], but just like this change there's no guarantee it won't occur > again. > > [1] https://chromium.googlesource.com/webm/libvpx/+/ > 8b4210940ce4183d4cfded42c323612c0c6d1688 > [2] https://chromium.googlesource.com/webm/libvpx/+/ > 82ea74223771793628dbd812c2fd50afcfb8183a > > > --- > > libavcodec/vp9.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > lgtm (Firefox beta crash reports confirmed that the bug is fixed.) Pushed. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/4] lavfi: split frame_count between input and output.
On 10/23/16, Nicolas George wrote: > AVFilterLink.frame_count is supposed to count the number of frames > that were passed on the link, but with min_samples, that number is > not always the same for the source and destination filters. > With the addition of a FIFO on the link, the difference will become > more significant. > > Split the variable in two: frame_count_in counts the number of > frames that entered the link, frame_count_out counts the number > of frames that were sent to the destination filter. > > Signed-off-by: Nicolas George > --- > libavfilter/af_ashowinfo.c | 2 +- > libavfilter/af_volume.c | 2 +- > libavfilter/asrc_sine.c | 2 +- > libavfilter/avf_showfreqs.c | 4 ++-- > libavfilter/avfilter.c | 5 +++-- > libavfilter/avfilter.h | 2 +- > libavfilter/f_loop.c | 2 +- > libavfilter/f_metadata.c | 4 ++-- > libavfilter/f_select.c | 2 +- > libavfilter/f_streamselect.c | 2 +- > libavfilter/vf_bbox.c| 2 +- > libavfilter/vf_blackdetect.c | 2 +- > libavfilter/vf_blend.c | 2 +- > libavfilter/vf_crop.c| 2 +- > libavfilter/vf_decimate.c| 2 +- > libavfilter/vf_detelecine.c | 2 +- > libavfilter/vf_drawtext.c| 4 ++-- > libavfilter/vf_eq.c | 2 +- > libavfilter/vf_fade.c| 8 > libavfilter/vf_fieldhint.c | 14 +++--- > libavfilter/vf_fieldmatch.c | 6 +++--- > libavfilter/vf_framestep.c | 2 +- > libavfilter/vf_geq.c | 2 +- > libavfilter/vf_hue.c | 2 +- > libavfilter/vf_overlay.c | 2 +- > libavfilter/vf_paletteuse.c | 2 +- > libavfilter/vf_perspective.c | 4 ++-- > libavfilter/vf_rotate.c | 2 +- > libavfilter/vf_showinfo.c| 2 +- > libavfilter/vf_swaprect.c| 2 +- > libavfilter/vf_telecine.c| 2 +- > libavfilter/vf_tinterlace.c | 4 ++-- > libavfilter/vf_vignette.c| 2 +- > libavfilter/vf_zoompan.c | 6 +++--- > libavfilter/vsrc_mptestsrc.c | 2 +- > 35 files changed, 55 insertions(+), 54 deletions(-) > Idea sounds sane, i havent chacked each change if it is correct. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vf_colorspace: don't spam console with warnings if range is unspecified.
Hi, On Sat, Oct 22, 2016 at 10:02 PM, Ronald S. Bultje wrote: > Hi, > > On Sat, Oct 22, 2016 at 12:19 PM, Moritz Barsnick > wrote: > >> On Sat, Oct 22, 2016 at 08:43:26 -0400, Ronald S. Bultje wrote: >> > Is there documentation on what coverity expects? >> >> I found this: >> https://lost-contact.mit.edu/afs/cs.stanford.edu/pkg/prevent >> -4.3.1/i386_linux26/opt/prevent-linux-4.3.1/cgi-bin/ >> doc/checker_ref.html#c_checker_MISSING_BREAK > > > I was hoping for documentation on what it expects, not what can be changed > about it :) Anyway, I'll change it, I don't really care. > Pushed with that modification. Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] libavcodec : add psd image file decoder
Hello,
In attach new patchs.
I tested with zzuf using
zzuf -c -s0:1 -r0.01 ./ffmpeg -i inFile -f null -
on one sample
and with
zzuf -c -s0:5000 -r0.01 ./ffmpeg -i inFile -f null
on 10 others samples
Martin
From 37a7ee0fb3b32336c5376f85f1caeafb03f9c0e5 Mon Sep 17 00:00:00 2001
From: Martin Vignali
Date: Mon, 24 Oct 2016 22:16:48 +0200
Subject: [PATCH 1/2] libavcodec : add decoder for Photoshop PSD image file.
Decode the Image Data Section (who contain merge picture).
Support RGB/A and Grayscale/A in 8bits and 16 bits by channel.
Support uncompress and rle compression in Image Data Section.
---
Changelog | 1 +
doc/general.texi | 2 +
libavcodec/Makefile| 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/avcodec.h | 1 +
libavcodec/psd.c | 412 +
6 files changed, 418 insertions(+)
create mode 100644 libavcodec/psd.c
diff --git a/Changelog b/Changelog
index e05b1af..21d04a7 100644
--- a/Changelog
+++ b/Changelog
@@ -38,6 +38,7 @@ version :
- Matroska muxer now writes CRC32 elements by default in all Level 1 elements
- sidedata video and asidedata audio filter
- Changed mapping of rtp MIME type G726 to codec g726le.
+- PSD Decoder
version 3.1:
diff --git a/doc/general.texi b/doc/general.texi
index f08c3cd..66424b5 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -584,6 +584,8 @@ following image formats are supported:
@item PNG @tab X @tab X
@item PPM @tab X @tab X
@tab Portable PixelMap image
+@item PSD @tab @tab X
+@tab Photoshop
@item PTX @tab @tab X
@tab V.Flash PTX format
@item SGI @tab X @tab X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index f1d5bf1..235f6f3 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -462,6 +462,7 @@ OBJS-$(CONFIG_PRORES_LGPL_DECODER) += proresdec_lgpl.o proresdsp.o proresdat
OBJS-$(CONFIG_PRORES_ENCODER) += proresenc_anatoliy.o
OBJS-$(CONFIG_PRORES_AW_ENCODER) += proresenc_anatoliy.o
OBJS-$(CONFIG_PRORES_KS_ENCODER) += proresenc_kostya.o proresdata.o
+OBJS-$(CONFIG_PSD_DECODER) += psd.o
OBJS-$(CONFIG_PTX_DECODER) += ptx.o
OBJS-$(CONFIG_QCELP_DECODER) += qcelpdec.o \
celp_filters.o acelp_vectors.o \
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index b592aa3..83136e6 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -288,6 +288,7 @@ void avcodec_register_all(void)
REGISTER_ENCODER(PRORES_AW, prores_aw);
REGISTER_ENCODER(PRORES_KS, prores_ks);
REGISTER_DECODER(PRORES_LGPL, prores_lgpl);
+REGISTER_DECODER(PSD, psd);
REGISTER_DECODER(PTX, ptx);
REGISTER_DECODER(QDRAW, qdraw);
REGISTER_DECODER(QPEG, qpeg);
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index af550a8..fe45a5f 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -411,6 +411,7 @@ enum AVCodecID {
AV_CODEC_ID_MAGICYUV,
AV_CODEC_ID_SHEERVIDEO,
AV_CODEC_ID_YLC,
+AV_CODEC_ID_PSD,
/* various PCM "codecs" */
AV_CODEC_ID_FIRST_AUDIO = 0x1, ///< A dummy id pointing at the start of audio codecs
diff --git a/libavcodec/psd.c b/libavcodec/psd.c
new file mode 100644
index 000..5dee821
--- /dev/null
+++ b/libavcodec/psd.c
@@ -0,0 +1,412 @@
+/*
+ * Photoshop (PSD) image decoder
+ * Copyright (c) 2016 Jokyo Images
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "bytestream.h"
+#include "internal.h"
+
+enum PsdCompr {
+PSD_RAW,
+PSD_RLE,
+PSD_ZIP_WITHOUT_P,
+PSD_ZIP_WITH_P,
+};
+
+enum PsdColorMode {
+PSD_BITMAP,
+PSD_GRAYSCALE,
+PSD_INDEXED,
+PSD_RGB,
+PSD_CMYK,
+PSD_MULTICHANNEL,
+PSD_DUOTONE,
+PSD_LAB,
+};
+
+typedef struct PSDContext {
+AVClass *class;
+AVFrame *picture;
+AVCodecContext *avctx;
+GetByteContext gb;
+
+uint8_t * tmp;
+
+uint16_t channel_count;
+uint16_t channel_depth;
+
+uint64_t uncompressed_size;
+unsigned int pixel_size;/* 1 for 8 bits, 2 for 16 bits */
+unsigned int l
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Mon, Oct 24, 2016 at 9:59 PM, Ronald S. Bultje wrote: > Good idea to reference Hendrik Gramner here, who keeps insisting we get rid > of all MMX code in ffmpeg (at least as an option) for future Intel CPUs in > which MMX will be deprecated. Replacing MMX with SSE2 is indeed the most "proper" fix in my opinion, but it's a fair amount of work and not done in an evening. The fact that a lot of assembly lacks unit tests is certainly not helping in that regard. Some MMX instructions are slower than the equivalent SSE2 code on Skylake. Intel hasn't officially commented on (as far as I know at least) if we should expect this trend to continue, but they certainly seem to treat MMX as legacy. I doubt they would completely remove support for it though, backwards compatibility is a big selling-point for x86. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
Hi, On Mon, Oct 24, 2016 at 4:26 PM, Henrik Gramner wrote: > On Mon, Oct 24, 2016 at 9:59 PM, Ronald S. Bultje > wrote: > > Good idea to reference Hendrik Gramner here, who keeps insisting we get > rid > > of all MMX code in ffmpeg (at least as an option) for future Intel CPUs > in > > which MMX will be deprecated. > > Replacing MMX with SSE2 is indeed the most "proper" fix in my opinion, > but it's a fair amount of work and not done in an evening. > > The fact that a lot of assembly lacks unit tests is certainly not > helping in that regard. > > Some MMX instructions are slower than the equivalent SSE2 code on > Skylake. Intel hasn't officially commented on (as far as I know at > least) if we should expect this trend to continue, but they certainly > seem to treat MMX as legacy. > > I doubt they would completely remove support for it though, backwards > compatibility is a big selling-point for x86. Well, it gives us another way of fixing this issue (on x86-64 only): have sse2 implementations for all code that has a mmx (register) path right now. Given the complexity of this issue, it's worth considering that fix along with the other possibilities... Ronald ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] ffmpeg: move some unrelated code out of a filter loop
---
Not sure if the chunk is even needed
---
ffmpeg.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/ffmpeg.c b/ffmpeg.c
index 3b91710..e8088c0 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2317,10 +2317,9 @@ static int decode_video(InputStream *ist, AVPacket *pkt,
int *got_output, int eo
}
frame_sample_aspect= av_opt_ptr(avcodec_get_frame_class(), decoded_frame,
"sample_aspect_ratio");
+if (!frame_sample_aspect->num)
+*frame_sample_aspect = ist->st->sample_aspect_ratio;
for (i = 0; i < ist->nb_filters; i++) {
-if (!frame_sample_aspect->num)
-*frame_sample_aspect = ist->st->sample_aspect_ratio;
-
if (i < ist->nb_filters - 1) {
f = ist->filter_frame;
err = av_frame_ref(f, decoded_frame);
--
2.10.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] ffmpeg: factor out sending frame to filters
Video doesn't exit ffmpeg on error anymore, and audio now prints an
error.
---
ffmpeg.c | 64 ++--
1 file changed, 30 insertions(+), 34 deletions(-)
diff --git a/ffmpeg.c b/ffmpeg.c
index e8088c0..b7b0dc6 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2055,9 +2055,35 @@ static int decode(AVCodecContext *avctx, AVFrame *frame,
int *got_frame, AVPacke
return 0;
}
+static int send_frame_to_filters(InputStream *ist, AVFrame *decoded_frame)
+{
+int i, ret;
+AVFrame *f;
+
+for (i = 0; i < ist->nb_filters; i++) {
+if (i < ist->nb_filters - 1) {
+f = ist->filter_frame;
+ret = av_frame_ref(f, decoded_frame);
+if (ret < 0)
+break;
+} else
+f = decoded_frame;
+ret = av_buffersrc_add_frame_flags(ist->filters[i]->filter, f,
+ AV_BUFFERSRC_FLAG_PUSH);
+if (ret == AVERROR_EOF)
+ret = 0; /* ignore */
+if (ret < 0) {
+av_log(NULL, AV_LOG_ERROR,
+ "Failed to inject frame into filter network: %s\n",
av_err2str(ret));
+break;
+}
+}
+return ret;
+}
+
static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output)
{
-AVFrame *decoded_frame, *f;
+AVFrame *decoded_frame;
AVCodecContext *avctx = ist->dec_ctx;
int i, ret, err = 0, resample_changed;
AVRational decoded_frame_tb;
@@ -2152,21 +2178,7 @@ static int decode_audio(InputStream *ist, AVPacket *pkt,
int *got_output)
(AVRational){1,
avctx->sample_rate}, decoded_frame->nb_samples,
&ist->filter_in_rescale_delta_last,
(AVRational){1,
avctx->sample_rate});
ist->nb_samples = decoded_frame->nb_samples;
-for (i = 0; i < ist->nb_filters; i++) {
-if (i < ist->nb_filters - 1) {
-f = ist->filter_frame;
-err = av_frame_ref(f, decoded_frame);
-if (err < 0)
-break;
-} else
-f = decoded_frame;
-err = av_buffersrc_add_frame_flags(ist->filters[i]->filter, f,
- AV_BUFFERSRC_FLAG_PUSH);
-if (err == AVERROR_EOF)
-err = 0; /* ignore */
-if (err < 0)
-break;
-}
+err = send_frame_to_filters(ist, decoded_frame);
decoded_frame->pts = AV_NOPTS_VALUE;
av_frame_unref(ist->filter_frame);
@@ -2176,7 +2188,7 @@ static int decode_audio(InputStream *ist, AVPacket *pkt,
int *got_output)
static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int
eof)
{
-AVFrame *decoded_frame, *f;
+AVFrame *decoded_frame;
int i, ret = 0, err = 0, resample_changed;
int64_t best_effort_timestamp;
int64_t dts = AV_NOPTS_VALUE;
@@ -2319,23 +2331,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt,
int *got_output, int eo
frame_sample_aspect= av_opt_ptr(avcodec_get_frame_class(), decoded_frame,
"sample_aspect_ratio");
if (!frame_sample_aspect->num)
*frame_sample_aspect = ist->st->sample_aspect_ratio;
-for (i = 0; i < ist->nb_filters; i++) {
-if (i < ist->nb_filters - 1) {
-f = ist->filter_frame;
-err = av_frame_ref(f, decoded_frame);
-if (err < 0)
-break;
-} else
-f = decoded_frame;
-err = av_buffersrc_add_frame_flags(ist->filters[i]->filter, f,
AV_BUFFERSRC_FLAG_PUSH);
-if (err == AVERROR_EOF) {
-err = 0; /* ignore */
-} else if (err < 0) {
-av_log(NULL, AV_LOG_FATAL,
- "Failed to inject frame into filter network: %s\n",
av_err2str(err));
-exit_program(1);
-}
-}
+err = send_frame_to_filters(ist, decoded_frame);
fail:
av_frame_unref(ist->filter_frame);
--
2.10.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Fate : add metadata test for aphasemeter
Hello, In attach a patch to add 2 test for aphasemeter metadata The sample can be found here : https://we.tl/AP7SY8eNSe As aphasemeter use float, i choose two "extreme" cases : - a mono file, with result of 1. - a out of phase 1000 Hz with result of -1. Martin From 73056155b32ad39c370518985de844bd9f46f1cc Mon Sep 17 00:00:00 2001 From: Martin Vignali Date: Mon, 24 Oct 2016 23:00:23 +0200 Subject: [PATCH] fate filter metadata : add test for aphasemeter - Test a mono file (in phase) -> 1. as result - Test a out of phase 1000 Hz -> -1. as result --- tests/fate/filter-video.mak| 8 +++ .../ref/fate/filter-metadata-avf-aphase-meter-mono | 25 ++ .../filter-metadata-avf-aphase-meter-out-of-phase | 25 ++ 3 files changed, 58 insertions(+) create mode 100644 tests/ref/fate/filter-metadata-avf-aphase-meter-mono create mode 100644 tests/ref/fate/filter-metadata-avf-aphase-meter-out-of-phase diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak index e2513f5..d79bbf6 100644 --- a/tests/fate/filter-video.mak +++ b/tests/fate/filter-video.mak @@ -653,6 +653,14 @@ FATE_METADATA_FILTER-$(call ALLYES, $(READVITC_METADATA_DEPS)) += fate-filter-me fate-filter-metadata-readvitc-thr: SRC = $(TARGET_SAMPLES)/filter/sample-vitc.avi fate-filter-metadata-readvitc-thr: CMD = run $(FILTER_METADATA_COMMAND) "movie='$(SRC)',readvitc=thr_b=0.3:thr_w=0.5" +AVF_PHASE_METER_DEPS = FFPROBE AVDEVICE LAVFI_INDEV AMOVIE_FILTER FLAC_DEMUXER FLAC_DECODER SINE_FILTER APHASEMETER_FILTER +FATE_METADATA_FILTER-$(call ALLYES, $(AVF_PHASE_METER_DEPS)) += fate-filter-metadata-avf-aphase-meter-mono +fate-filter-metadata-avf-aphase-meter-mono: CMD = run $(FILTER_METADATA_COMMAND) sine="frequency=1000:sample_rate=48000:duration=1,aphasemeter" + +FATE_METADATA_FILTER-$(call ALLYES, $(AVF_PHASE_METER_DEPS)) += fate-filter-metadata-avf-aphase-meter-out-of-phase +fate-filter-metadata-avf-aphase-meter-out-of-phase: SRC = $(TARGET_SAMPLES)/filter/out-of-phase-1000hz.flac +fate-filter-metadata-avf-aphase-meter-out-of-phase: CMD = run $(FILTER_METADATA_COMMAND) "amovie='$(SRC)',aphasemeter" + tests/data/file4560-override2rotate0.mov: TAG = GEN tests/data/file4560-override2rotate0.mov: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \ diff --git a/tests/ref/fate/filter-metadata-avf-aphase-meter-mono b/tests/ref/fate/filter-metadata-avf-aphase-meter-mono new file mode 100644 index 000..3c72375 --- /dev/null +++ b/tests/ref/fate/filter-metadata-avf-aphase-meter-mono @@ -0,0 +1,25 @@ +pkt_pts=0|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=1920|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=3840|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=5760|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=7680|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=9600|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=11520|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=13440|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=15360|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=17280|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=19200|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=21120|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=23040|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=24960|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=26880|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=28800|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=30720|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=32640|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=34560|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=36480|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=38400|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=40320|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=42240|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=44160|tag:lavfi.aphasemeter.phase=1.00 +pkt_pts=46080|tag:lavfi.aphasemeter.phase=1.00 diff --git a/tests/ref/fate/filter-metadata-avf-aphase-meter-out-of-phase b/tests/ref/fate/filter-metadata-avf-aphase-meter-out-of-phase new file mode 100644 index 000..425b2ae --- /dev/null +++ b/tests/ref/fate/filter-metadata-avf-aphase-meter-out-of-phase @@ -0,0 +1,25 @@ +pkt_pts=0|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=1920|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=3840|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=5760|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=7680|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=9600|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=11520|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=13440|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=15360|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=17280|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=19200|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=21120|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=23040|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=24960|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=26880|tag:lavfi.aphasemeter.phase=-1.00 +pkt_pts=28800|tag:lavfi.aphaseme
[FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.
From: liangsi
---
libavformat/isom.h | 1 +
libavformat/mov.c | 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index 2246fed..6824f7e 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -214,6 +214,7 @@ typedef struct MOVContext {
int use_absolute_path;
int ignore_editlist;
int ignore_chapters;
+int ignore_moov_atom_in_free;
int seek_individually;
int64_t next_root_atom; ///< offset of the next root atom
int export_all;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a15c8d1..1d80c09 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4848,7 +4848,8 @@ static int mov_read_default(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
if (atom.size >= 8) {
a.size = avio_rb32(pb);
a.type = avio_rl32(pb);
-if (a.type == MKTAG('f','r','e','e') &&
+if (!c->ignore_moov_atom_in_free &&
+a.type == MKTAG('f','r','e','e') &&
a.size >= 8 &&
c->moov_retry) {
uint8_t buf[8];
@@ -5926,6 +5927,8 @@ static const AVOption mov_options[] = {
0, 1, FLAGS},
{"ignore_chapters", "", OFFSET(ignore_chapters), AV_OPT_TYPE_BOOL, {.i64 =
0},
0, 1, FLAGS},
+{"ignore_moov_atom_in_free", "", OFFSET(ignore_moov_atom_in_free),
AV_OPT_TYPE_BOOL,
+{.i64 = 0}, 0, 1, FLAGS},
{"use_mfra_for",
"use mfra for fragment timestamps",
OFFSET(use_mfra_for), AV_OPT_TYPE_INT, {.i64 = FF_MOV_FLAG_MFRA_AUTO},
--
2.8.0.rc3.226.g39d4020
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] PATCH: dshow prevent some windows 10 anniv. ed crashes
On 9/5/16, Roger Pack wrote: > On 9/4/16, Carl Eugen Hoyos wrote: >> Hi! >> >> 2016-08-20 12:09 GMT+02:00 Timo Rothenpieler : >>> On 8/19/2016 3:28 PM, Roger Pack wrote: No complaints, would someone please push it for me? Sorry still haven't figured out the key thing yet. >>> >>> pushed >> >> Did this fix ticket #5775? OK I heard a rumor microsoft had fixed it on their end, so feel free to close that ticket now. Thanks. > Not entirely, still readying another patch FWIW. > >> Carl Eugen >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] img2: added support for %t output pattern
On 10/16/16, Michael Niedermayer wrote:
> On Mon, Oct 10, 2016 at 02:56:24PM -0600, Roger Pack wrote:
>> On 9/22/16, Roger Pack wrote:
>> > On 1/4/12, Yuval Adam wrote:
>> >> From: Yuval Adam
>> >>
>> >> The image2 muxer now supports timestamps in output filenames.
>> >> When used in an output patterm '%t' will be replaced with the frames
>> >> timestamp in hours, minutes and seconds (hh:mm:ss).
>> >
>> > A somewhat updated (but not yet cleaned up) revision:
>> >
>> > https://gist.github.com/rdp/e518616f2a702367ae5a922b56e09e04
>> >
>> > see also https://trac.ffmpeg.org/ticket/1452
>>
>> OK attached is the "cleaned up" patch, ready for review/commit.
>>
>> how to test:
>> (apply then) run this:
>>
>> ./ffmpeg -i input -copyts -vsync vfr temp/abc-%d-%t.jpeg
>> and compare filenames with the timestamps from video packets of
>> ffprobe -show_packets.
>>
>> Probably a better way would have been to mix it into
>> av_bprint_strftime however I wasn't sure how to use that within
>> libavformat/utils.c av_get_frame_filename2
>>
>> Adam's initial patch
>> (https://github.com/yuvadm/FFmpeg/commit/0eb002821a2076cb3593c823399aeef9fdd29525)
>> also deprecated av_get_frame_filename
>>
>> but I wasn't sure if we wanted that here or not so didn't include it.
>> Thank you for your consideration.
>> -roger-
>
>> doc/muxers.texi| 19 ---
>> libavformat/avformat.h |3 ++-
>> libavformat/hlsenc.c |6 +++---
>> libavformat/img2enc.c |7 +--
>> libavformat/utils.c| 36
>> 5 files changed, 58 insertions(+), 13 deletions(-)
>> 06950fc8ba5a9163ffb838a2bff9933e69255b41
>> 0001-img2-encoder-allow-t-in-filename-based-on-patch-from.patch
>> From 11deddfacc595c43a4f542fffe5e90b142e39c85 Mon Sep 17 00:00:00 2001
>> From: rogerdpack
>> Date: Mon, 10 Oct 2016 14:50:20 -0600
>> Subject: [PATCH] img2 encoder: allow %t in filename, based on patch from
>> Yuval
>> Adam
>>
>> Signed-off-by: rogerdpack
>> ---
>> doc/muxers.texi| 19 ---
>> libavformat/avformat.h | 3 ++-
>> libavformat/hlsenc.c | 6 +++---
>> libavformat/img2enc.c | 7 +--
>> libavformat/utils.c| 36
>> 5 files changed, 58 insertions(+), 13 deletions(-)
>>
>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> index 9ec2e31..6fff966 100644
>> --- a/doc/muxers.texi
>> +++ b/doc/muxers.texi
>> @@ -619,6 +619,12 @@ If the pattern contains "%d" or "%0@var{N}d", the
>> first filename of
>> the file list specified will contain the number 1, all the following
>> numbers will be sequential.
>>
>> +If the pattern contains "%t", the frame's timestamps will be inserted
>> +in the filename like "00.00.00.000" for hours, minutes, seconds,
>> +and milliseconds.
>> +
>> +The "%t" and "%d" patterns may be used simultaneously.
>> +
>> The pattern may contain a suffix which is used to automatically
>> determine the format of the image files to write.
>>
>> @@ -635,7 +641,7 @@ The following example shows how to use
>> @command{ffmpeg} for creating a
>> sequence of files @file{img-001.jpeg}, @file{img-002.jpeg}, ...,
>> taking one image every second from the input video:
>> @example
>> -ffmpeg -i in.avi -vsync 1 -r 1 -f image2 'img-%03d.jpeg'
>> +ffmpeg -i in.avi -vsync cfr -r 1 -f image2 'img-%03d.jpeg'
>> @end example
>>
>> Note that with @command{ffmpeg}, if the format is not specified with the
>> @@ -643,12 +649,12 @@ Note that with @command{ffmpeg}, if the format is
>> not specified with the
>> format, the image2 muxer is automatically selected, so the previous
>> command can be written as:
>> @example
>> -ffmpeg -i in.avi -vsync 1 -r 1 'img-%03d.jpeg'
>> +ffmpeg -i in.avi -vsync cfr -r 1 'img-%03d.jpeg'
>> @end example
>>
>> Note also that the pattern must not necessarily contain "%d" or
>> "%0@var{N}d", for example to create a single image file
>> -@file{img.jpeg} from the input video you can employ the command:
>> +@file{img.jpeg} from the start of the input video you can employ the
>> command:
>> @example
>> ffmpeg -i in.avi -f image2 -frames:v 1 img.jpeg
>> @end example
>> @@ -664,6 +670,13 @@ can be used:
>> ffmpeg -f v4l2 -r 1 -i /dev/video0 -f image2 -strftime 1
>> "%Y-%m-%d_%H-%M-%S.jpg"
>> @end example
>>
>> +The following example uses the timestamp parameter to generate one
>> +image file per video frame from the input, and name it including its
>> original
>> +timestamp.
>> +@example
>> +ffmpeg -i in.avi -vsync vfr -copyts img-%t.jpg
>> +@end example
>> +
>> @subsection Options
>>
>> @table @option
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 057f8c5..4eeb6f4 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -2744,10 +2744,11 @@ void av_dump_format(AVFormatContext *ic,
>> * @param path numbered sequence string
>> * @param number frame number
>> * @param flags AV_FRAME_FILENAME_FLAGS_*
>> + * @param ts frame timestamp in AV_TIME_BASE f
Re: [FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.
2016-10-24 23:11 GMT+02:00 Zhenni Huang :
[...]
Does the following inlined patch help you?
Carl Eugen
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 357d800..ed099fc 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4884,6 +4884,7 @@
a.type = avio_rl32(pb);
if (a.type == MKTAG('f','r','e','e') &&
a.size >= 8 &&
+c->fc->strict_std_compliance < FF_COMPLIANCE_VERY_STRICT &&
c->moov_retry) {
uint8_t buf[8];
uint32_t *type = (uint32_t *)buf + 1;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop
On Mon, Oct 24, 2016 at 10:31 PM, Ronald S. Bultje wrote: > Hi, > > On Mon, Oct 24, 2016 at 4:26 PM, Henrik Gramner wrote: > >> On Mon, Oct 24, 2016 at 9:59 PM, Ronald S. Bultje >> wrote: >> > Good idea to reference Hendrik Gramner here, who keeps insisting we get >> rid >> > of all MMX code in ffmpeg (at least as an option) for future Intel CPUs >> in >> > which MMX will be deprecated. >> >> Replacing MMX with SSE2 is indeed the most "proper" fix in my opinion, >> but it's a fair amount of work and not done in an evening. >> >> The fact that a lot of assembly lacks unit tests is certainly not >> helping in that regard. >> >> Some MMX instructions are slower than the equivalent SSE2 code on >> Skylake. Intel hasn't officially commented on (as far as I know at >> least) if we should expect this trend to continue, but they certainly >> seem to treat MMX as legacy. >> >> I doubt they would completely remove support for it though, backwards >> compatibility is a big selling-point for x86. > > > Well, it gives us another way of fixing this issue (on x86-64 only): have > sse2 implementations for all code that has a mmx (register) path right now. > I don't think the argument for pre-sse2 CPUs is that strong on 32-bit systems, either. I wouldn't necessarily limit this to x86-64 at that, considering its probably another half year at least, probably longer, until such a conversion could be completed. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] libavcodec : add psd image file decoder
2016-10-24 22:21 GMT+02:00 Martin Vignali : > Hello, > > In attach new patchs. > /* reorganize uncompress data. Each channel is stored one after the other */ (Sorry if I misread the code.) Did you see AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRAP, AV_PIX_FMT_GBRP16 and AV_PIX_FMT_GBRAP16? And the more I think of it, the more likely it seems that (GBR) 9, 10, 12 and 14 could also be of use. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Use the correct timescale when seeking for audio.
On Sun, Oct 23, 2016 at 10:41:52PM -0700, Sasi Inguva wrote: > Attaching the fate sample. uploaded thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you drop bombs on a foreign country and kill a hundred thousand innocent people, expect your government to call the consequence "unprovoked inhuman terrorist attacks" and use it to justify dropping more bombs and killing more people. The technology changed, the idea is old. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.
Hi Carl,
Thanks for your reply. Setting strict_std_compliance to 2 could help in
this case. However, as it is a global flag, it could influence other parts
in demuxers. It is preferable if we can have control of whether to use moov
in free with one separate flag.
Thanks,
Zhenni
On Mon, Oct 24, 2016 at 2:54 PM, Carl Eugen Hoyos
wrote:
> 2016-10-24 23:11 GMT+02:00 Zhenni Huang ffmpeg.org>:
>
> [...]
>
> Does the following inlined patch help you?
>
> Carl Eugen
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 357d800..ed099fc 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4884,6 +4884,7 @@
> a.type = avio_rl32(pb);
> if (a.type == MKTAG('f','r','e','e') &&
> a.size >= 8 &&
> +c->fc->strict_std_compliance < FF_COMPLIANCE_VERY_STRICT
> &&
> c->moov_retry) {
> uint8_t buf[8];
> uint32_t *type = (uint32_t *)buf + 1;
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms, so apps can have flexibility to use moov atom not in free atoms as default.
2016-10-25 1:23 GMT+02:00 Zhenni Huang : > Thanks for your reply. Setting strict_std_compliance to 2 could > help in this case. However, as it is a global flag, it could influence > other parts in demuxers. Yes, this is intended: If you don't want to read invalid mov files, it seems logical that you don't want to read other invalid files. Or do you have another use case? > It is preferable if we can have control of whether to use > moov in free with one separate flag. I disagree. Please do not top-post here, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec: validate codec parameters in avcodec_parameters_to_context
This should reduce the impact of a demuxer (or API user) setting bogus
codec parameters.
Suggested-by: wm4
Signed-off-by: Andreas Cadhalpun
---
libavcodec/utils.c | 82 +-
1 file changed, 81 insertions(+), 1 deletion(-)
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 87de15f..9977ffd 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -4227,8 +4227,20 @@ int avcodec_parameters_to_context(AVCodecContext *codec,
codec->codec_id = par->codec_id;
codec->codec_tag = par->codec_tag;
+if (par->bit_rate < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid bit rate %"PRId64"\n",
par->bit_rate);
+return AVERROR(EINVAL);
+}
codec->bit_rate = par->bit_rate;
+if (par->bits_per_coded_sample < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid bits per coded sample %d\n",
par->bits_per_coded_sample);
+return AVERROR(EINVAL);
+}
codec->bits_per_coded_sample = par->bits_per_coded_sample;
+if (par->bits_per_raw_sample < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid bits per raw sample %d\n",
par->bits_per_raw_sample);
+return AVERROR(EINVAL);
+}
codec->bits_per_raw_sample = par->bits_per_raw_sample;
codec->profile = par->profile;
codec->level = par->level;
@@ -4236,42 +4248,110 @@ int avcodec_parameters_to_context(AVCodecContext
*codec,
switch (par->codec_type) {
case AVMEDIA_TYPE_VIDEO:
codec->pix_fmt= par->format;
+if ( (par->width || par->height) && av_image_check_size(par->width,
par->height, 0, codec) < 0)
+return AVERROR(EINVAL);
codec->width = par->width;
codec->height = par->height;
codec->field_order= par->field_order;
+if (par->color_range < 0 || par->color_range > AVCOL_RANGE_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color range %d\n",
par->color_range);
+return AVERROR(EINVAL);
+}
codec->color_range= par->color_range;
+if (par->color_primaries < 0 || par->color_primaries > AVCOL_PRI_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color primaries %d\n",
par->color_primaries);
+return AVERROR(EINVAL);
+}
codec->color_primaries= par->color_primaries;
+if (par->color_trc < 0 || par->color_trc > AVCOL_TRC_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color transfer
characteristics %d\n", par->color_trc);
+return AVERROR(EINVAL);
+}
codec->color_trc = par->color_trc;
+if (par->color_space < 0 || par->color_space > AVCOL_SPC_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid color space %d\n",
par->color_space);
+return AVERROR(EINVAL);
+}
codec->colorspace = par->color_space;
+if (par->chroma_location < 0 || par->chroma_location >
AVCHROMA_LOC_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid chroma location %d\n",
par->chroma_location);
+return AVERROR(EINVAL);
+}
codec->chroma_sample_location = par->chroma_location;
+if (par->sample_aspect_ratio.num < 0 || par->sample_aspect_ratio.den <
0) {
+av_log(codec, AV_LOG_ERROR, "Invalid sample aspect ratio %d/%d\n",
+ par->sample_aspect_ratio.num, par->sample_aspect_ratio.den);
+return AVERROR(EINVAL);
+}
codec->sample_aspect_ratio= par->sample_aspect_ratio;
+if (par->video_delay < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid video delay %d\n",
par->video_delay);
+return AVERROR(EINVAL);
+}
codec->has_b_frames = par->video_delay;
break;
case AVMEDIA_TYPE_AUDIO:
+if (par->format < -1 || par->format > AV_SAMPLE_FMT_NB) {
+av_log(codec, AV_LOG_ERROR, "Invalid sample format %d\n",
par->format);
+return AVERROR(EINVAL);
+}
codec->sample_fmt = par->format;
codec->channel_layout = par->channel_layout;
+if (par->channels < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid number of channels %d\n",
par->channels);
+return AVERROR(EINVAL);
+}
codec->channels = par->channels;
+if (par->sample_rate < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid sample rate %d\n",
par->sample_rate);
+return AVERROR(EINVAL);
+}
codec->sample_rate = par->sample_rate;
+if (par->block_align < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid block align %d\n",
par->block_align);
+return AVERROR(EINVAL);
+}
codec->block_align = par->block_align;
+if (par->frame_size < 0) {
+av_log(codec, AV_LOG_ERROR, "Invalid frame size
[FFmpeg-devel] [PATCH] scale_npp: fix passthrough mode
The pass through mode is broken, since error is returned when none of the stages is needed. In such a case only 0 should be returned. Try command line with i/p resolution = o/p resolution to observe the issue. Sample Command Line : ffmpeg -y -hwaccel cuvid -c:v h264_cuvid -vsync 0 -i amazing_10.mp4 -vf scale_npp=1920:1072 -vcodec h264_nvenc tmp0.mp4 (amazing.mp4 should be 1920x1072 in this case) Thanks, Yogender --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- 0001-scale_npp-fix-passthrough-mode.patch Description: 0001-scale_npp-fix-passthrough-mode.patch ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] is it possible that avfilter graph updates its setting in realtime?
Hi, I have one question about some rare usage for avfilter graph: Sometimes, I want to change the setting in avfilter graph. For example, fps filter is used to set output frame rate, and it's expected that fps can be changed in accordance to real needs. Sometimes, fps is set to 25, and later 15 as network bandwidth is not good. Is it possible that avfilter graph updates its setting in realtime, such as fps in fps filter, and widthxheight in scale filter? Thanks! Regards Andrew ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
