Re: [FFmpeg-devel] [PATCH] avutil/tests: Improved code coverage for random_seed

[James Almer]

On 12/22/2016 9:12 PM, Thomas Turner wrote:
> Signed-off-by: Thomas Turner 
> ---
>  libavutil/tests/random_seed.c | 34 +-
>  tests/ref/fate/random_seed|  1 +
>  2 files changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
> index ebe9b3e..fcd68bc 100644
> --- a/libavutil/tests/random_seed.c
> +++ b/libavutil/tests/random_seed.c
> @@ -23,24 +23,32 @@
>  
>  #undef printf
>  #define N 256
> +#define F 2
>  #include 
>  
> +typedef uint32_t (*random_seed_ptr_t)(void);
> +
>  int main(void)
>  {
> -int i, j, retry;
> +int i, j, rsf, retry;
>  uint32_t seeds[N];
> +random_seed_ptr_t random_seed[F] = {av_get_random_seed, 
> get_generic_seed};
>  
> -for (retry=0; retry<3; retry++){
> -for (i=0; i -seeds[i] = av_get_random_seed();
> -for (j=0; j -if (seeds[j] == seeds[i])
> -goto retry;
> +for (rsf=0; rsf +for (retry=0; retry<3; retry++){
> +for (i=0; i +seeds[i] = random_seed[rsf]();
> +for (j=0; j +if (seeds[j] == seeds[i])
> +goto retry;
> +}
> +printf("seeds OK\n");
> +goto next;
> +retry:;
>  }
> -printf("seeds OK\n");
> -return 0;
> -retry:;
> +printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
> +return 1;
> +next:;
>  }
> -printf("FAIL at %d with %X\n", j, seeds[j]);
> -return 1;
> -}
> +return 0;
> + }
> \ No newline at end of file
> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
> index 2b5b3af..ef0eef2 100644
> --- a/tests/ref/fate/random_seed
> +++ b/tests/ref/fate/random_seed
> @@ -1 +1,2 @@
>  seeds OK
> +seeds OK

This is making the test run for an absurd amount of time, to the point
FATE clients are killing the process because it was just not stopping.

Before this it would take a few milliseconds.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/utvideodec: add SIMD support for median prediction for planar formats

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 05:44:22PM +0100, Paul B Mahol wrote:
> ffmpeg | branch: master | Paul B Mahol  | Fri Dec 23 
> 15:41:51 2016 +0100| [ea93052db3594f93f2d10be085a770184da0513d] | committer: 
> Paul B Mahol
> 
> avcodec/utvideodec: add SIMD support for median prediction for planar formats
> 
> ~10% faster overall.
> 
> Signed-off-by: Paul B Mahol 
> 
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=ea93052db3594f93f2d10be085a770184da0513d
> ---
> 
>  configure   |   2 +-
>  libavcodec/utvideo.h|   2 +
>  libavcodec/utvideodec.c | 164 
> +++-
>  libavcodec/utvideoenc.c |   3 +
>  4 files changed, 140 insertions(+), 31 deletions(-)

This breaks
> create a file with: (can be any ffmpeg version)
> ./ffmpeg -i matrixbench_mpeg2.mpg -an -vcodec utvideo -t 1 -pix_fmt yuv420p 
> -pred median utv.avi
>
> if this is played with ffplay (with patch) it shows a brighter
> artifact on the left side of te screen

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi/buffersrc: push the frame deeper if requested.

[Michael Niedermayer]

On Sat, Dec 24, 2016 at 03:18:11AM +0100, Michael Niedermayer wrote:
> On Sat, Dec 24, 2016 at 03:10:28AM +0100, Michael Niedermayer wrote:
> > On Fri, Dec 23, 2016 at 09:49:52PM +0100, Nicolas George wrote:
> > > Reduce peak memory consumption with ffmpeg in certain cases.
> > > 
> > > Signed-off-by: Nicolas George 
> > > ---
> > >  libavfilter/buffersrc.c | 25 +
> > >  1 file changed, 25 insertions(+)
> > 
> > deadlocks
> > 
> > ./ffmpeg -i mpegts_with_dvbsubs.ts   -filter_complex 
> > '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]'
> >  -map '[v]' -map '#0x1048' -t 1 a.mkv
> > 
> > http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts
> > 
> > also this patch changes the output of ffmpeg in many cases
> 
> heres one case that shows artifacts
> create a file with: (can be any ffmpeg version)
> ./ffmpeg -i matrixbench_mpeg2.mpg -an -vcodec utvideo -t 1 -pix_fmt yuv420p 
> -pred median utv.avi
> 
> if this is played with ffplay (with patch) it shows a brighter
> artifact on the left side of te screen

the utvideo change is caused by ea93052db3594f93f2d10be085a770184da0513d.
not by this patch. i had both locally and just now double checked

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi/buffersrc: push the frame deeper if requested.

[Michael Niedermayer]

On Sat, Dec 24, 2016 at 03:10:28AM +0100, Michael Niedermayer wrote:
> On Fri, Dec 23, 2016 at 09:49:52PM +0100, Nicolas George wrote:
> > Reduce peak memory consumption with ffmpeg in certain cases.
> > 
> > Signed-off-by: Nicolas George 
> > ---
> >  libavfilter/buffersrc.c | 25 +
> >  1 file changed, 25 insertions(+)
> 
> deadlocks
> 
> ./ffmpeg -i mpegts_with_dvbsubs.ts   -filter_complex 
> '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]'
>  -map '[v]' -map '#0x1048' -t 1 a.mkv
> 
> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts
> 
> also this patch changes the output of ffmpeg in many cases

heres one case that shows artifacts
create a file with: (can be any ffmpeg version)
./ffmpeg -i matrixbench_mpeg2.mpg -an -vcodec utvideo -t 1 -pix_fmt yuv420p 
-pred median utv.avi

if this is played with ffplay (with patch) it shows a brighter
artifact on the left side of te screen


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi/buffersrc: push the frame deeper if requested.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 09:49:52PM +0100, Nicolas George wrote:
> Reduce peak memory consumption with ffmpeg in certain cases.
> 
> Signed-off-by: Nicolas George 
> ---
>  libavfilter/buffersrc.c | 25 +
>  1 file changed, 25 insertions(+)

deadlocks

./ffmpeg -i mpegts_with_dvbsubs.ts   -filter_complex 
'[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]'
 -map '[v]' -map '#0x1048' -t 1 a.mkv

http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts

also this patch changes the output of ffmpeg in many cases


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 11:46:07PM +0100, Nicolas George wrote:
> Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> > shouldnt there be a public annoncement about the intend to make the API 
> > public
> > shouldnt there be a public call for API design suggestions and discussion
> > shouldnt there be a public call for API related patches with deadline
> > shouldnt there be a go/no-go poll of the FFmpeg developers
> 
> I am not sure what you are angling at here. I announced my projects to
> rework the internal workings and global API when I started working on
> making request_frame non-recursive (note: request_frame, not
> filter_frame; this patch landed more than a year ago) and mentioned them
> several times since then, for example when Paul considered working on
> threading IIRC. I did not gave that much details because nobody seemed
> interested in them.
> 
> And of course, no significant change in API or design went in without
> staying for quite a time on the mailing-list.

you snipped the context and reply to the quote as if it was meant
in a different context.
Not everyone intends to attack you :)

The quoted text are the steps which IMHO make sense to design a
API and to then make it public. What you talk about are the steps that
make sense to get the changes you want in.
theres nothing wrong with that it just has a differnt goal and i
thought we talk about the other.


> 
> > I dont think a wait period makes sense. By the time everyone is ok
> > with making the API public the code will have been more then
> > extensivly tested and waiting further would likely add little
> 
> So you are saying that the wait period will take care of itself even if
> we do not decide to enforce it. I am ok with it.
> 

> And, after all, what do you want, concretely?

as you ask ...

Id like to see libavfilter be less centralized, id like to be able
to write a filter and know that if it gets rejected on ffmpeg-dev that
i can still publish it on my own git repo and everyone could easily use
it. (as with plugins which could have bin packages in distros for easy
use not requireng users to patch / merge source)
This is an especially annoying situation when theres a consulting job
and then someone decides to demand some change which practically is
preventing the actual usecase one is paid for to be solved.

If its not a consulting job droping some patch isnt a huge
problem if a reviewer is stubborn and insists on something which is
unpractical.
But if one is doing the work for someone else, droping a patch is much
more annoying. Being able to create a plugin sidesteps this.


> 
> I, personally, am not working on making external filters possible. But I
> am not preventing it either. (Well, this patch makes it a little bit
> more work, but just work, not hard.) And I think that when I am done
> with my plan, it will be actually easier because a cleaner internal API
> is easier to make public.
> 
> Another point: this particular patch, and the series that surrounds it,
> I do not really want it. For all I care, we could drop it (either all
> the series, or just 5-6 because 1-4 make the code clearer).
> 

> On the other hand, Andreas, Clément, Hendrik and James would not be
> happy, because they insisted to hide the innards of AVFilterLink more
> deeply after the filter_frame patch. This was discussed on the mailing
> list. I probably should let you discuss the issue with them. The code is
> written, it did not take much time and does not interact with my current
> work: it can be applied or dropped indifferently.

My oppinion is that If we intend to have AVFilterLink public in the
future then making it private now is a bad idea and wasted time.

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

> Hello everybody,
>
> it's been a while since my contribution to matroskaenc in July. In case you 
> don't remember my scenario - it's about realtime transcoding and and 
> streaming of mkv files.
> That means that the header is sent to the client before the ffmpeg process is 
> completed.

>> can you identify (with git bisect) which change made the difference ?

Hello Michael,

it seems that this error has been introduced with commit 
ee888cfbe777cd2916a3548c750e433ab8f8e6a5.

Best regards,

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec: add Apple Pixlet decoder

[Andreas Cadhalpun]

On 23.12.2016 09:17, Paul B Mahol wrote:
> On 12/23/16, Andreas Cadhalpun  wrote:
>> On 22.12.2016 22:52, Paul B Mahol wrote:
>>> +xflag = flag + cnt1;
>>> +yflag = xflag;
>>> +
>>> +if (flag + cnt1 == 0) {
>>> +value = 0;
>>> +} else {
>>> +xflag &= 1u;
>>> +tmp = c * ((yflag + 1) >> 1) + (c >> 1);
>>
>> This can overflow.
> 
> And?

That is undefined behavior. It looks like at least a cast to int64_t is missing.

>>> +value = xflag + (tmp ^ -xflag);
>>> +}
>>> +
>>> +i++;
>>> +dst[j++] = value;
>>> +if (j == width) {
>>> +j = 0;
>>> +dst += stride;
>>> +}
>>> +state += d * yflag - (d * state >> 8);
>>
>> This can overflow, too.
> 
> And?

Same as above.

>> [...]
>>> +static void postprocess_chroma(AVFrame *frame, int w, int h, int depth)
>>> +{
>>> +uint16_t *dstu = (uint16_t *)frame->data[1];
>>> +uint16_t *dstv = (uint16_t *)frame->data[2];
>>> +int16_t *srcu  = (int16_t *)frame->data[1];
>>> +int16_t *srcv  = (int16_t *)frame->data[2];
>>> +ptrdiff_t strideu = frame->linesize[1] / 2;
>>> +ptrdiff_t stridev = frame->linesize[2] / 2;
>>> +const int add = 1 << (depth - 1);
>>> +const int shift = 16 - depth;
>>> +int i, j;
>>> +
>>> +for (j = 0; j < h; j++) {
>>> +for (i = 0; i < w; i++) {
>>> +dstu[i] = (add + srcu[i]) << shift;
>>> +dstv[i] = (add + srcv[i]) << shift;
>>
>> These result in undefined shifts, since the value to be shifted can be
>> negative.
> 
> OK.

Making add unsigned would fix that.

>>> +}
>>> +dstu += strideu;
>>> +dstv += stridev;
>>> +srcu += strideu;
>>> +srcv += stridev;
>>> +}
>>> +}
>>> +
>>> +static int decode_plane(AVCodecContext *avctx, int plane, AVPacket
>>> *avpkt, AVFrame *frame)
>>> +{
>>> +PixletContext *ctx = avctx->priv_data;
>>> +ptrdiff_t stride = frame->linesize[plane] / 2;
>>> +unsigned shift = plane > 0;
>>> +int16_t *dst;
>>> +int i, ret;
>>> +
>>> +for (i = ctx->levels - 1; i >= 0; i--) {
>>> +ctx->scaling[plane][H][i] = 100. /
>>> sign_extend(bytestream2_get_be32(>gb), 32);
>>> +ctx->scaling[plane][V][i] = 100. /
>>> sign_extend(bytestream2_get_be32(>gb), 32);
>>
>> Here the denominators can be zero.
> 
> And?

That is undefined behavior.

>>> +*got_frame = 1;
>>> +
>>> +return pktsize;
>>
>> Since this is a video decoder, this should always return the full
>> avpkt->size.
> 
> Wrong. It returns what it consumes.

Video decoders are expected to consume the full packet and the new decoding API 
does that.
So not returning the full packet size results in a behavior difference between 
the new
and the old API, which is bad. Why would you want that?

Best regards,
Andreas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] Size of bufferqueue (was: [PATCH 2/2] lavfi: make filter_frame) non-recursive.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Marton Balint a écrit :
> I guess we could buffer the undecoded packets instead of the decoded frames
> if there is a higher-than-usual delay between the streams. Is this also your
> plan?

Well, at some point in the far future I would have libavfilter capable
of handling AVMEDIA_TYPE_DATA frames and codecs as filters. If that
happens, then tweaking the scheduling to prioritize consuming large
frames first would be useful.

But that is not the issue at all.

The problem with bufferqueue is this:

For normal cases, cases that are supposed to work, the need of
bufferqueue is bounded. But with some codecs (those with huge frames
that get split, for example) or formats (depending on the A-V muxing
delay), it can sometimes be quite large. You observed that 64 is not
enough for your uses but 256 is. Probably, someone somewhere needs 1024
or more.

Increasing the size of the queue wastes a little memory. Not much, but
still not satisfactory. The obvious solution is to make it dynamic.
Inserting the fifo filter as input achieves it, because fifo has an
unlimited buffer, not implemented using bufferqueue. Bug fifo breaks the
scheduling because the buffer is hidden.

The new data structure I added to AVFilterLink a few days ago is exactly
what we need, it gives us a dynamic FIFO that works fine with the
scheduling. It still requires some code to permit filters to use it
directly (done, in my work tree), and then adapt amerge and the other
filters to do it (not yet done).

But there is a catch: sometimes people do something crazy, often
inadvertently, and that causes unlimited amounts of frames to accumulate
in the filter's input before any processing can be done.

Currently, when that happens, they see "buffer queue overflow" and the
encoding stops, and they come and ask for help on the mailing-list.

If the queue is not bounded, it will grow, eat all memory, then start
filling the swap, until the OOM-killer intervenes. Not good, really not
good.

My solution: make the queue unbounded in principle, but keep stats on
the use and add a configurable limit. Quite obvious, actually.

I have two bonus in my plans.

First, make the stats global to the filter graph. Instead of limiting to
(numbers completely arbitrary) 1000 frames on each of the 2 inputs of
each of the 4 instances of amerge (which would never be used, since when
all inputs on amerge have frames, amerge can process), limit to 8000
frames on the whole filter graph.

Second, keep stats on the approximate memory use of the frames, and set
a limit on that. That way, we can allow thousandths of 16-samples
frames, but bail after only a dozen 4k video frames.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/1] flv demuxer supports live rtmp inputs but there is no any info about it in the docs

[Bodecs Bela]

>From af75a5bbfe9f37672478b6a67f8bcfe32b7b997f Mon Sep 17 00:00:00 2001
From: Bela Bodecs 
Date: Sat, 24 Dec 2016 00:21:23 +0100
Subject: [PATCH 1/1] flv demuxer supports live rtmp inputs but there is no any
 info about it in the docs.

Signed-off-by: Bela Bodecs 
---
 doc/demuxers.texi | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 2934a1c..c12c07e 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -243,11 +243,17 @@ file subdir/file-2.wav
 @end example
 @end itemize
 
-@section flv
+@section flv, live_flv
 
 Adobe Flash Video Format demuxer.
 
-This demuxer is used to demux FLV files and RTMP network streams.
+This demuxer is used to demux FLV files and RTMP network streams. In case of live network streams, if you force format, you may use live_flv option instead of flv to survive timestamp discontinuities.
+
+@example
+ffmpeg -f flv -i myfile.flv ...
+ffmpeg -f live_flv -i rtmp:///anything/key 
+@end example
+
 
 @table @option
 @item -flv_metadata @var{bool}
-- 
2.5.3.windows.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/magicyuv: add SIMD for median of 10bits

[Ronald S. Bultje]

Hi,

On Fri, Dec 23, 2016 at 6:18 PM, James Almer  wrote:

> On 12/23/2016 8:00 PM, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Fri, Dec 23, 2016 at 12:32 PM, Paul B Mahol  wrote:
> >
> >> diff --git a/libavcodec/lossless_videodsp.h b/libavcodec/lossless_
> >> videodsp.h
> >>
> > [..]
> >
> >> @@ -32,6 +32,7 @@ typedef struct LLVidDSPContext {
> >>
> > [..]
> >
> >> +void (*add_magy_median_pred_int16)(uint16_t *dst, const uint16_t
> >> *top, const uint16_t *diff, unsigned mask, int w, int *left, int
> *left_top);
> >>
> >
> > That seems wrong. Why would you add a magicuv-specific function to
> > losslessdsp-context which is intended for functions shared between many
> > (not just one) lossless codecs? You probably want a new dsp for magicyuv
> > specifically.
> >
> > I know this is tedious, but we're very specifically trying to prevent
> > dsputil from ever happening again.
> >
> > Ronald
>
> Some functions in this dsp are used only by huffyuv. Only one is used by
> both huffyuv and magicyuv.
> To properly apply what you mention, it would need to be split in two,
> huffyuvdsp and lldsp, then this new function added to a new dsp called
> magicyuvdsp.


That would be even better, yes.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/magicyuv: add SIMD for median of 10bits

[James Almer]

On 12/23/2016 8:00 PM, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Dec 23, 2016 at 12:32 PM, Paul B Mahol  wrote:
> 
>> diff --git a/libavcodec/lossless_videodsp.h b/libavcodec/lossless_
>> videodsp.h
>>
> [..]
> 
>> @@ -32,6 +32,7 @@ typedef struct LLVidDSPContext {
>>
> [..]
> 
>> +void (*add_magy_median_pred_int16)(uint16_t *dst, const uint16_t
>> *top, const uint16_t *diff, unsigned mask, int w, int *left, int *left_top);
>>
> 
> That seems wrong. Why would you add a magicuv-specific function to
> losslessdsp-context which is intended for functions shared between many
> (not just one) lossless codecs? You probably want a new dsp for magicyuv
> specifically.
> 
> I know this is tedious, but we're very specifically trying to prevent
> dsputil from ever happening again.
> 
> Ronald

Some functions in this dsp are used only by huffyuv. Only one is used by
both huffyuv and magicyuv.
To properly apply what you mention, it would need to be split in two,
huffyuvdsp and lldsp, then this new function added to a new dsp called
magicyuvdsp.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/magicyuv: add SIMD for median of 10bits

[Ronald S. Bultje]

Hi,

On Fri, Dec 23, 2016 at 12:32 PM, Paul B Mahol  wrote:

> diff --git a/libavcodec/lossless_videodsp.h b/libavcodec/lossless_
> videodsp.h
>
[..]

> @@ -32,6 +32,7 @@ typedef struct LLVidDSPContext {
>
[..]

> +void (*add_magy_median_pred_int16)(uint16_t *dst, const uint16_t
> *top, const uint16_t *diff, unsigned mask, int w, int *left, int *left_top);
>

That seems wrong. Why would you add a magicuv-specific function to
losslessdsp-context which is intended for functions shared between many
(not just one) lossless codecs? You probably want a new dsp for magicyuv
specifically.

I know this is tedious, but we're very specifically trying to prevent
dsputil from ever happening again.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> shouldnt there be a public annoncement about the intend to make the API public
> shouldnt there be a public call for API design suggestions and discussion
> shouldnt there be a public call for API related patches with deadline
> shouldnt there be a go/no-go poll of the FFmpeg developers

I am not sure what you are angling at here. I announced my projects to
rework the internal workings and global API when I started working on
making request_frame non-recursive (note: request_frame, not
filter_frame; this patch landed more than a year ago) and mentioned them
several times since then, for example when Paul considered working on
threading IIRC. I did not gave that much details because nobody seemed
interested in them.

And of course, no significant change in API or design went in without
staying for quite a time on the mailing-list.

> I dont think a wait period makes sense. By the time everyone is ok
> with making the API public the code will have been more then
> extensivly tested and waiting further would likely add little

So you are saying that the wait period will take care of itself even if
we do not decide to enforce it. I am ok with it.

And, after all, what do you want, concretely?

I, personally, am not working on making external filters possible. But I
am not preventing it either. (Well, this patch makes it a little bit
more work, but just work, not hard.) And I think that when I am done
with my plan, it will be actually easier because a cleaner internal API
is easier to make public.

Another point: this particular patch, and the series that surrounds it,
I do not really want it. For all I care, we could drop it (either all
the series, or just 5-6 because 1-4 make the code clearer).

On the other hand, Andreas, Clément, Hendrik and James would not be
happy, because they insisted to hide the innards of AVFilterLink more
deeply after the filter_frame patch. This was discussed on the mailing
list. I probably should let you discuss the issue with them. The code is
written, it did not take much time and does not interact with my current
work: it can be applied or dropped indifferently.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[Nicolas George]

Le tridi 13 frimaire, an CCXXV, Nicolas George a écrit :
> > Does this make AV_BUFFERSRC_FLAG_PUSH useless?
> For the most part, yes. I intended to do the cleanup of these small
> aspects of the API after implementing a cleaner over API.

Hum, scratch that, it is still acutely needed with certain ways of
using the API. I intend to propose to deprecate them when I have
implemented better ones, but that is for later.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 09:23:50PM +0100, Nicolas George wrote:
> Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> > APIs in FFmpeg will change as long as the project is alive.
> > 
> > new developers join, older ones leave, peoples goals and oppinions
> > change. The libavfilter API is based on the lessons learned from
> > previous projects and frameworks, in that way this codebase has a quite
> > long timeline and many experienced developers from multiple other
> > free software projects upon whos shoulders this rests in some sense.
> > 
> > If we only make the API public once the ultimate global optimum is
> > reached we will never do so.
> 
> I was not referring to a hypothetical global optimum, but right now we
> are nowhere near a local optimum: you have certainly noticed that since
> I pushed the 45k patch making filter_frame non-recursive, I had to fix
> several bugs. Well, there are still a few of them lurking, and then we
> need to reap the benefits of the design change.
> 
> Two days ago, I outlined my plans for lavfi:
> 
> https://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/204686.html
> 
> I say: first do all these points, because any of these might require a
> surprise rework of some internal API. Then (in parallel when relevant),
> adapt ffmpeg.c to work with the cleaned-up API of lavfi and fix the
> scheduling.
> 
> Then wait a year, to be sure.
> 
> Then start working on external filters.
> 
> Does it seem unreasonable?

shouldnt there be a public annoncement about the intend to make the API public
shouldnt there be a public call for API design suggestions and discussion
shouldnt there be a public call for API related patches with deadline
shouldnt there be a go/no-go poll of the FFmpeg developers

I dont think a wait period makes sense. By the time everyone is ok
with making the API public the code will have been more then
extensivly tested and waiting further would likely add little

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/avidec: skip odml master index chunks in avi_sync

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 02:50:16PM +0100, Tobias Rapp wrote:
> Fixes pts gaps when reading AVI files > 256GiB generated by FFmpeg.
> 
> Signed-off-by: Tobias Rapp 
> ---
>  libavformat/avidec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

applied

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavfi/buffersrc: push the frame deeper if requested.

[Nicolas George]

Reduce peak memory consumption with ffmpeg in certain cases.

Signed-off-by: Nicolas George 
---
 libavfilter/buffersrc.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
index 1314397a32..77fd174219 100644
--- a/libavfilter/buffersrc.c
+++ b/libavfilter/buffersrc.c
@@ -173,6 +173,20 @@ int attribute_align_arg 
av_buffersrc_add_frame_flags(AVFilterContext *ctx, AVFra
 return ret;
 }
 
+static int push_frame(AVFilterGraph *graph)
+{
+int ret;
+
+while (1) {
+ret = ff_filter_graph_run_once(graph);
+if (ret == AVERROR(EAGAIN))
+break;
+if (ret < 0)
+return ret;
+}
+return 0;
+}
+
 static int av_buffersrc_add_frame_internal(AVFilterContext *ctx,
AVFrame *frame, int flags)
 {
@@ -185,6 +199,11 @@ static int av_buffersrc_add_frame_internal(AVFilterContext 
*ctx,
 if (!frame) {
 s->eof = 1;
 ff_avfilter_link_set_in_status(ctx->outputs[0], AVERROR_EOF, 
AV_NOPTS_VALUE);
+if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
+ret = push_frame(ctx->graph);
+if (ret < 0)
+return ret;
+}
 return 0;
 } else if (s->eof)
 return AVERROR(EINVAL);
@@ -239,6 +258,12 @@ static int av_buffersrc_add_frame_internal(AVFilterContext 
*ctx,
 if ((ret = ctx->output_pads[0].request_frame(ctx->outputs[0])) < 0)
 return ret;
 
+if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
+ret = push_frame(ctx->graph);
+if (ret < 0)
+return ret;
+}
+
 return 0;
 }
 
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] lavf/matroska: comment a missing error check.

[Nicolas George]

Signed-off-by: Nicolas George 
---
 libavformat/matroskaenc.c | 1 +
 1 file changed, 1 insertion(+)


I do not have time to fix this, but I got a few segfaults here, so it needs
to be fixed. Either push the patch or, better push an actual fix.


diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 827d7550c2..8e536f6774 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -351,6 +351,7 @@ static void end_ebml_master_crc32(AVIOContext *pb, 
AVIOContext **dyn_cp, Matrosk
 
 if (pb->seekable) {
 size = avio_close_dyn_buf(*dyn_cp, );
+/* FIXME missing error check */
 if (mkv->write_crc && mkv->mode != MODE_WEBM) {
 skip = 6; /* Skip reserved 6-byte long void element from the 
dynamic buffer. */
 AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), 
UINT32_MAX, buf + skip, size - skip) ^ UINT32_MAX);
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> APIs in FFmpeg will change as long as the project is alive.
> 
> new developers join, older ones leave, peoples goals and oppinions
> change. The libavfilter API is based on the lessons learned from
> previous projects and frameworks, in that way this codebase has a quite
> long timeline and many experienced developers from multiple other
> free software projects upon whos shoulders this rests in some sense.
> 
> If we only make the API public once the ultimate global optimum is
> reached we will never do so.

I was not referring to a hypothetical global optimum, but right now we
are nowhere near a local optimum: you have certainly noticed that since
I pushed the 45k patch making filter_frame non-recursive, I had to fix
several bugs. Well, there are still a few of them lurking, and then we
need to reap the benefits of the design change.

Two days ago, I outlined my plans for lavfi:

https://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/204686.html

I say: first do all these points, because any of these might require a
surprise rework of some internal API. Then (in parallel when relevant),
adapt ffmpeg.c to work with the cleaned-up API of lavfi and fix the
scheduling.

Then wait a year, to be sure.

Then start working on external filters.

Does it seem unreasonable?

I can assure you, when the last points in the plan will be done, I will
be so fed up with it that I will easily leave the API alone for a year.
There are plenty of other parts of the code I would like to work on.

For reference, I am halfway through the second item in the plan.
Probably more than halfway, actually.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 06:51:38PM +0100, Nicolas George wrote:
> Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> > libavfilter had a public API that allowed filters to be implemented
> > these changes move it farther away from it
> > I do care about having some public API that allows filters
> > to be implemented.
> > You pointed out that you work alone and people dont review your lavfi
> > patches. I think if you integrate others goals more people would
> > 
> > For example your plans can stay the same but include the declared
> > goal of making the API public once they are all done. And suddenly
> > i would be interrested in this. Of course it doesnt make days
> > have more hours or my todo shorter but pushing the public API away
> > makes me loose interrest for sure.
> > 
> > Other people might have other ideas and goals too, which they want
> > lavfi to achieve.
> > If more things can be integrated more people might contribute ...
> 
> You misunderstood: I am not excluding an API to allow external filters,
> I am saying that it is too soon to think about it. The way filters must
> be written is in a complete rework. Once it is stabilized, tested
> extensively, we know it will not changing again, then we can make parts
> of it public to allow external filters.
> 
> Anything else would be a compatibility nightmare. Do you not agree?

APIs in FFmpeg will change as long as the project is alive.

new developers join, older ones leave, peoples goals and oppinions
change. The libavfilter API is based on the lessons learned from
previous projects and frameworks, in that way this codebase has a quite
long timeline and many experienced developers from multiple other
free software projects upon whos shoulders this rests in some sense.

If we only make the API public once the ultimate global optimum is
reached we will never do so.
What iam asking for is not that you declare a API public that you
intend to change but that we commit to declaring the API public in the
reachable future and not as a goalline that keeps shifting farther
into the future forever.
you may eventually be happy with the API in libavfilter but by the time
others with other goals and oppinions will just start their journy.

Also making the API public should be a real goal and be considered in
the steps taken 

For example if AVFilterLink isnt intended to be substantially changed
and is intended to be public when the API becomes public
then i dont see why it should not stay public
making it private, rewritig code, adding public accessors just to
then make it public again and deprecate the accessors is a huge
amount of wasted time.

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] doc/scaler: mention default scaling algorithm

[Lou Logan]

On Fri, 23 Dec 2016 03:58:16 +0100, Michael Niedermayer wrote:

> On Thu, Dec 22, 2016 at 03:39:05PM -0900, Lou Logan wrote:
> > Signed-off-by: Lou Logan 
> > ---
> >  doc/scaler.texi | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/scaler.texi b/doc/scaler.texi
> > index 3e115cd..7d482dd 100644
> > --- a/doc/scaler.texi
> > +++ b/doc/scaler.texi
> > @@ -13,7 +13,8 @@ FFmpeg tools. For programmatic use, they can be set 
> > explicitly in the
> >  @anchor{sws_flags}
> >  @item sws_flags
> >  Set the scaler flags. This is also used to set the scaling
> > -algorithm. Only a single algorithm should be selected.
> > +algorithm. Only a single algorithm should be selected. Default
> > +value is @samp{bilinear}.  
> 
> are you sure this is correct ?

That's an idiotic typo: it should have been "bicubic". I shouldn't make
patches on three hours of sleep...
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> libavfilter had a public API that allowed filters to be implemented
> these changes move it farther away from it
> I do care about having some public API that allows filters
> to be implemented.
> You pointed out that you work alone and people dont review your lavfi
> patches. I think if you integrate others goals more people would
> 
> For example your plans can stay the same but include the declared
> goal of making the API public once they are all done. And suddenly
> i would be interrested in this. Of course it doesnt make days
> have more hours or my todo shorter but pushing the public API away
> makes me loose interrest for sure.
> 
> Other people might have other ideas and goals too, which they want
> lavfi to achieve.
> If more things can be integrated more people might contribute ...

You misunderstood: I am not excluding an API to allow external filters,
I am saying that it is too soon to think about it. The way filters must
be written is in a complete rework. Once it is stabilized, tested
extensively, we know it will not changing again, then we can make parts
of it public to allow external filters.

Anything else would be a compatibility nightmare. Do you not agree?

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/magicyuv: add SIMD for median of 10bits

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavcodec/lossless_videodsp.c  | 18 ++
 libavcodec/lossless_videodsp.h  |  1 +
 libavcodec/magicyuv.c   | 23 +---
 libavcodec/x86/lossless_videodsp.asm| 62 +
 libavcodec/x86/lossless_videodsp_init.c |  2 ++
 5 files changed, 84 insertions(+), 22 deletions(-)

diff --git a/libavcodec/lossless_videodsp.c b/libavcodec/lossless_videodsp.c
index 3491621..15189f1 100644
--- a/libavcodec/lossless_videodsp.c
+++ b/libavcodec/lossless_videodsp.c
@@ -77,6 +77,23 @@ static void add_hfyu_median_pred_int16_c(uint16_t *dst, 
const uint16_t *src, con
 *left_top = lt;
 }
 
+static void add_magy_median_pred_int16_c(uint16_t *dst, const uint16_t *src, 
const uint16_t *diff, unsigned mask, int w, int *left, int *left_top){
+int i;
+uint16_t l, lt;
+
+l  = *left;
+lt = *left_top;
+
+for(i=0; iadd_hfyu_left_pred_int16   = add_hfyu_left_pred_int16_c;
 c->add_hfyu_median_pred_int16 = add_hfyu_median_pred_int16_c;
 c->sub_hfyu_median_pred_int16 = sub_hfyu_median_pred_int16_c;
+c->add_magy_median_pred_int16 = add_magy_median_pred_int16_c;
 
 if (ARCH_X86)
 ff_llviddsp_init_x86(c, avctx);
diff --git a/libavcodec/lossless_videodsp.h b/libavcodec/lossless_videodsp.h
index 040902e..c7a6881 100644
--- a/libavcodec/lossless_videodsp.h
+++ b/libavcodec/lossless_videodsp.h
@@ -32,6 +32,7 @@ typedef struct LLVidDSPContext {
 void (*sub_hfyu_median_pred_int16)(uint16_t *dst, const uint16_t *src1, 
const uint16_t *src2, unsigned mask, int w, int *left, int *left_top);
 void (*add_hfyu_median_pred_int16)(uint16_t *dst, const uint16_t *top, 
const uint16_t *diff, unsigned mask, int w, int *left, int *left_top);
 int  (*add_hfyu_left_pred_int16)(uint16_t *dst, const uint16_t *src, 
unsigned mask, int w, unsigned left);
+void (*add_magy_median_pred_int16)(uint16_t *dst, const uint16_t *top, 
const uint16_t *diff, unsigned mask, int w, int *left, int *left_top);
 } LLVidDSPContext;
 
 void ff_llviddsp_init(LLVidDSPContext *llviddsp, AVCodecContext *avctx);
diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 16d7027..f65c434 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -144,27 +144,6 @@ static int huff_build(VLC *vlc, uint8_t *len)
   syms,  sizeof(*syms),  sizeof(*syms), 0);
 }
 
-static void magicyuv_median_pred10(uint16_t *dst, const uint16_t *src1,
-   const uint16_t *diff, intptr_t w,
-   int *left, int *left_top)
-{
-int i;
-uint16_t l, lt;
-
-l  = *left;
-lt = *left_top;
-
-for (i = 0; i < w; i++) {
-l  = mid_pred(l, src1[i], (l + src1[i] - lt)) + diff[i];
-l &= 0x3FF;
-lt = src1[i];
-dst[i] = l;
-}
-
-*left = l;
-*left_top = lt;
-}
-
 static int magy_decode_slice10(AVCodecContext *avctx, void *tdata,
int j, int threadnr)
 {
@@ -265,7 +244,7 @@ static int magy_decode_slice10(AVCodecContext *avctx, void 
*tdata,
 dst += stride;
 }
 for (k = 1 + interlaced; k < height; k++) {
-magicyuv_median_pred10(dst, dst - fake_stride, dst, width, 
, );
+s->llviddsp.add_magy_median_pred_int16(dst, dst - fake_stride, 
dst, 1023, width, , );
 lefttop = left = dst[0];
 dst += stride;
 }
diff --git a/libavcodec/x86/lossless_videodsp.asm 
b/libavcodec/x86/lossless_videodsp.asm
index f06fcdf..8a2eb26 100644
--- a/libavcodec/x86/lossless_videodsp.asm
+++ b/libavcodec/x86/lossless_videodsp.asm
@@ -292,3 +292,65 @@ cglobal sub_hfyu_median_pred_int16, 7,7,0, dst, src1, 
src2, mask, w, left, left_
 movzx maskd, word [src2q + wq - 2]
 mov [leftq], maskd
 RET
+
+cglobal add_magy_median_pred_int16, 7,7,0, dst, top, diff, mask, w, left, 
left_top
+add  wd, wd
+movdmm6, maskd
+SPLATW  mm6, mm6
+movqmm0, [topq]
+movqmm2, mm0
+movdmm4, [left_topq]
+psllq   mm2, 16
+movqmm1, mm0
+por mm4, mm2
+movdmm3, [leftq]
+psubw   mm0, mm4 ; t-tl
+adddstq, wq
+addtopq, wq
+add   diffq, wq
+neg  wq
+jmp .skip
+.loop:
+movqmm4, [topq+wq]
+movqmm0, mm4
+psllq   mm4, 16
+por mm4, mm1
+movqmm1, mm0 ; t
+psubw   mm0, mm4 ; t-tl
+.skip:
+   

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 05:08:49PM +0100, Nicolas George wrote:
> Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> > From these 2 the first
> 
> Ok, that makes one.
> 
> >  but i think the user app needs more access
> > to be able to implement filters and this could make either API
> > obsoleete
> 
> User app implementing filters is not for today nor tomorrow. I think it
> is better to start thinking on how to cross that bridge when we start
> seeing it on the horizon.

libavfilter had a public API that allowed filters to be implemented
these changes move it farther away from it
I do care about having some public API that allows filters
to be implemented.
You pointed out that you work alone and people dont review your lavfi
patches. I think if you integrate others goals more people would

For example your plans can stay the same but include the declared
goal of making the API public once they are all done. And suddenly
i would be interrested in this. Of course it doesnt make days
have more hours or my todo shorter but pushing the public API away
makes me loose interrest for sure.

Other people might have other ideas and goals too, which they want
lavfi to achieve.
If more things can be integrated more people might contribute ...

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[James Almer]

On 12/23/2016 11:31 AM, Nicolas George wrote:
> L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit :
>> +AVRational   av_buffersink_get_frame_rate  (const 
>> AVFilterContext *ctx);
>> +int  av_buffersink_get_w   (const 
>> AVFilterContext *ctx);
>> +int  av_buffersink_get_h   (const 
>> AVFilterContext *ctx);
>> +AVRational   av_buffersink_get_sample_aspect_ratio (const 
>> AVFilterContext *ctx);
> 
> So, I ask this of everybody who care: what API do you prefer?
> 
> This one, i.e.:
> 
> encoder->width   = av_buffersink_get_w(sink);
> encoder->height  = av_buffersink_get_h(sink);
> encoder->sample_aspect_ratio = 
> av_buffersink_get_sample_aspect_ratio(sink);
> 
> Or one with a single access to all the properties:
> 
> const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink);
> encoder->width   = fmt->w;
> encoder->height  = fmt->h;
> encoder->sample_aspect_ratio = fmt->sample_aspect_ratio;
> 
> Regards,

I very much prefer the latter. Only one symbol, a (hopefully) easily
extensible struct if needed, etc.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[wm4]

On Fri, 23 Dec 2016 15:49:16 +0100
Nicolas George  wrote:

> Le primidi 1er nivôse, an CCXXV, wm4 a écrit :
> 
> [about windows COM system]
> 
> > To make it short, everything in COM consists of structs with function
> > pointers. Structs are never extended, if you need new function
> > pointers, you just add new structs, which you can dynamically query
> > from the old ones. This gives you 100% ABI downwards and upwards
> > compatibility. You also don't have to worry about linking to functions
> > not present in older Windows or whatever versions, because structs with
> > function pointers are a compile-time thing. You will merely get a
> > runtime failure when trying to query support for an unsupported struct.
> > (I tried to avoid COM terms, but COM calls these structs "interfaces".)
> > 
> > It's a pretty fascinating contrast to the fuckhackery we do with
> > extending structs while trying to keep ABI compatibility, version
> > and configure checks in API user code when trying to stay compatible
> > with multiple libavcodec versions, etc.  
> 
> It is probably a large part of the reason windows is slow on tomorrow's
> computers while Linux and BSD are fast on yesterday's.

No, it's not.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> also AVCodecParameters would be an option to use as a struct if a
> struct is used

Unfortunately not, since it lacks at least time_base and frame_rate. We
could add them, but think it would be a bad idea. It also has a lot of
other fields that are of no use for this case, that would need
documentation.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Michael Niedermayer a écrit :
> From these 2 the first

Ok, that makes one.

>but i think the user app needs more access
> to be able to implement filters and this could make either API
> obsoleete

User app implementing filters is not for today nor tomorrow. I think it
is better to start thinking on how to cross that bridge when we start
seeing it on the horizon.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 04:59:45PM +0100, Michael Niedermayer wrote:
> On Fri, Dec 23, 2016 at 03:31:45PM +0100, Nicolas George wrote:
> > L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit :
> > > +AVRational   av_buffersink_get_frame_rate  (const 
> > > AVFilterContext *ctx);
> > > +int  av_buffersink_get_w   (const 
> > > AVFilterContext *ctx);
> > > +int  av_buffersink_get_h   (const 
> > > AVFilterContext *ctx);
> > > +AVRational   av_buffersink_get_sample_aspect_ratio (const 
> > > AVFilterContext *ctx);
> > 
> > So, I ask this of everybody who care: what API do you prefer?
> > 
> > This one, i.e.:
> > 
> > encoder->width   = av_buffersink_get_w(sink);
> > encoder->height  = av_buffersink_get_h(sink);
> > encoder->sample_aspect_ratio = 
> > av_buffersink_get_sample_aspect_ratio(sink);
> > 
> > Or one with a single access to all the properties:
> > 
> > const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink);
> > encoder->width   = fmt->w;
> > encoder->height  = fmt->h;
> > encoder->sample_aspect_ratio = fmt->sample_aspect_ratio;
> 
> From these 2 the first but i think the user app needs more access
> to be able to implement filters and this could make either API
> obsoleete

also AVCodecParameters would be an option to use as a struct if a
struct is used, the lack of AVClass/AVOption in it may cause problems
though when lib versions differ and field have been added between

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 03:31:45PM +0100, Nicolas George wrote:
> L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit :
> > +AVRational   av_buffersink_get_frame_rate  (const 
> > AVFilterContext *ctx);
> > +int  av_buffersink_get_w   (const 
> > AVFilterContext *ctx);
> > +int  av_buffersink_get_h   (const 
> > AVFilterContext *ctx);
> > +AVRational   av_buffersink_get_sample_aspect_ratio (const 
> > AVFilterContext *ctx);
> 
> So, I ask this of everybody who care: what API do you prefer?
> 
> This one, i.e.:
> 
> encoder->width   = av_buffersink_get_w(sink);
> encoder->height  = av_buffersink_get_h(sink);
> encoder->sample_aspect_ratio = 
> av_buffersink_get_sample_aspect_ratio(sink);
> 
> Or one with a single access to all the properties:
> 
> const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink);
> encoder->width   = fmt->w;
> encoder->height  = fmt->h;
> encoder->sample_aspect_ratio = fmt->sample_aspect_ratio;

From these 2 the first but i think the user app needs more access
to be able to implement filters and this could make either API
obsoleete

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/af_amerge: do not guess an output channel layout

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 03:12:26PM +0100, Nicolas George wrote:
> Le quintidi 25 frimaire, an CCXXV, Marton Balint a écrit :
[...]
> > --- a/tests/ref/fate/filter-amerge
> > +++ b/tests/ref/fate/filter-amerge
> > @@ -2,8 +2,8 @@
> >  #media_type 0: audio
> >  #codec_id 0: pcm_s16le
> >  #sample_rate 0: 44100
> > -#channel_layout 0: 3
> > -#channel_layout_name 0: stereo
> > +#channel_layout 0: 0
> > +#channel_layout_name 0: 2 channels
> 
> Michael: this is an example of filter graph that breaks: the output was
> "stereo", it becomes "2 channels". Since there are
> av_get_default_channel_layout() all over the place anyway, I think
> something else would have caught it and hidden the difference for all
> practical purposes.

I thought so too

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

Le primidi 1er nivôse, an CCXXV, wm4 a écrit :

[about windows COM system]

> To make it short, everything in COM consists of structs with function
> pointers. Structs are never extended, if you need new function
> pointers, you just add new structs, which you can dynamically query
> from the old ones. This gives you 100% ABI downwards and upwards
> compatibility. You also don't have to worry about linking to functions
> not present in older Windows or whatever versions, because structs with
> function pointers are a compile-time thing. You will merely get a
> runtime failure when trying to query support for an unsupported struct.
> (I tried to avoid COM terms, but COM calls these structs "interfaces".)
> 
> It's a pretty fascinating contrast to the fuckhackery we do with
> extending structs while trying to keep ABI compatibility, version
> and configure checks in API user code when trying to stay compatible
> with multiple libavcodec versions, etc.

It is probably a large part of the reason windows is slow on tomorrow's
computers while Linux and BSD are fast on yesterday's.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/3] Revert "avcodec: Add max_pixels options"

[Nicolas George]

Le duodi 22 frimaire, an CCXXV, Michael Niedermayer a écrit :
> OOM is a security issue under some(1) circumstances, one can hit OOM due
> to too many pixels (or streams), this specific issue is fixed by the
> options

> (1) For example a server process that dies due to it. Even if restarted
> this can put load on the machine to be a effective was to DOS it

If memory is limited by explicit limits at the OS level, then huge
memory consumption in FFmpeg would not result in bursts of the
OOM-killer but just return ENOMEM like a normal error.

> completely independant of this, the option is needed to fuzz FFmpeg
> efficiently as andreas explained

> and also completely independant of this, the option is needed to
> allow finding some fixable OOM bugs that i would like to fix

For that case, I think that the idea of an independent branch is
interesting. Or possibly: the option is implemented in a separate
branch, and whenever it is needed, the commit is cherry-picked on top of
head.

But more importantly: since this is for convenience rather than
security, it does not need to be backported to older releases, and
therefore the compatibility issue is moot.

The fixes for the bugs found by fuzzing, of course, can be backported.

> Also its certainly true that this does not fix every OOM issue but
> no bugfix that fixes a out of array read fixes every out of array read
> either

I would never oppose to a change on the grounds that it only does part
of a task impossible to do completely. I think I denounced that kind of
arguments recently about DCE :)

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] lavfi/buffersink: add accessors for the stream properties.

[Nicolas George]

L'octidi 28 frimaire, an CCXXV, Nicolas George a écrit :
> +AVRational   av_buffersink_get_frame_rate  (const 
> AVFilterContext *ctx);
> +int  av_buffersink_get_w   (const 
> AVFilterContext *ctx);
> +int  av_buffersink_get_h   (const 
> AVFilterContext *ctx);
> +AVRational   av_buffersink_get_sample_aspect_ratio (const 
> AVFilterContext *ctx);

So, I ask this of everybody who care: what API do you prefer?

This one, i.e.:

encoder->width   = av_buffersink_get_w(sink);
encoder->height  = av_buffersink_get_h(sink);
encoder->sample_aspect_ratio = av_buffersink_get_sample_aspect_ratio(sink);

Or one with a single access to all the properties:

const AVBufferSinkProperties *fmt = av_buffersink_get_properties(sink);
encoder->width   = fmt->w;
encoder->height  = fmt->h;
encoder->sample_aspect_ratio = fmt->sample_aspect_ratio;

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, James Almer a écrit :
> Yes, i can confirm this fixes it.

Thanks, pushed.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[James Almer]

On 12/23/2016 7:25 AM, Nicolas George wrote:
> Le tridi 3 nivôse, an CCXXV, Nicolas George a écrit :
>> Yes, thanks. This is the glitch I am currently working on, and I think
>> it is almost fixed cleanly. Note that the actual time is still very
>> small, it is only ffmpeg keeps thinking the filter is not ready and
>> sleeping.
> Actually, it was not exactly the same. Please see the attached patch.
> 
> Regards,
> 
> -- Nicolas George
> 
> 
> 0001-lavfi-avfilter_graph_request_oldest-request-a-frame-.patch
> 
> 
> From c7f2f6d8e2ea776dc4c097685dc0e8255051d1ee Mon Sep 17 00:00:00 2001
> From: Nicolas George 
> Date: Fri, 23 Dec 2016 11:18:09 +0100
> Subject: [PATCH] lavfi: avfilter_graph_request_oldest: request a frame again
>  before returning.
> 
> With min_samples, if a frame arrives but is too small, it clears
> frame_wanted_out. In most cases, the destination filter would be
> activated again later because of frame_wanted_out on its own
> outputs, but not sinks.
> avfilter_graph_request_oldest() is doing the work of the sink
> itself, and is therefore allowed to use frame_blocked_in.
> 
> Signed-off-by: Nicolas George 
> ---
>  libavfilter/avfiltergraph.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 6b5a6f34ec..cb5d618ab1 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -1407,7 +1407,11 @@ int avfilter_graph_request_oldest(AVFilterGraph *graph)
>  frame_count = oldest->frame_count_out;
>  while (frame_count == oldest->frame_count_out) {
>  r = ff_filter_graph_run_once(graph);
> -if (r < 0)
> +if (r == AVERROR(EAGAIN) &&
> +!oldest->frame_wanted_out && !oldest->frame_blocked_in &&
> +!oldest->status_in)
> +ff_request_frame(oldest);
> +else if (r < 0)
>  return r;
>  }
>  return 0;
> -- 2.11.0

Yes, i can confirm this fixes it.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/af_amerge: do not guess an output channel layout

[Nicolas George]

Le quintidi 25 frimaire, an CCXXV, Marton Balint a écrit :
> This is the right thing to do, but I am afraid this will break too many
> existing filter chains. How can we implement this properly? Ideas/options:
> 
> - change it, break it, users will fix it
> - add a guess_output_layout option which will be true for now, false after a
>   major bump, and mention this incompatible change in the next release
> - add amerge2 filter, deprecate amerge filter

I do not like the option or the amerge2 solution, because that leaves
traces we can not get rid of.

My gut feeling wants me to say: there already was a warning, people who
ignore warnings deserve what they get. If you want to be extra careful
and are not in a hurry, you could reword the warning to notify that it
will change soon, wait some time (next release) and do the change.

> 
> Signed-off-by: Marton Balint 
> ---
>  libavfilter/af_amerge.c  | 4 +---
>  tests/ref/fate/filter-amerge | 4 ++--
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/libavfilter/af_amerge.c b/libavfilter/af_amerge.c
> index 3bc7d89..43a2d95 100644
> --- a/libavfilter/af_amerge.c
> +++ b/libavfilter/af_amerge.c
> @@ -114,9 +114,7 @@ static int query_formats(AVFilterContext *ctx)

> "output layout will be determined by the number of distinct 
> input channels\n");

I think the warning needs to be reworded anyway.

>  for (i = 0; i < nb_ch; i++)
>  s->route[i] = i;
> -outlayout = av_get_default_channel_layout(nb_ch);
> -if (!outlayout && nb_ch)
> -outlayout = 0xULL >> (64 - nb_ch);
> +outlayout = FF_COUNT2LAYOUT(nb_ch);
>  } else {
>  int *route[SWR_CH_MAX];
>  int c, out_ch_number = 0;
> diff --git a/tests/ref/fate/filter-amerge b/tests/ref/fate/filter-amerge
> index b3e5eb5..8118b4e 100644

> --- a/tests/ref/fate/filter-amerge
> +++ b/tests/ref/fate/filter-amerge
> @@ -2,8 +2,8 @@
>  #media_type 0: audio
>  #codec_id 0: pcm_s16le
>  #sample_rate 0: 44100
> -#channel_layout 0: 3
> -#channel_layout_name 0: stereo
> +#channel_layout 0: 0
> +#channel_layout_name 0: 2 channels

Michael: this is an example of filter graph that breaks: the output was
"stereo", it becomes "2 channels". Since there are
av_get_default_channel_layout() all over the place anyway, I think
something else would have caught it and hidden the difference for all
practical purposes.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Muhammad Faiz]

On 12/23/16, Soft Works  wrote:
>
>
> 
> From: ffmpeg-devel  on behalf of Paul B
> Mahol 
> Sent: Friday, December 23, 2016 12:21 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] Serious Regression in matroskaenc:
>
> On 12/23/16, Soft Works  wrote:
>> Hello everybody,
>>
>> it's been a while since my contribution to matroskaenc in July. In case
>> you
>> don't remember my scenario - it's about realtime transcoding and and
>> streaming of mkv files.
>> That means that the header is sent to the client before the ffmpeg process
>> is completed.
>>
>> In this context we discovered that the preliminary mkv header that is
>> written initially is corrupted in some way!
>>
>> When transcoding is aborted or completed, the header is written a second
>> time, overwriting a few things. This final header write operation always
>> fixes the problem and leaves the file with a correct header.
>>
>> I haven't investigated yet, what's actually being written wrong. I used
>> different parsing implementations (mediainfo, ffprobe, mkvvalidator,
>> mkvtoolnix) and all are producing different results. Some report CRC
>> error,
>> others report unknown or duplicate header elements. Some are able to
>> detect
>> the stream information others are not.
>>
>> Now, I'm wondering if one of you guys could take care of this? Or do I
>> have
>> to find the bug on my own?
>
>>> Do you have steps to reproduce this?
>
> To reproduce, transcode an mkv file, even if all streams are simply copied.
>
> While the transcoding is in progress, I copied the output file to another
> file
> (works on Windows at least), but probably you can simply kill the ffmpeg
> process.
>
> Only thing that you shouldn't do is breaking the ffmpeg process via key
> commands
> because in this case the header would still be finalized.
>
> On the result file, you can run various diagnostics then. I tried mediainfo,
> ffprobe, mkvvalidator, mkvtoolnix. All showed different results but
> always with errors.
>
> This doesn't happen with matroskaenc.c from July 17, 2016 and other versions
> before that date.

See https://lists.ffmpeg.org/pipermail/ffmpeg-user/2016-December/034770.html
This is ticket #5977

Thx
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/af_amerge: properly handle unknown input layouts

[Nicolas George]

Le quintidi 25 frimaire, an CCXXV, Marton Balint a écrit :
> Signed-off-by: Marton Balint 
> ---
>  libavfilter/af_amerge.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)

LGTM (I trust you tested it), sorry for the delay.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avformat/avidec: skip odml master index chunks in avi_sync

[Tobias Rapp]

Fixes pts gaps when reading AVI files > 256GiB generated by FFmpeg.

Signed-off-by: Tobias Rapp 
---
 libavformat/avidec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index d465965..4a054ee 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -1211,7 +1211,8 @@ start_sync:
 if ((d[0] == 'i' && d[1] == 'x' && n < s->nb_streams) ||
 // parse JUNK
 (d[0] == 'J' && d[1] == 'U' && d[2] == 'N' && d[3] == 'K') ||
-(d[0] == 'i' && d[1] == 'd' && d[2] == 'x' && d[3] == '1')) {
+(d[0] == 'i' && d[1] == 'd' && d[2] == 'x' && d[3] == '1') ||
+(d[0] == 'i' && d[1] == 'n' && d[2] == 'd' && d[3] == 'x')) {
 avio_skip(pb, size);
 goto start_sync;
 }
-- 
1.9.1


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avfilter/af_amerge: do not guess an output channel layout

[Michael Niedermayer]

On Thu, Dec 15, 2016 at 04:39:19AM +0100, Marton Balint wrote:
> This is the right thing to do, but I am afraid this will break too many
> existing filter chains. How can we implement this properly? Ideas/options:

do you have an example of a filter chain it would break ?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

>> To matroskaenc were only a few.

I'm counting 33 commits since my change from 17 July 2016

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Paul B Mahol]

On 12/23/16, Soft Works  wrote:
>
> On Fri, Dec 23, 2016 at 11:19:06AM +, Soft Works wrote:
>> Hello everybody,
>>
>> it's been a while since my contribution to matroskaenc in July. In case
>> you don't remember my scenario - it's about realtime transcoding and and
>> streaming of mkv files.
>> That means that the header is sent to the client before the ffmpeg process
>> is completed.
>
>>> can you identify (with git bisect) which change made the difference ?
>
> Unfortunately not yet; there were a lot of changes during the last few
> months...

To matroskaenc were only a few.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

On Fri, Dec 23, 2016 at 11:19:06AM +, Soft Works wrote:
> Hello everybody,
>
> it's been a while since my contribution to matroskaenc in July. In case you 
> don't remember my scenario - it's about realtime transcoding and and 
> streaming of mkv files.
> That means that the header is sent to the client before the ffmpeg process is 
> completed.

>> can you identify (with git bisect) which change made the difference ?

Unfortunately not yet; there were a lot of changes during the last few months...

softworkz

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

Nicolas, I'm afraid but I got the feeling that you haven't really understood 
what this is about.

In July I already submitted a patch for matroscaenc.c in order to write 
duration metadata to the 
mkv header early. The purpose was to have a duration value in the mkv header 
even in cases
where the mkv file is streamed to the client before ffmpeg as even finished 
it's process.

This patch has been accepted and merged.

This time I'm reporting that the preliminary header written to mkv files is 
corrupted since recent 
versions. This hadn't been the case for ffmpeg versions during the last 3 
years. A clear regression,
I don't think that there is anything to discuss!

So, a precise analysis is what I already have, and all I'm asking is if 
somebody wants to take care 
of it (like someone that has done those recent changes) or if I should work on 
a fix on my own.

I think this is a legitimate question for the developer list and not a user 
question.

Regards, 

softworkz


From: ffmpeg-devel  on behalf of Nicolas 
George 
Sent: Friday, December 23, 2016 12:46 PM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] Serious Regression in matroskaenc:

Le tridi 3 nivôse, an CCXXV, Soft Works a écrit :
> Are you making some fun out of me..?

No, sorry if it seemed that way.

> What I laid out was a method to reproduce as requested.

And the clear answer is: it is not a bug. You were using ffmpeg in an
unsupported way.

Note: for bugs without patch (or at least precise analysis) and usage
questions, see ffmpeg-u...@ffmpeg.org

Regards,

--
  Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 11:19:06AM +, Soft Works wrote:
> Hello everybody,
> 
> it's been a while since my contribution to matroskaenc in July. In case you 
> don't remember my scenario - it's about realtime transcoding and and 
> streaming of mkv files.
> That means that the header is sent to the client before the ffmpeg process is 
> completed.

can you identify (with git bisect) which change made the difference ?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf: identify MP1 and MP2 as distinct containers from MP3

[Michael Niedermayer]

On Fri, Dec 23, 2016 at 12:01:08AM -0600, Rodger Combs wrote:
> This allows us to report the correct codec ID here
> ---
>  libavformat/allformats.c |  3 ++-
>  libavformat/mp3dec.c | 66 
> +++-
>  libavformat/rawenc.c | 13 ++
>  libavformat/utils.c  |  4 +--
>  libavformat/version.h|  2 +-
>  5 files changed, 61 insertions(+), 27 deletions(-)

seems to break detecting 2 streams in
http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4221/teletext-streams-misrecognized.ts

./ffmpeg -i teletext-streams-misrecognized.ts -vn -map 0:4 test.wav


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Soft Works a écrit :
> Are you making some fun out of me..?

No, sorry if it seemed that way.

> What I laid out was a method to reproduce as requested.

And the clear answer is: it is not a bug. You were using ffmpeg in an
unsupported way.

Note: for bugs without patch (or at least precise analysis) and usage
questions, see ffmpeg-u...@ffmpeg.org

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

>Le tridi 3 nivôse, an CCXXV, Soft Works a écrit :
>> Only thing that you shouldn't do is breaking the ffmpeg process via key 
>> commands
>> because in this case the header would still be finalized.
>
>Then do that!
>
>Or possibly use the "live" option.
>
>Regards,
>
>--
>  Nicolas George

Are you making some fun out of me..?

What I laid out was a method to reproduce as requested.

I had described the actual scenario in my previous message.

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Soft Works a écrit :
> Only thing that you shouldn't do is breaking the ffmpeg process via key 
> commands
> because in this case the header would still be finalized.

Then do that!

Or possibly use the "live" option.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

From: ffmpeg-devel  on behalf of Paul B Mahol 

Sent: Friday, December 23, 2016 12:21 PM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] Serious Regression in matroskaenc:

On 12/23/16, Soft Works  wrote:
> Hello everybody,
>
> it's been a while since my contribution to matroskaenc in July. In case you
> don't remember my scenario - it's about realtime transcoding and and
> streaming of mkv files.
> That means that the header is sent to the client before the ffmpeg process
> is completed.
>
> In this context we discovered that the preliminary mkv header that is
> written initially is corrupted in some way!
>
> When transcoding is aborted or completed, the header is written a second
> time, overwriting a few things. This final header write operation always
> fixes the problem and leaves the file with a correct header.
>
> I haven't investigated yet, what's actually being written wrong. I used
> different parsing implementations (mediainfo, ffprobe, mkvvalidator,
> mkvtoolnix) and all are producing different results. Some report CRC error,
> others report unknown or duplicate header elements. Some are able to detect
> the stream information others are not.
>
> Now, I'm wondering if one of you guys could take care of this? Or do I have
> to find the bug on my own?

>> Do you have steps to reproduce this?

To reproduce, transcode an mkv file, even if all streams are simply copied.

While the transcoding is in progress, I copied the output file to another file 
(works on Windows at least), but probably you can simply kill the ffmpeg 
process.

Only thing that you shouldn't do is breaking the ffmpeg process via key commands
because in this case the header would still be finalized.

On the result file, you can run various diagnostics then. I tried mediainfo, 
ffprobe, mkvvalidator, mkvtoolnix. All showed different results but
always with errors.

This doesn't happen with matroskaenc.c from July 17, 2016 and other versions 
before that date.

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Serious Regression in matroskaenc:

[Paul B Mahol]

On 12/23/16, Soft Works  wrote:
> Hello everybody,
>
> it's been a while since my contribution to matroskaenc in July. In case you
> don't remember my scenario - it's about realtime transcoding and and
> streaming of mkv files.
> That means that the header is sent to the client before the ffmpeg process
> is completed.
>
> In this context we discovered that the preliminary mkv header that is
> written initially is corrupted in some way!
>
> When transcoding is aborted or completed, the header is written a second
> time, overwriting a few things. This final header write operation always
> fixes the problem and leaves the file with a correct header.
>
> I haven't investigated yet, what's actually being written wrong. I used
> different parsing implementations (mediainfo, ffprobe, mkvvalidator,
> mkvtoolnix) and all are producing different results. Some report CRC error,
> others report unknown or duplicate header elements. Some are able to detect
> the stream information others are not.
>
> Now, I'm wondering if one of you guys could take care of this? Or do I have
> to find the bug on my own?

Do you have steps to reproduce this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] Serious Regression in matroskaenc:

[Soft Works]

Hello everybody,

it's been a while since my contribution to matroskaenc in July. In case you 
don't remember my scenario - it's about realtime transcoding and and streaming 
of mkv files.
That means that the header is sent to the client before the ffmpeg process is 
completed.

In this context we discovered that the preliminary mkv header that is written 
initially is corrupted in some way!

When transcoding is aborted or completed, the header is written a second time, 
overwriting a few things. This final header write operation always fixes the 
problem and leaves the file with a correct header.

I haven't investigated yet, what's actually being written wrong. I used 
different parsing implementations (mediainfo, ffprobe, mkvvalidator, 
mkvtoolnix) and all are producing different results. Some report CRC error, 
others report unknown or duplicate header elements. Some are able to detect the 
stream information others are not. 

Now, I'm wondering if one of you guys could take care of this? Or do I have to 
find the bug on my own?


Best regards,

softworkz
 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Nicolas George a écrit :
> Yes, thanks. This is the glitch I am currently working on, and I think
> it is almost fixed cleanly. Note that the actual time is still very
> small, it is only ffmpeg keeps thinking the filter is not ready and
> sleeping.

Actually, it was not exactly the same. Please see the attached patch.

Regards,

-- 
  Nicolas George
From c7f2f6d8e2ea776dc4c097685dc0e8255051d1ee Mon Sep 17 00:00:00 2001
From: Nicolas George 
Date: Fri, 23 Dec 2016 11:18:09 +0100
Subject: [PATCH] lavfi: avfilter_graph_request_oldest: request a frame again
 before returning.

With min_samples, if a frame arrives but is too small, it clears
frame_wanted_out. In most cases, the destination filter would be
activated again later because of frame_wanted_out on its own
outputs, but not sinks.
avfilter_graph_request_oldest() is doing the work of the sink
itself, and is therefore allowed to use frame_blocked_in.

Signed-off-by: Nicolas George 
---
 libavfilter/avfiltergraph.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index 6b5a6f34ec..cb5d618ab1 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -1407,7 +1407,11 @@ int avfilter_graph_request_oldest(AVFilterGraph *graph)
 frame_count = oldest->frame_count_out;
 while (frame_count == oldest->frame_count_out) {
 r = ff_filter_graph_run_once(graph);
-if (r < 0)
+if (r == AVERROR(EAGAIN) &&
+!oldest->frame_wanted_out && !oldest->frame_blocked_in &&
+!oldest->status_in)
+ff_request_frame(oldest);
+else if (r < 0)
 return r;
 }
 return 0;
-- 
2.11.0



signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavfi: make filter_frame non-recursive.

[Nicolas George]

Le duodi 2 nivôse, an CCXXV, James Almer a écrit :
> This patch made audio encoding extremely slow.
> 
> time ./ffmpeg -i INPUT.wav -c:a tta -f null -
> 
> Before
> size=N/A time=00:00:22.98 bitrate=N/A speed= 373x
> video:0kB audio:1272kB subtitle:0kB other streams:0kB global headers:0kB 
> muxing overhead: unknown
> real0m0.089s
> user0m0.000s
> sys 0m0.000s
> 
> After
> size=N/A time=00:00:22.98 bitrate=N/A speed=2.29x
> video:0kB audio:1272kB subtitle:0kB other streams:0kB global headers:0kB 
> muxing overhead: unknown
> real0m10.087s
> user0m0.015s
> sys 0m0.000s
> 
> CPU usage for the latter is almost zero during the entire 10 seconds. And it 
> can be reproduced
> with pretty much any encoder.

Yes, thanks. This is the glitch I am currently working on, and I think
it is almost fixed cleanly. Note that the actual time is still very
small, it is only ffmpeg keeps thinking the filter is not ready and
sleeping.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/pixlet: remove unnecessary double to float conversion

[Paul B Mahol]

On 12/23/16, James Almer  wrote:
> Signed-off-by: James Almer 
> ---
>  libavcodec/pixlet.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

applied
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/pixlet: remove unnecessary double to float conversion

[Nicolas George]

Le tridi 3 nivôse, an CCXXV, Paul B Mahol a écrit :
> I found no difference, but using float could help more one day when
> someone write SIMD code.

Ok, it was a reason to use floats that I did not know. Thanks.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Removing DCE

[Nicolas George]

Le primidi 1er nivôse, an CCXXV, Michael Niedermayer a écrit :
> how hard would it be to write a preprocessor like tool to convert
> all if (ARCH/HAVE/CONFIG_SYMBOL ...)
> to
> #if
> ?

For a very general case, quite hard, but we do not need it.

If we stick to a few reasonable cosmetic conventions on the affected
blocks (at a guess: correct indentation of the braces, balanced
parentheses in the condition; we want to do that anyway), then it should
be quite easy to write a preprocessor like that.

I do not know how to integrate that in the build system, though. That is
the annoying part IMHO.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/pixlet: remove unnecessary double to float conversion

[Paul B Mahol]

On 12/23/16, Nicolas George  wrote:
> Le duodi 2 nivôse, an CCXXV, James Almer a écrit :
>> Signed-off-by: James Almer 
>> ---
>>  libavcodec/pixlet.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/pixlet.c b/libavcodec/pixlet.c
>> index c5d37bb..c1bd321 100644
>> --- a/libavcodec/pixlet.c
>> +++ b/libavcodec/pixlet.c
>> @@ -457,7 +457,7 @@ static void postprocess_luma(AVFrame *frame, int w,
>> int h, int depth)
>>  uint16_t *dsty = (uint16_t *)frame->data[0];
>>  int16_t *srcy  = (int16_t *)frame->data[0];
>>  ptrdiff_t stridey = frame->linesize[0] / 2;
>> -const float factor = 1. / ((1 << depth) - 1);
>> +const float factor = 1.0f / ((1 << depth) - 1);
>
> Any reason, here and everywhere, to do it that way and not the other way
> around, i.e. change all floats to doubles? There are only a 32-cells
> array and a few local variables, so memory is not a concern.

I found no difference, but using float could help more one day when
someone write SIMD code.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/pixlet: remove unnecessary double to float conversion

[Nicolas George]

Le duodi 2 nivôse, an CCXXV, James Almer a écrit :
> Signed-off-by: James Almer 
> ---
>  libavcodec/pixlet.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/pixlet.c b/libavcodec/pixlet.c
> index c5d37bb..c1bd321 100644
> --- a/libavcodec/pixlet.c
> +++ b/libavcodec/pixlet.c
> @@ -457,7 +457,7 @@ static void postprocess_luma(AVFrame *frame, int w, int 
> h, int depth)
>  uint16_t *dsty = (uint16_t *)frame->data[0];
>  int16_t *srcy  = (int16_t *)frame->data[0];
>  ptrdiff_t stridey = frame->linesize[0] / 2;
> -const float factor = 1. / ((1 << depth) - 1);
> +const float factor = 1.0f / ((1 << depth) - 1);

Any reason, here and everywhere, to do it that way and not the other way
around, i.e. change all floats to doubles? There are only a 32-cells
array and a few local variables, so memory is not a concern.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec: add Apple Pixlet decoder

[Paul B Mahol]

On 12/23/16, Andreas Cadhalpun  wrote:
> On 22.12.2016 22:52, Paul B Mahol wrote:
>> ffmpeg | branch: master | Paul B Mahol  | Fri Dec  2
>> 20:30:50 2016 +0100| [73651090ca1183f37753ee30a7e206ca4fb9f4f0] |
>> committer: Paul B Mahol
>>
>> avcodec: add Apple Pixlet decoder
>>
>> Signed-off-by: Paul B Mahol 
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=73651090ca1183f37753ee30a7e206ca4fb9f4f0
>> ---
>>
>>  Changelog   |   1 +
>>  doc/general.texi|   1 +
>>  libavcodec/Makefile |   1 +
>>  libavcodec/allcodecs.c  |   1 +
>>  libavcodec/avcodec.h|   1 +
>>  libavcodec/codec_desc.c |   7 +
>>  libavcodec/pixlet.c | 672
>> 
>>  libavcodec/version.h|   2 +-
>>  libavformat/isom.c  |   2 +
>>  9 files changed, 687 insertions(+), 1 deletion(-)
> [...]
>> +static int read_high_coeffs(AVCodecContext *avctx, uint8_t *src, int16_t
>> *dst, int size,
>> +int c, int a, int d,
>> +int width, ptrdiff_t stride)
>> +{
>> +PixletContext *ctx = avctx->priv_data;
>> +GetBitContext *b = >gbit;
>> +unsigned cnt1, shbits, rlen, nbits, length, i = 0, j = 0, k;
>> +int ret, escape, pfx, value, yflag, xflag, flag = 0;
>> +int64_t state = 3, tmp;
>> +
>> +if ((ret = init_get_bits8(b, src,
>> bytestream2_get_bytes_left(>gb))) < 0)
>> +  return ret;
>> +
>> +if ((a >= 0) + (a ^ (a >> 31)) - (a >> 31) != 1) {
>> +nbits = 33 - ff_clz((a >= 0) + (a ^ (a >> 31)) - (a >> 31) - 1);
>> +if (nbits > 16)
>> +return AVERROR_INVALIDDATA;
>> +} else {
>> +nbits = 1;
>> +}
>> +
>> +length = 25 - nbits;
>> +
>> +while (i < size) {
>> +if (state >> 8 != -3) {
>> +value = ff_clz((state >> 8) + 3) ^ 0x1F;
>> +} else {
>> +value = -1;
>> +}
>> +
>> +cnt1 = get_unary(b, 0, length);
>> +
>> +if (cnt1 >= length) {
>> +cnt1 = get_bits(b, nbits);
>> +} else {
>> +pfx = 14 + uint64_t)(value - 14)) >> 32) & (value - 14));
>> +cnt1 *= (1 << pfx) - 1;
>> +shbits = show_bits(b, pfx);
>> +if (shbits <= 1) {
>> +skip_bits(b, pfx - 1);
>> +} else {
>> +skip_bits(b, pfx);
>> +cnt1 += shbits - 1;
>> +}
>> +}
>> +
>> +xflag = flag + cnt1;
>> +yflag = xflag;
>> +
>> +if (flag + cnt1 == 0) {
>> +value = 0;
>> +} else {
>> +xflag &= 1u;
>> +tmp = c * ((yflag + 1) >> 1) + (c >> 1);
>
> This can overflow.

And?

>
>> +value = xflag + (tmp ^ -xflag);
>> +}
>> +
>> +i++;
>> +dst[j++] = value;
>> +if (j == width) {
>> +j = 0;
>> +dst += stride;
>> +}
>> +state += d * yflag - (d * state >> 8);
>
> This can overflow, too.

And?

>
>> +
>> +flag = 0;
>> +
>> +if (state * 4 > 0xFF || i >= size)
>> +continue;
>> +
>> +pfx = ((state + 8) >> 5) + (state ? ff_clz(state): 32) - 24;
>> +escape = av_mod_uintp2(16383, pfx);
>
> Here pfx can be negative, but av_mod_uintp2 takes an unsigned argument, so
> it's
> interpreted as very large exponent, causing undefined shifts.

I think I will make it always >= 0

>
>> +cnt1 = get_unary(b, 0, 8);
>> +if (cnt1 < 8) {
>> +value = show_bits(b, pfx);
>
> And a negative pfx triggers an assertion in show_bits.
>
> [...]
>> +static void postprocess_chroma(AVFrame *frame, int w, int h, int depth)
>> +{
>> +uint16_t *dstu = (uint16_t *)frame->data[1];
>> +uint16_t *dstv = (uint16_t *)frame->data[2];
>> +int16_t *srcu  = (int16_t *)frame->data[1];
>> +int16_t *srcv  = (int16_t *)frame->data[2];
>> +ptrdiff_t strideu = frame->linesize[1] / 2;
>> +ptrdiff_t stridev = frame->linesize[2] / 2;
>> +const int add = 1 << (depth - 1);
>> +const int shift = 16 - depth;
>> +int i, j;
>> +
>> +for (j = 0; j < h; j++) {
>> +for (i = 0; i < w; i++) {
>> +dstu[i] = (add + srcu[i]) << shift;
>> +dstv[i] = (add + srcv[i]) << shift;
>
> These result in undefined shifts, since the value to be shifted can be
> negative.

OK.

>
>> +}
>> +dstu += strideu;
>> +dstv += stridev;
>> +srcu += strideu;
>> +srcv += stridev;
>> +}
>> +}
>> +
>> +static int decode_plane(AVCodecContext *avctx, int plane, AVPacket
>> *avpkt, AVFrame *frame)
>> +{
>> +PixletContext *ctx = avctx->priv_data;
>> +ptrdiff_t stride = frame->linesize[plane] / 2;
>> +unsigned shift = plane > 0;
>> +int16_t *dst;
>> +int i, ret;
>> +
>> +for (i = ctx->levels - 1; i >= 0; i--) {
>> +ctx->scaling[plane][H][i] = 100. /
>>