Re: [FFmpeg-devel] Removing DCE

[Carl Eugen Hoyos]

2016-12-16 3:48 GMT+01:00 Matt Oliver :

> For DCE:
> 1) Ends up with a horrible mess of ifdefs.

This argument alone has often been sufficient to refuse patches here.

> 6) Issues with lto with Clang and Gold.
> 7) Other unspecified issues with debug builds

Could you elaborate on these two?

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

Re: [FFmpeg-devel] [PATCH 1/3] lavf: identify MP2 as a distinct container from MP3

[Carl Eugen Hoyos]

2016-12-21 5:48 GMT+01:00 Rodger Combs :
> This allows us to report the correct codec ID here

Just curious: What does this fix?

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

Re: [FFmpeg-devel] Removing DCE

[Matt Oliver]

On 21 December 2016 at 19:26, Carl Eugen Hoyos  wrote:

> 2016-12-16 3:48 GMT+01:00 Matt Oliver :
>
> > For DCE:
> > 1) Ends up with a horrible mess of ifdefs.
>
> This argument alone has often been sufficient to refuse patches here.
>

Hence the initial email as I think that this is a highly subjective point
(see #9), and point #8 was taken from one of wm4s responses (not to put him
on the spot or anything). So there are devs on either side of this. So the
point was to decide whether peoples subjective dislike of ifdefs is enough
to justify breaking compatibility in certain circumstances.

> 6) Issues with lto with Clang and Gold.
> > 7) Other unspecified issues with debug builds
>
> Could you elaborate on these two?
>

These were taken from previous email threads, so point 6) was from an email
by Derek Buitenhuis who basically stated exactly what was written. So for
more information youll have to ask him directly. I was just collating
points that have been previously made by others on the mailing list in
order to jump start a discussion.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 6/6] lavfi: make AVFilterLink opaque in two major bumps.

[Nicolas George]

Le decadi 30 frimaire, an CCXXV, Michael Niedermayer a écrit :
> it shouldnt really be complex, not in concept, maybe in (efficient)
> implementation

I think it is.

> For example as a concept one could imagine each filter runs as its
> own thread and waits on its inputs availability and output space.
> if it gets input and has output space its woken up and works and
> produces output and then wakes its surroundings up.
> no difference between a linear chain or a complex graph here

Sorry, but it does not work, at least not just like that:

If you make the pipes between filters asynchronous and unlimited, then
you could have movie from a fast codec flooding its output while overlay
is waiting on something slower on its other input. OOM.

If you make the pipes synchronous or limited, then you have problems
when there are cycles in the undirected graph, i.e. if there are
split+merge situations (a situation supported from the beginning and
present in many of the examples): merge starves on one input while split
is blocked trying to feed its other input. Deadlock.

Maybe it could be made to work with some kind of "elastic" pipes: the
fuller they are, the lower the priority. But my point is already proven:
it is complicated.

Note that what I ended up doing is based on exactly that. But it does
not only track where frames are possible but also where they are needed.

> iam not sure its usfull but
> Another view from a different direction would be to see the filter
> graph as a network of pipes with flowing water and interconnected
> pumps. This too is a simple concept and intuition would suggest that
> this would not easily end up with water being stuck nor too much
> accumulating. It does not 1:1 match real filters though which work
> on discrete frames, i wonder if this view is usefull
> It would allow awnsering global questions about a graph, like what
> inputs are useless when some output is welded shut or vice versa

Basically, you are suggesting to apply graph theory to the filter graph.
That would be very smart. Alas to do that, it is necessary to actually
know the graph. We do not: we do not know how inputs and outputs are
connected within filters. For example, the connection is very different
for overlay and concat, but from the outside they are indistinguishable.
And select is even worse, of course.

> i think the original lavfi design didnt really had any issue with graphs with
> multiple inputs or outputs. A user app could decide were to in and out
> put. but FFmpeg didnt support this at the time IIRC so the people working
> on the original lavfi had nothing to implement.
> the problems came when this support was added much later

Not only that: before I added it (i.e. not in the original design),
lavfi did not give the application enough information to decide what
input to feed. It is still missing in lithe fork's implementation.

> > - Add a callback AVFilter.activate() to replace filter_frame() on all
> >   inputs and request_frame() on all outputs. Most non-trivial filters
> >   are written that way in the first place.
> ok thats mostly cosmetic id say.

I think the difference between a mediocre API and a good one is partly
cosmetic.

But this change is not cosmetic at all. Right now, if you want to notify
a filter that EOF arrived on one input, you have to request a frame on
one of its output and hope that it will in turn cause a read on that
input. But you could end up pumping on another input instead.

> > - Change buffersink to implement that callback and peek directly in the
> >   FIFO.
> ok, "cosmetic"

Except for the EOF thingie, which is the biggest glitch at this time
AFAIK.

> > - Rewrite framesync (the utility system for filters with several video
> >   inputs that need synchroized frames) to implement activate and use the
> >   FIFO directly.
> cosmetic :)

Ditto.

> > - Allow to give buffersrc a timestamp on EOF, make sure the timestamp is
> >   forwarded by most filters and allow to retrieve it from buffersink.
> > 
> >   (If somebody has a suggestion of a good data structure for that...)

Actually, the question in parentheses was about the priority queue, they
got separated when I reorganized my paragraphs. Sorry.

> AVFrame.duration
> This possibly is a big subject for discussion on its own, but maybe
> not i dont know.

Indeed, there are pros and cons. I found that an actual timestamp was
slightly better, mainly because we often do not know the duration
immediately.

> > - Allow to set a callback on buffersinks and buffersrcs to be notified
> >   when a frame arrives or is needed. It is much more convenient than
> >   walking the buffers to check one by one.
> agree that walking is bad.
> cannot ATM argue on what is better as i dont feel that i have a clear
> enough view of this and the surroundings.

You could have said it: cosmetic :)

> > - Allow to merge several filter graphs into one. This may be useful when
> >   applications have several graphs to run simultaneously, so 

Re: [FFmpeg-devel] Removing DCE

[Nicolas George]

Le sextidi 26 frimaire, an CCXXV, Matt Oliver a écrit :
> For DCE:

I do not care much about DCE one way or the other, but I do care about
valid arguments.

> 1) Ends up with a horrible mess of ifdefs.
> 2) Disabled parts of code will not be checked for syntax.
> 
> Against DCE:

> 3) DCE is not actually specified in the C specification. So compilers are
> actually being 100% compliant by not supporting it and should not be
> expected to change just for ffmpegs use case.

That is true in principle, IF the project decides it wants to be
standard C. This is an option, but not the only one. For example, the
code would break if ints were 16 bits, thus we require POSIX, not only
C. It would also break if intXX_t were not available, thus we require
optional parts of the C standard.

Similarly, the project is perfectly allowed to require non-standard
extensions. "If your compiler does not do DCE, we do not support it,
sorry, do it yourself."

And if somebody argues about the "standard" / "non-standard" thingie, I
create the Cigaes Standardization Organization and publish the Cigae C
Standard, where DCE is specified.

> 4) Breaks debug and lto builds on msvc.
> 5) Breaks debug and lto builds on icl.
> 6) Issues with lto with Clang and Gold.
> 7) Other unspecified issues with debug builds
> 
> Rebuttals against above arguments:

> 8) There are already 3961 #ifs(not including header guards) in the code so
> there is already a lot of code that doesn't use DCE. (In response to #1 for
> DCE).

There are unavoidable cases, that does not make it ok for the avoidable
cases.

> 9) Avoiding #ifdefs is a personal opinion as to whether they are ugly or
> not (some prefer ifdefs as IDEs will correctly highlight disabled
> sections). Someones personal preference for what looks better should not be
> justification to break supported configurations/compilers. (In response to
> #1 for DCE).

Someone's personal preferences affect their willingness to work on the
project. Since the project is perpetually short on manpower, this is a
very serious issue.

> 10) There is --enable-random which is supposed to be used to find
> configurations that don't compile. (in response to #2 for DCE).

--enale-random is useful for checking problems caused by optional
features that are usually disabled: A depends on B but it is not stated
in configure, thus breaking if B is disabled but A enabled.

The benefits of DCE are in the other directions: testing (partially, see
below) features that are usually disabled. For example, I develop on
Linux, the windows and mac features are always disabled, but I still
have a safety net preventing me from the easiest breakages.

> 11) Just because something compiles does not mean that it actually works,

Yes, but if it does not compile, then it can not work.

> relying on just syntax checking is dangerous as it gives false security as
> the code is not actually being tested. (in response to #2 for DCE)

It must be eventually tested. But catching errors earlier is better, it
spares developer time.

This argument is "it is not perfect, therefore it worthless". Well, your
bank account is not perfect, it does not contain all the money in the
world. Is it worthless? If you think so, feel free to give it to me :)

> 12) There are numerous FATE tests that should find all the configuration
> errors. (in response to #2 for DCE)

Please tell me how I can run the FATE instances for other platforms than
mine BEFORE pushing patches.

> 12) MSVC is broken and we shouldn't support it so Microsoft are forced to
> fix it (in response to #4 against DCE) - This point is countermanded by #3
> against DCE and reported issues with other compilers as well.

Regards,

-- 
  Nicolas George
___
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 decadi 30 frimaire, an CCXXV, Andreas Cadhalpun a écrit :
> I'm not aware of a user outside of ffmpeg and const changes shouldn't cause 
> big
> problems, as only the API changes, not the ABI. So it is probably OK.

Thanks.

> I'm not convinced that this is more convenient. APIchanges can still be
> changed after a commit, but the commit message can't. Also it can only replace
> APIchanges when everyone (including Libav) uses it.
> So I'd prefer if you added an APIchanges entry in addition to using this tag.

I can think of a few solutions, to merge the APIchanges and the snippets
from the commit messages and allow fixes.

But you are right in principle: I should not start using a feature
before it is implemented.

Regards,

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

Re: [FFmpeg-devel] [FFmpeg-cvslog] lavfi: make filter_frame non-recursive.

[Nicolas George]

Le decadi 30 frimaire, an CCXXV, Michael Niedermayer a écrit :
> > ./ffmpeg -i ~/tickets/860/jpeg2000.mov -vframes 3  -vcodec huffyuv -y 
> > test.avi

> can some bitexact flag be used to eliminate these (admitably) rare
> cases ?

The obvious answer would be: use -t instead of -vframes, it is already
bit-exact.

To make -vrames bit-exact, it would require ordering the frames in the
various streams by timestamp, like you did to make -shortest work. I do
not think it is worth it: -t is good.

Regards,

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

Re: [FFmpeg-devel] [PATCH 1/1] Updated version of patch 1840 (ticket 6021)

[Moritz Barsnick]

On Mon, Dec 19, 2016 at 03:05:18 +, Erik Bråthen Solem wrote:

> Subject: [FFmpeg-devel] [PATCH 1/1] Updated version of patch 1840 (ticket 
> 6021)

You need to give a proper commit message. Do have a look at other
commits in the repo.

The first line of the message (which is shown in this subject, the way
you are submitting your patch here) would be something like:

lavc/movtextdec: fix incorrect offset calculation for UTF-8 characters

(i.e. describe what it does [to what], not what it is)
and then an empty line and then the full description, which then also
has an extra line at the end such as:

Fixes trac #6021.

> Between testing and patch generation a character was deleted by mistake, which
> broke the patch. This updated version fixes this.

The way you are submitting your patch here, this text becomes part of
the commit message, but it doesn't belong there.

>  while (text < text_end) {
> -if (m->box_flags & STYL_BOX) {
> -for (i = 0; i < m->style_entries; i++) {
> -if (m->s[i]->style_flag && text_pos == m->s[i]->style_end) {
> -av_bprintf(buf, "{\\r}");
> +if ((*text & 0xC0) != 0x80) { // Boxes never split multibyte 
> characters
> +if (m->box_flags & STYL_BOX) {
> +for (i = 0; i < m->style_entries; i++) {
> +if (m->s[i]->style_flag &&
> +text_pos_chars == m->s[i]->style_end)
> +{
> +av_bprintf(buf, "{\\r}");
> +}
>  }

You are doing two to three things at the same time in this part of the
patch:
- wrapping existing code inside an if() clause or shifting its
  layering: fine
- re-indenting the code that has now moved into a new block:
  this is hard to review (i.e. to see that the block has remained
  unchanged) and it is usually requested that you do the re-indentation
  in a follow-up patch.
- re-formatting the code: even if useful, falls into the previous
  category
- re-formatting to non-ffmpeg style: that's a no-go.

(Let me guess that your editor is doing the reformatting for you.
Usually, that's nifty. ;-))

I believe the patch would be *much* easier to read and much shorter if
you only did the very first step in the initial patch.

> -return ff_ass_subtitle_header(avctx, m->d.font, m->d.fontsize, 
> m->d.color,
> -m->d.back_color, m->d.bold, m->d.italic,
> -m->d.underline, ASS_DEFAULT_BORDERSTYLE,
> -m->d.alignment);
> +return ff_ass_subtitle_header(avctx, m->d.font, m->d.fontsize,
> +m->d.color, m->d.back_color, m->d.bold,
> +m->d.italic, m->d.underline,
> +ASS_DEFAULT_BORDERSTYLE, m->d.alignment);

And as far as I can tell, this is *only* reformatting. This also
belongs into a separate patch (if at all useful).

> -if (m->tracksize + m->size_var + box_types[i].base_size 
> > avpkt->size)
> +if (m->tracksize + m->size_var +
> +box_types[i].base_size > avpkt->size)

Same here.

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

Re: [FFmpeg-devel] [PATCH 1/7] avutil: add FF_BAIL_ON_OVERFLOW

[wm4]

On Wed, 21 Dec 2016 01:43:46 +0100
Andreas Cadhalpun  wrote:

> On 20.12.2016 15:22, wm4 wrote:
> > On Mon, 19 Dec 2016 23:36:11 +0100
> > Andreas Cadhalpun  wrote:
> >   
> >> On 16.12.2016 17:22, wm4 wrote:  
> >>> On Fri, 16 Dec 2016 03:32:07 +0100
> >>> Andreas Cadhalpun  wrote:
> >>> 
>  Suggested-by: Rodger Combs 
>  Signed-off-by: Andreas Cadhalpun 
>  ---
>   libavutil/common.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
>  diff --git a/libavutil/common.h b/libavutil/common.h
>  index 8142b31..00b7504 100644
>  --- a/libavutil/common.h
>  +++ b/libavutil/common.h
>  @@ -99,6 +99,8 @@
>   #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= 
>  SWAP_tmp;}while(0)
>   #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0]))
>   
>  +#define FF_BAIL_ON_OVERFLOW(ctx, x) if (x) {av_log(ctx, AV_LOG_ERROR, 
>  "Overflow check failed: " #x"\n"); return AVERROR_INVALIDDATA;}
>  +
>   /* misc math functions */
>   
>   #ifdef HAVE_AV_CONFIG_H
> >>>
> >>> Are you sure we need the message?
> >>
> >> Yes, since such an overflow could just be a sign of a limitation in our
> >> framework (think of bit_rate being int32_t) and does not necessarily mean
> >> that the sample is invalid.
> >>  
> >>> It's quite ugly.
> >>
> >> Do you have any suggestions for improving it?  
> > 
> > I'm pretty much against such macros for rather specific use-cases, and
> > putting them into a public headers.  
> 
> It is added to an "internal and external API header".
> Feel free to send patches separating it into an internal and a public header.

Macros starting with FF are public API, so don't put that macro in a
public header. Or we're stuck with it forever.

> > I'm thinking it'd be better to actually provide overflow-checking 
> > primitives,  
> 
> Why?

Because that would have actual value, since overflowing checks are
annoying and there's also a chance to get them wrong. The code gets
less ugly too. If you're going to add such overflow checks to every
single operation in libavcodec that could overflow, you better come up
with something that won't add tons of crap to the code.

> > and I also don't think every overflow has to be logged.  
> 
> I disagree for the reason I mentioned above.

Which was? Not going to read the whole thread again.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Removing DCE

[wm4]

On Fri, 16 Dec 2016 13:48:16 +1100
Matt Oliver  wrote:

> Recently we have again received several patches that are trying to add
> workarounds for ffmpegs use of DCE. This is not the first time this has
> happened and wont be the last until a decision is made about the use of
> DCE. So I think its time that we made a official decision on the use of
> DCE. This is of course something that should be properly agreed upon by
> developers going forward as different people have different opinions on the
> matter so to help assist this I will summaries all of the previously made
> arguments from both sides of the discussion.
> 
> For DCE:
> 1) Ends up with a horrible mess of ifdefs.
> 2) Disabled parts of code will not be checked for syntax.
> 
> Against DCE:
> 3) DCE is not actually specified in the C specification. So compilers are
> actually being 100% compliant by not supporting it and should not be
> expected to change just for ffmpegs use case.
> 4) Breaks debug and lto builds on msvc.
> 5) Breaks debug and lto builds on icl.
> 6) Issues with lto with Clang and Gold.
> 7) Other unspecified issues with debug builds
> 
> Rebuttals against above arguments:
> 8) There are already 3961 #ifs(not including header guards) in the code so
> there is already a lot of code that doesn't use DCE. (In response to #1 for
> DCE).
> 9) Avoiding #ifdefs is a personal opinion as to whether they are ugly or
> not (some prefer ifdefs as IDEs will correctly highlight disabled
> sections). Someones personal preference for what looks better should not be
> justification to break supported configurations/compilers. (In response to
> #1 for DCE).
> 10) There is --enable-random which is supposed to be used to find
> configurations that don't compile. (in response to #2 for DCE).
> 11) Just because something compiles does not mean that it actually works,
> relying on just syntax checking is dangerous as it gives false security as
> the code is not actually being tested. (in response to #2 for DCE)
> 12) There are numerous FATE tests that should find all the configuration
> errors. (in response to #2 for DCE)
> 12) MSVC is broken and we shouldn't support it so Microsoft are forced to
> fix it (in response to #4 against DCE) - This point is countermanded by #3
> against DCE and reported issues with other compilers as well.
> 
> 
> Most of the above points were taken from peoples posts in the following
> mailing list thread:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-April/193437.html
> https://ffmpeg.org/pipermail/ffmpeg-devel/2016-May/194115.html
> 
> Its my personal opinion that DCE should be removed from the code but this
> is something I am aware will require a developer consensus and perhaps even
> a vote. Stating something is broken is one thing so I will also put forward
> a solution in that if it is agreed upon to remove DCE usage then I will
> spend the time and effort to go through the existing code base and replace
> DCE with appropriate #ifs.

I was completely lost here, until I opened one of the link and realized
you're talking about Dead Code Elimination.

Summary for those missing context: we rely on DCE to remove code that
would otherwise fail at link time.

For example consider this code, which will be compiled on _all_
platforms:

  void decode_something_x86();

  if (HAVE_X86)
 function_ptr = decode_something_x86;

Now if this is compiled on ARM, HAVE_X86 will be 0, and the
function_ptr assignment is dead code. DCE will remove this code, and
remove the decode_something_x86 reference. If DCE doesn't work, this
will fail to link, since decode_something_x86 will not be defined
anywhere on ARM.

I still think this is a bad idea, because compilers are not required to
perform DCE. In fact, ffmpeg will fail to compile with many compilers
if optimizations are disabled. This can make debugging a pain. Beyond
that, it could legitimately fail if the compiler just decides not to do
DCE for whatever reasons. (The argument has always been that a compiler
that fails to DCE such simple cases is not worth being used... strange
argument, since we work around other compiler bugs in a regular
fashion.)

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

Re: [FFmpeg-devel] Trying to ftp upload sample video

[wm4]

On Tue, 20 Dec 2016 20:34:28 +0100
Paul B Mahol  wrote:

> On 12/20/16, Ray Pasco  wrote:
> > There doesn't appear to be enough detail for uploading a sample file by
> > ftp.
> >
> > [image: Inline image 1]
> >
> > What are the values for *port* and *password* ?
> >  
> 
> Upload it somewhere else.

Maybe we should stop recommending the FTP for sample uploading? I never
had much luck on it (on both sides: uploading, or looking at something
someone else uploaded).

Also remove the recommendation for this datafilehost site. It's a
terrible POS site which deserves to die.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 6/6] lavfi: make AVFilterLink opaque in two major bumps.

[Michael Niedermayer]

On Wed, Dec 21, 2016 at 10:27:13AM +0100, Nicolas George wrote:
> Le decadi 30 frimaire, an CCXXV, Michael Niedermayer a écrit :
[...]
> 
> > iam not sure its usfull but
> > Another view from a different direction would be to see the filter
> > graph as a network of pipes with flowing water and interconnected
> > pumps. This too is a simple concept and intuition would suggest that
> > this would not easily end up with water being stuck nor too much
> > accumulating. It does not 1:1 match real filters though which work
> > on discrete frames, i wonder if this view is usefull
> > It would allow awnsering global questions about a graph, like what
> > inputs are useless when some output is welded shut or vice versa
> 
> Basically, you are suggesting to apply graph theory to the filter graph.
> That would be very smart. Alas to do that, it is necessary to actually
> know the graph. We do not: we do not know how inputs and outputs are
> connected within filters. For example, the connection is very different
> for overlay and concat, but from the outside they are indistinguishable.
> And select is even worse, of course.

The framework could monitor filters to determine their apparent
behavior. This would of course not give exact prediction of future
behavior.
I cant say how useful it would be to use this of course ...


> 
> > i think the original lavfi design didnt really had any issue with graphs 
> > with
> > multiple inputs or outputs. A user app could decide were to in and out
> > put. but FFmpeg didnt support this at the time IIRC so the people working
> > on the original lavfi had nothing to implement.
> > the problems came when this support was added much later
> 
> Not only that: before I added it (i.e. not in the original design),
> lavfi did not give the application enough information to decide what
> input to feed. It is still missing in lithe fork's implementation.

well, in the original design a filter graph can either be used in a
pull based application in which primarly data is requested from its
outputs and the requests recursivly move to its inputs triggering
data read through callbacks from some source filter. [applications
could implement their own source filter as there was a public API]

Or in a push based application each source would have a fifo,
if its empty the application needs to push data into the fifo, data
again is returned by requesting from the sink(s).

Which sink to pull data from could be determied by first pulling
from ones that had data when polled and then it would be up to the
application to decide, your lowest timestamp choice would have been
a possibility, keeping track of apparent in-out relations would
be another. (this was either way application side and not lavfis
choice)

So i stand by my oppinion that the original lavfi design didnt really
had an issue with graphs with multiple inputs or outputs.

No question it wasnt perfect and considering it wasnt used at the time
at all that shouldnt be surprising

but it really doesnt matter now, we moved forward from there and need
to move more forward


> 
> > > - Add a callback AVFilter.activate() to replace filter_frame() on all
> > >   inputs and request_frame() on all outputs. Most non-trivial filters
> > >   are written that way in the first place.
> > ok thats mostly cosmetic id say.
> 
> I think the difference between a mediocre API and a good one is partly
> cosmetic.
> 
> But this change is not cosmetic at all. Right now, if you want to notify
> a filter that EOF arrived on one input, you have to request a frame on
> one of its output and hope that it will in turn cause a read on that
> input. But you could end up pumping on another input instead.
> 
> > > - Change buffersink to implement that callback and peek directly in the
> > >   FIFO.
> > ok, "cosmetic"
> 
> Except for the EOF thingie, which is the biggest glitch at this time
> AFAIK.
> 
> > > - Rewrite framesync (the utility system for filters with several video
> > >   inputs that need synchroized frames) to implement activate and use the
> > >   FIFO directly.
> > cosmetic :)
> 
> Ditto.

differences in corner cases yes, i didnt mean to imply that its
purely and 100% cosmetic. More that its basically a cosmetic change
replacing how the more or less same code is triggered and that maybe
some of this could be done by some gsoc student or other volunteer.
Aka at least part of this seems time consuming but not highly complex
work.


> 
> > > - Allow to give buffersrc a timestamp on EOF, make sure the timestamp is
> > >   forwarded by most filters and allow to retrieve it from buffersink.
> > > 
> > >   (If somebody has a suggestion of a good data structure for that...)
> 
> Actually, the question in parentheses was about the priority queue, they
> got separated when I reorganized my paragraphs. Sorry.
> 
> > AVFrame.duration
> > This possibly is a big subject for discussion on its own, but maybe
> > not i dont know.
> 
> Indeed, there are pros and cons. I found 

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

[wm4]

On Tue, 20 Dec 2016 22:58:28 +0100
Nicolas George  wrote:

> Le decadi 30 frimaire, an CCXXV, wm4 a écrit :
> > > You mean a single structure returned by a single accessor with all the
> > > stream's properties instead of individual accessors for individual
> > > properties? Well, the naive methods of returning a structure to the
> > > caller would make the size of the structure part of the API, but there
> > > are ways around it.
> > > 
> > > I do not dislike the idea, if that is what people prefer.  
> 
> You did not answer that block. Was it an oversight?

No. I didn't see it as necessary. We seemed to be on the same page for
something that could be done theoretically.

> 
> > In general, replacing public fields with macro-generated accessors is
> > really just a rename.  
> 
> Yes. But I was thinking of this case specifically. In this case
> specifically, I think the accessors are an enhancement by themselves
> because they avoid digging in the filter's innards.

Not sure if I'd call that "innards". Though I admit that it requires
some understanding how the filters are linked together. Anyway,
accessing the links could be useful for inspecting other connections
between filters and so on. (With your patch this whole introspection
ability seems to go away? Though I'm not sure how much of it was
declared public API/ABI.)

> > AVOptions are unfortunately a good example how to create
> > encoder-specific options without "polluting" the common API. I don't
> > quite like AVOptions though, because they're too "fuzzy" for a C API
> > (av_opt essentially emulates dynamic/weak typing to some degree), and
> > they are typically even less well documented and defined like the C
> > API.  
> 
> I agree on this judgement about AVOptions.
> 
> > I don't really have a good idea how options specific to single codecs
> > or a small number of codecs should be handled. In some cases, side-data
> > is a good mechanism to move overly specific use-cases out of the common
> > API, especially for decoding.  
> 
> For that, I strongly disagree. Side data id an awful API. Whether it is
> implemented as side data or a separate field, there is an optional bit
> of information, and each part of the code needs to decide if it cares
> about it or not.
> 
> As is, side data brings exactly nothing. For each AVFrameSideDataType,
> we could have had a pointer field in the AVFrame structure, with NULL
> meaning it is not present, and that would have worked exactly the same.
> For our needs, really exactly. That would have cost 96 octets per
> frame.

Well yes, having such pointers to "optional" data would be equivalent
on some higher conceptual level, just with quite different ways to
use/access it in practice.

IMHO the important part is that presence or absence of a whole group of
fields can be signaled. Which is good for API. In general, side data
can be ignored until you're looking for certain information. That's
much better than just dumping everything into AVFrame, where everything
demands attention at once, along with essential fields.

> The differences that side data brings, or could bring if we needed it,
> or could have brought if it was implemented properly:
> 
> - Lots of accounting, memory management, unsafe casts, etc.
> 
> - Impossible to use complex data structure, side data is always flat.
> 
> - Side data can be repeated. But AFAIK we never use it. And fields could
>   point to an array or a list anyway.
> 
> - Side data is ordered. We do not use it.
> 
> - Side data can be processed in sequence, without looking at its type.
>   Except that to do anything useful except copying and freeing, we need
>   to know the type.
> 
>   So yeah, we gain something by using side data: each time we add a new
>   type, we gain two lines:
>   "if (av_something_copy(&dst->field, src->field)) goto fail;" in
>   frame_copy_props() and "av_something_freep(&frame->field)" in
>   av_frame_unref(). Big whoop!
> 
> - Side data could have been decentralized: lavc/fooenc and lavc/foodec
>   define their own type, nobody else cares about it; lavfi/foodetect and
>   lavfi/footweak define their own type, nobody else cares about it.
>   Except AVFrameSideDataType is centralized. Too bad.
> 
> I think some people entertained the fantasy that AVFrame could be
> "generic", but did not really think it through.

I agree about those points. I particularly dislike that we can't seem
to decide whether side data should be a bytestream (defined explicitly,
like a file format) or the contents of a struct (defined by C API/ABI).

You forgot to mention the code duplication between frame/packet/stream
side data management.

> At some point (after reinventing the options system as outlined in
> http://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/184525.html), I
> will probably propose to get rid of all this, unless it has actually
> become useful.
> 
> But it has gotten quite out of topic.
> 
> > If you look how Microsoft handles this, you'll see that they put
>

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

[wm4]

On Wed, 21 Dec 2016 01:56:59 +0100
Andreas Cadhalpun  wrote:

> On 20.12.2016 15:46, wm4 wrote:
> > In general, replacing public fields with macro-generated accessors is
> > really just a rename. Admittedly, it removes a confusing indirection
> > (though ->inputs[0]) in this case, but in general the improvements are
> > very minor. What does it matter if whether there are 100 fields or 100
> > set/get functions?  
> 
> There are several benefits of using accessors:
>  * It is much easier to keep the ABI stable for accessor functions than
>for public structs, because the order of members doesn't affect them.
>  * It is much easier to check which binary uses which ABI, because the
>functions are listed in the symbols table, while checking which
>struct member is accessed requires disassembling.
>  * Having the struct private means it can't be allocated on the stack
>by API users, preventing problems when the struct size changes.

Acknowledged. I was talking mostly about API.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Removing DCE

[Michael Niedermayer]

On Fri, Dec 16, 2016 at 01:48:16PM +1100, Matt Oliver wrote:
> Recently we have again received several patches that are trying to add
> workarounds for ffmpegs use of DCE. This is not the first time this has
> happened and wont be the last until a decision is made about the use of
> DCE. So I think its time that we made a official decision on the use of
> DCE. This is of course something that should be properly agreed upon by
> developers going forward as different people have different opinions on the
> matter so to help assist this I will summaries all of the previously made
> arguments from both sides of the discussion.
> 
> For DCE:
> 1) Ends up with a horrible mess of ifdefs.
> 2) Disabled parts of code will not be checked for syntax.
> 
> Against DCE:
> 3) DCE is not actually specified in the C specification. So compilers are
> actually being 100% compliant by not supporting it and should not be
> expected to change just for ffmpegs use case.
> 4) Breaks debug and lto builds on msvc.
> 5) Breaks debug and lto builds on icl.
> 6) Issues with lto with Clang and Gold.
> 7) Other unspecified issues with debug builds
> 
> Rebuttals against above arguments:
> 8) There are already 3961 #ifs(not including header guards) in the code so
> there is already a lot of code that doesn't use DCE. (In response to #1 for
> DCE).
> 9) Avoiding #ifdefs is a personal opinion as to whether they are ugly or
> not (some prefer ifdefs as IDEs will correctly highlight disabled
> sections). Someones personal preference for what looks better should not be
> justification to break supported configurations/compilers. (In response to
> #1 for DCE).
> 10) There is --enable-random which is supposed to be used to find
> configurations that don't compile. (in response to #2 for DCE).
> 11) Just because something compiles does not mean that it actually works,
> relying on just syntax checking is dangerous as it gives false security as
> the code is not actually being tested. (in response to #2 for DCE)
> 12) There are numerous FATE tests that should find all the configuration
> errors. (in response to #2 for DCE)
> 12) MSVC is broken and we shouldn't support it so Microsoft are forced to
> fix it (in response to #4 against DCE) - This point is countermanded by #3
> against DCE and reported issues with other compilers as well.
> 
> 
> Most of the above points were taken from peoples posts in the following
> mailing list thread:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-April/193437.html
> https://ffmpeg.org/pipermail/ffmpeg-devel/2016-May/194115.html
> 
> Its my personal opinion that DCE should be removed from the code but this
> is something I am aware will require a developer consensus and perhaps even
> a vote. Stating something is broken is one thing so I will also put forward
> a solution in that if it is agreed upon to remove DCE usage then I will
> spend the time and effort to go through the existing code base and replace
> DCE with appropriate #ifs.

how hard would it be to write a preprocessor like tool to convert
all if (ARCH/HAVE/CONFIG_SYMBOL ...)
to
#if
?

If that was doable and if someone wants to do it then this would
put another option on the table

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


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] lavf: identify MP2 as a distinct container from MP3

[Michael Niedermayer]

On Tue, Dec 20, 2016 at 10:48:12PM -0600, Rodger Combs wrote:
> This allows us to report the correct codec ID here
> ---
>  libavformat/allformats.c |  2 +-
>  libavformat/mp3dec.c | 62 
> +++-
>  libavformat/version.h|  2 +-
>  3 files changed, 42 insertions(+), 24 deletions(-)

this breaks demuxing some files

for example this one:
Input #0, mp3, from 'VIZ010-01_128.mups':
  Metadata:
title   : VIZ010-01_128
artist  : Joshua Iz feat. Chez Damier
album   : Sentimental Love (Remixes)
TIT1: VIZ010
copyright   : © & (P) Vizual Records 2011. All rights reserved.
encoded_by  : iTunes 10.4.1
date: 2011
  Duration: 00:02:00.01, start: 0.00, bitrate: 267 kb/s
Stream #0:0: Audio: mp3, 44100 Hz, stereo, s16p, 128 kb/s
Stream #0:1: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 1800x1800 
[SAR 100:100 DAR 1:1], 90k tbr, 90k tbn, 90k tbc
Metadata:
  comment : Other
Stream #0:2: Video: mjpeg, yuvj444p(pc, bt470bg/unknown/unknown), 1800x1800 
[SAR 1:1 DAR 1:1], 90k tbr, 90k tbn, 90k tbc
Metadata:
  comment : Other

after the patch:

VIZ010-01_128.mups: Invalid data found when processing input

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


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

[FFmpeg-devel] avcodec_open2 return -1

[sea]

In transcoding project of LeiXiaoHua(demo of usage of filters), if I use media 
file with h264 video stream as input file, the function  avcodec_open2 will 
return -1.  Media file with video stream in other format, such as mpeg4, mjpeg 
return 0 (OK). Why ? Someone meets the same problem?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 6/6] lavfi: make AVFilterLink opaque in two major bumps.

[Nicolas George]

Le primidi 1er nivôse, an CCXXV, Michael Niedermayer a écrit :
> The framework could monitor filters to determine their apparent
> behavior. This would of course not give exact prediction of future
> behavior.
> I cant say how useful it would be to use this of course ...

I do not see how to integrate that in something reliable, but somebody
may.

> well, in the original design a filter graph can either be used in a
> pull based application in which primarly data is requested from its
> outputs and the requests recursivly move to its inputs triggering
> data read through callbacks from some source filter. [applications
> could implement their own source filter as there was a public API]
> 
> Or in a push based application each source would have a fifo,
> if its empty the application needs to push data into the fifo, data
> again is returned by requesting from the sink(s).
> 
> Which sink to pull data from could be determied by first pulling
> from ones that had data when polled and then it would be up to the
> application to decide, your lowest timestamp choice would have been
> a possibility, keeping track of apparent in-out relations would
> be another. (this was either way application side and not lavfis
> choice)

I am not sure I can easily keep up the discussion: we are going back to
the basics of the scheduling, I worked on it in spring-summer 2012.
Since then, I remember a lot of "that does not work" but not all the
"because" that come after. I can give a few examples.

I think pull-only mode may work if the application is aware of all the
sinks and makes assumptions about their synchronization (i.e.
split -> normal speed + slow motion would not work), which seems
reasonable. Unfortunately, pull-only is the least useful way of using
that kind of library.

Push-only mode does not work with several inputs (or sources), because
you can not know which one needs a frame (the actual need may be far
downstream), and assumptions about synchronization are really not
acceptable in input.

Local-mixed pull-push (i.e. filters can issue a pull in reaction to a
push and reciprocally) solves these issues, but can result in infinite
loops: split pushes, first on out0 connected to overlay in0, overlays
pulls on in1 connected to split out1, split pushes, Redo From Start,
stack overflow. That was what we had before 2012.

Global-mixed pull-push (i.e. the application pushes on inputs and drives
by pulling on outputs, but filters are not allowed to make U-turns)
works, with the same caveats as pull-only. That is what we have since
2012.

And on top of that, you have to consider the case of inputs versus
sources that are always ready like testsrc. They bring their own
complications.

Of course, any of these issues can be solved using flags or something,
but the hard part is to solve all of them at the same time. We no longer
are at the simple original design, but a complicated set of workarounds
on top of the original design.

And the new non-recursive design is not more complex than the recursive
one, the one that works. It is the same, plus the code for the FIFO. If
it was done like that in the first place, it would have worked fine. The
complex part in it is the compatibility layer: use filters designed for
the recursive version unchanged in the non-recursive version. Hopefully,
some of that complexity will go away as difficult filters are adapted.

Plus, unlike the recursive design that mandates a depth-first order of
processing, the non-recursive design can work in any order. I suspect it
will give us the local-mixed pull-push mode for almost free. But I have
yet to test.

> differences in corner cases yes, i didnt mean to imply that its
> purely and 100% cosmetic. More that its basically a cosmetic change
> replacing how the more or less same code is triggered and that maybe
> some of this could be done by some gsoc student or other volunteer.
> Aka at least part of this seems time consuming but not highly complex
> work.

Indeed. And I must say it makes a nice change after a year spent
understanding why corner cases in the non-recursive design did not work.

> i dont think not knowing the duration is a problem.
> you need replicated frames possibly elsewere already. Like for
> subtitles, its not much different to duplicating the last frame with
> the remainining to EOF duration to be added to the last 1 or 0 duration
> but i didnt think deeply about this now so i might miss details
> the issue is also not specific to subtitles, audio tracks with "holes"
> in them exist too so do video slidshows. At least in some usecases
> limiting the distance between frames is needed. (for example to
> ensure random access as in keyframes. The issue can to some extend
> be pushed into the container format i guess but for truely streamed
> formats if you dont repeat your video frame and subtitles which
> are currently disaplyed it just wont get displayed if you start viewing
> around that point)
> so to me it seems there are a lo

Re: [FFmpeg-devel] [PATCH 1/3] lavf: identify MP2 as a distinct container from MP3

[Rodger Combs]

> On Dec 21, 2016, at 02:27, Carl Eugen Hoyos  wrote:
> 
> 2016-12-21 5:48 GMT+01:00 Rodger Combs :
>> This allows us to report the correct codec ID here
> 
> Just curious: What does this fix?

Reporting in ffprobe, or when using lavf as a library. Some devices and 
decoders either refuse to decode MP2, or need to be told that the input is MP2 
as opposed to MP3 ahead of time. This also means we'll write the correct ID 
when remuxing.

> 
> 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 2/2] lavfi: make filter_frame non-recursive.

[Marton Balint]

On Sun, 18 Dec 2016, Nicolas George wrote:


Le primidi 21 frimaire, an CCXXV, Nicolas George a écrit :

I will do so after the end of this week if I see no news.


Done.



It seems after this patch I got an infinite loop if I try to convert the 
attached file using this command line:


./ffmpeg -i amerge-test.mov -filter_complex "[0:a:0][0:a:1]amerge=2[aout]" 
-map "[aout]" out.wav


Note that in my build FF_BUFQUEUE_SIZE in libavfilter/bufferqueue.h is set 
to 256, because otherwise it errors out with ENOMEM, but that also happens 
with the old filtering code. On the other hand, the old filtering code and 
FF_BUFQUEUE_SIZE set to 256 does allow the file to properly convert.


Thanks,
Marton

amerge-test.mov
Description: QuickTime movie
___
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 primidi 1er nivôse, an CCXXV, Marton Balint a écrit :
> It seems after this patch I got an infinite loop if I try to convert the
> attached file using this command line:
> 
> ./ffmpeg -i amerge-test.mov -filter_complex "[0:a:0][0:a:1]amerge=2[aout]"
> -map "[aout]" out.wav

That is a bigger glitch. I will look into it as soon as possible.

Regards,

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

Re: [FFmpeg-devel] Trying to ftp upload sample video

[compn]

On Wed, 21 Dec 2016 14:31:43 +0100
wm4  wrote:

> On Tue, 20 Dec 2016 20:34:28 +0100
> Paul B Mahol  wrote:
> 
> > Upload it somewhere else.
> 
> Maybe we should stop recommending the FTP for sample uploading? I
> never had much luck on it (on both sides: uploading, or looking at
> something someone else uploaded).

we need to talk to vlc about getting better ftp/http etc with our
incoming dir.

> Also remove the recommendation for this datafilehost site. It's a
> terrible POS site which deserves to die.

agree dfh sucks, but it would also be nice to have a replacement. one
that accepts large files.

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

Re: [FFmpeg-devel] [PATCH 1/3] lavf: identify MP2 as a distinct container from MP3

[compn]

On Wed, 21 Dec 2016 12:51:18 -0600
Rodger Combs  wrote:

> 
> > On Dec 21, 2016, at 02:27, Carl Eugen Hoyos 
> > wrote:
> > 
> > 2016-12-21 5:48 GMT+01:00 Rodger Combs :
> >> This allows us to report the correct codec ID here
> > 
> > Just curious: What does this fix?
> 
> Reporting in ffprobe, or when using lavf as a library. Some devices
> and decoders either refuse to decode MP2, or need to be told that the
> input is MP2 as opposed to MP3 ahead of time. This also means we'll
> write the correct ID when remuxing.

if mp2 is a problem, i would rather make it more difficult for the user
to encode mp2. by defaulting to mp3 and / or printing a warning message
about mp2. honestly i cant remember any good coming from someone using
mp2.

mp2 is dead, long live mp3.

also please share your mp2 sample, if you have not already done so, if
possible.

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

Re: [FFmpeg-devel] [PATCH 1/3] lavf: identify MP2 as a distinct container from MP3

[James Almer]

On 12/21/2016 4:50 PM, compn wrote:
> On Wed, 21 Dec 2016 12:51:18 -0600
> Rodger Combs  wrote:
> 
>>
>>> On Dec 21, 2016, at 02:27, Carl Eugen Hoyos 
>>> wrote:
>>>
>>> 2016-12-21 5:48 GMT+01:00 Rodger Combs :
 This allows us to report the correct codec ID here
>>>
>>> Just curious: What does this fix?
>>
>> Reporting in ffprobe, or when using lavf as a library. Some devices
>> and decoders either refuse to decode MP2, or need to be told that the
>> input is MP2 as opposed to MP3 ahead of time. This also means we'll
>> write the correct ID when remuxing.
> 
> if mp2 is a problem, i would rather make it more difficult for the user
> to encode mp2. by defaulting to mp3 and / or printing a warning message
> about mp2. honestly i cant remember any good coming from someone using
> mp2.

We use it to encode bitexact audio for FATE tests :P

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

[FFmpeg-devel] [PATCH] avcodec: add Apple Pixlet decoder

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 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 | 756 
 libavformat/isom.c  |   2 +
 8 files changed, 770 insertions(+)
 create mode 100644 libavcodec/pixlet.c

diff --git a/Changelog b/Changelog
index b36a631..c03f8f2 100644
--- a/Changelog
+++ b/Changelog
@@ -9,6 +9,7 @@ version :
 - Support for spherical videos
 - configure now fails if autodetect-libraries are requested but not found
 - PSD Decoder
+- Apple Pixlet decoder
 
 version 3.2:
 - libopenmpt demuxer
diff --git a/doc/general.texi b/doc/general.texi
index 9ea3ba3..8f88b37 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -627,6 +627,7 @@ following image formats are supported:
 @item ANSI/ASCII art @tab @tab  X
 @item Apple Intermediate Codec @tab @tab  X
 @item Apple MJPEG-B  @tab @tab  X
+@item Apple Pixlet   @tab @tab  X
 @item Apple ProRes   @tab  X  @tab  X
 @item Apple QuickDraw@tab @tab  X
 @tab fourcc: qdrw
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 23e41dd..5e8eb67 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -452,6 +452,7 @@ OBJS-$(CONFIG_PGMYUV_DECODER)  += pnmdec.o pnm.o
 OBJS-$(CONFIG_PGMYUV_ENCODER)  += pnmenc.o
 OBJS-$(CONFIG_PGSSUB_DECODER)  += pgssubdec.o
 OBJS-$(CONFIG_PICTOR_DECODER)  += pictordec.o cga_data.o
+OBJS-$(CONFIG_PIXLET_DECODER)  += pixlet.o
 OBJS-$(CONFIG_PJS_DECODER) += textdec.o ass.o
 OBJS-$(CONFIG_PNG_DECODER) += png.o pngdec.o pngdsp.o
 OBJS-$(CONFIG_PNG_ENCODER) += png.o pngenc.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index bbcecce..9df6390 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -281,6 +281,7 @@ void avcodec_register_all(void)
 REGISTER_ENCDEC (PGM,   pgm);
 REGISTER_ENCDEC (PGMYUV,pgmyuv);
 REGISTER_DECODER(PICTOR,pictor);
+REGISTER_DECODER(PIXLET,pixlet);
 REGISTER_ENCDEC (PNG,   png);
 REGISTER_ENCDEC (PPM,   ppm);
 REGISTER_ENCDEC (PRORES,prores);
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 098debf..9699f70 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -412,6 +412,7 @@ enum AVCodecID {
 AV_CODEC_ID_SHEERVIDEO,
 AV_CODEC_ID_YLC,
 AV_CODEC_ID_PSD,
+AV_CODEC_ID_PIXLET,
 
 /* various PCM "codecs" */
 AV_CODEC_ID_FIRST_AUDIO = 0x1, ///< A dummy id pointing at the 
start of audio codecs
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 29ffcb9..f09d047 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1339,6 +1339,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
 .long_name = NULL_IF_CONFIG_SMALL("YUY2 Lossless Codec"),
 .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
 },
+{
+.id= AV_CODEC_ID_PIXLET,
+.type  = AVMEDIA_TYPE_VIDEO,
+.name  = "pixlet",
+.long_name = NULL_IF_CONFIG_SMALL("Apple Pixlet"),
+.props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
+},
 
 /* image codecs */
 {
diff --git a/libavcodec/pixlet.c b/libavcodec/pixlet.c
new file mode 100644
index 000..6b78fed
--- /dev/null
+++ b/libavcodec/pixlet.c
@@ -0,0 +1,756 @@
+/*
+ * Apple Pixlet decoder
+ * Copyright (c) 2016 Paul B Mahol
+ * Copyright (c) 2015 Vittorio Giovara 
+ *
+ * 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 "libavutil/imgutils.h"
+#include "libavutil/intmath.h"
+#include "libavutil/opt.h"
+
+#include "avcodec.h"
+#include "bytestream.h"
+#include "get_bits.h"
+#include "unary.h"
+#include "internal.h"
+#include "thread.h"
+
+#define NB_LEVELS 4
+
+#define H 0
+#define V 1
+
+typedef struct SubBand {
+unsigned width;
+unsigned height;
+unsigned size;
+int16_t *coeffs;
+} SubBand;
+
+typedef struct LowPass {
+unsigned width;
+unsigned height;

Re: [FFmpeg-devel] [PATCH 1/3] lavf: identify MP2 as a distinct container from MP3

[Rodger Combs]

> On Dec 21, 2016, at 13:50, compn  wrote:
> 
> On Wed, 21 Dec 2016 12:51:18 -0600
> Rodger Combs  wrote:
> 
>> 
>>> On Dec 21, 2016, at 02:27, Carl Eugen Hoyos 
>>> wrote:
>>> 
>>> 2016-12-21 5:48 GMT+01:00 Rodger Combs :
 This allows us to report the correct codec ID here
>>> 
>>> Just curious: What does this fix?
>> 
>> Reporting in ffprobe, or when using lavf as a library. Some devices
>> and decoders either refuse to decode MP2, or need to be told that the
>> input is MP2 as opposed to MP3 ahead of time. This also means we'll
>> write the correct ID when remuxing.
> 
> if mp2 is a problem, i would rather make it more difficult for the user
> to encode mp2. by defaulting to mp3 and / or printing a warning message
> about mp2. honestly i cant remember any good coming from someone using
> mp2.
> 
> mp2 is dead, long live mp3.
> 
> also please share your mp2 sample, if you have not already done so, if
> possible.

Here's the sample provided by a user: http://www.ste.no/jens/sample.mp4 

They said they're using MP2 because of some audio desync issue they get when 
using VLC to stream-dump ("well there's your problem"), but I don't think the 
reasoning behind this particular sample is relevant to the legitimacy of the 
issue.

> 
> -compn
> ___
> 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] lavfi: add FFFrameQueue API.

[James Almer]

On 11/27/2016 1:08 PM, 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
> 
> 
> Unchanged.
> 
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile

This patchset broke 83 fate tests using valgrind. Looks like memleaks.

http://fate.ffmpeg.org/report.cgi?time=20161221122451&slot=x86_64-archlinux-gcc-valgrindundef
http://fate.ffmpeg.org/report.cgi?time=20161221133057&slot=x86_64-archlinux-gcc-valgrind

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

Re: [FFmpeg-devel] [PATCH 1/2] lavfi: add FFFrameQueue API.

[Nicolas George]

Le primidi 1er nivôse, an CCXXV, James Almer a écrit :
> This patchset broke 83 fate tests using valgrind. Looks like memleaks.
> 
> http://fate.ffmpeg.org/report.cgi?time=20161221122451&slot=x86_64-archlinux-gcc-valgrindundef
> http://fate.ffmpeg.org/report.cgi?time=20161221133057&slot=x86_64-archlinux-gcc-valgrind

I thought I had got them all :( I will look into it.

Regards,

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

Re: [FFmpeg-devel] [PATCH 1/3] lavf: identify MP2 as a distinct container from MP3

[Nicolas George]

Le primidi 1er nivôse, an CCXXV, compn a écrit :
> if mp2 is a problem, i would rather make it more difficult for the user
> to encode mp2. by defaulting to mp3 and / or printing a warning message
> about mp2. honestly i cant remember any good coming from someone using
> mp2.
> 
> mp2 is dead, long live mp3.

IIRC, Michael said that it was probably rather easy to get the MP2
encoder do output a MP3 bistream, although without using the MP3
features and having only MP2 quality. Not something I would be able to
do, though.

Regards,

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

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[James Almer]

On 12/20/2016 11:36 PM, Michael Niedermayer wrote:
> On Tue, Dec 20, 2016 at 04:53:51PM -0800, Thomas Turner wrote:
>> Signed-off-by: Thomas Turner 
>> ---
>>  libavutil/Makefile   |   1 +
>>  libavutil/tests/audio_fifo.c | 196 +
>>  tests/fate/libavutil.mak |   4 +
>>  tests/ref/fate/audio_fifo| 228 
>> +++
>>  4 files changed, 429 insertions(+)
>>  create mode 100644 libavutil/tests/audio_fifo.c
>>  create mode 100644 tests/ref/fate/audio_fifo
> 
> applied
> 
> thx

This is crashing on some fate clients.

I noticed it's using malloc and free instead of the av_malloc family,
so maybe it's related to that?
Patch attached in any case, it's proper even if not the reason behind
the crashes.

>From 61898e2580a67250cb3e1fb25a4e170fd00feada Mon Sep 17 00:00:00 2001
From: James Almer 
Date: Wed, 21 Dec 2016 21:41:20 -0300
Subject: [PATCH] test/audio_fifo: use av_malloc() family of functions

Signed-off-by: James Almer 
---
 libavutil/tests/audio_fifo.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/libavutil/tests/audio_fifo.c b/libavutil/tests/audio_fifo.c
index dbadded..e9b1470 100644
--- a/libavutil/tests/audio_fifo.c
+++ b/libavutil/tests/audio_fifo.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include "libavutil/mem.h"
 #include "libavutil/audio_fifo.c"
 
 #define MAX_CHANNELS32
@@ -50,15 +51,6 @@ static void ERROR(const char *str)
 exit(1);
 }
 
-static void* allocate_memory(size_t size)
-{
-void *ptr = malloc(size);
-if (ptr == NULL){
-ERROR("failed to allocate memory!");
-}
-return ptr;
-}
-
 static void print_audio_bytes(const TestStruct *test_sample, void **data_planes, int nb_samples)
 {
 int p, b, f;
@@ -85,11 +77,15 @@ static int read_samples_from_audio_fifo(AVAudioFifo* afifo, void ***output, int
 int samples= FFMIN(nb_samples, afifo->nb_samples);
 int tot_elements   = !(planes = av_sample_fmt_is_planar(afifo->sample_fmt))
  ? samples : afifo->channels * samples;
-void **data_planes = allocate_memory(sizeof(void*) * planes);
+void **data_planes = av_malloc_array(planes, sizeof(void*));
+if (!data_planes)
+ERROR("failed to allocate memory!");
 *output= data_planes;
 
 for (i = 0; i < afifo->nb_buffers; ++i){
-data_planes[i] = allocate_memory(afifo->sample_size * tot_elements);
+data_planes[i] = av_malloc_array(tot_elements, afifo->sample_size);
+if (!data_planes[i])
+ERROR("failed to allocate memory!");
 }
 
 return av_audio_fifo_read(afifo, *output, nb_samples);
@@ -178,9 +174,9 @@ static void test_function(const TestStruct test_sample)
 
 /* deallocate */
 for (i = 0; i < afifo->nb_buffers; ++i){
-free(output_data[i]);
+av_freep(&output_data[i]);
 }
-free(output_data);
+av_freep(&output_data);
 av_audio_fifo_free(afifo);
 }
 
-- 
2.10.2

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

Re: [FFmpeg-devel] [PATCH] Add initial program date time option (hls_init_prog_time)

[Steven Liu]

2016-12-18 10:10 GMT+08:00 Steven Liu :

>
>
> 2016-12-17 15:58 GMT+08:00 Robert Nagy :
>
>> From 14da4c9610ac0cf257b2c28f21899e854592e646 Mon Sep 17 00:00:00 2001
>> From: Jesper Ek 
>> Date: Wed, 7 Dec 2016 16:01:08 +0100
>> Subject: [PATCH] Add initial program date time option (hls_init_prog_time)
>>
>> It is often useful to specify the initial program date time, rather
>> than relying on the current system time. This commit adds an argument
>> option to specify the number of seconds since epoch.
>> ---
>>  libavformat/hlsenc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index d03cf02..a0c8cfc 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -791,7 +791,7 @@ static int hls_write_header(AVFormatContext *s)
>>  hls->recording_time = (hls->init_time ? hls->init_time : hls->time) *
>> AV_TIME_BASE;
>>  hls->start_pts  = AV_NOPTS_VALUE;
>>
>> -if (hls->flags & HLS_PROGRAM_DATE_TIME) {
>> +if (hls->flags & HLS_PROGRAM_DATE_TIME && hls->initial_prog_date_time
>> == 0) {
>>
> This maybe can more simple.
>
>>  time_t now0;
>>  time(&now0);
>>  hls->initial_prog_date_time = now0;
>> @@ -1101,6 +1101,7 @@ static const AVOption options[] = {
>>  {"start_number",  "set first number in the sequence",
>>  OFFSET(start_sequence),AV_OPT_TYPE_INT64,  {.i64 = 0}, 0, INT64_MAX,
>> E},
>>  {"hls_time",  "set segment length in seconds",
>> OFFSET(time),AV_OPT_TYPE_FLOAT,  {.dbl = 2}, 0, FLT_MAX, E},
>>  {"hls_init_time", "set segment length in seconds at init list",
>> OFFSET(init_time),AV_OPT_TYPE_FLOAT,  {.dbl = 0}, 0, FLT_MAX,
>> E},
>> +{"hls_init_prog_time", "set initial program date time in seconds
>> since
>> epoch", OFFSET(initial_prog_date_time),AV_OPT_TYPE_DOUBLE,  {.dbl =
>> 0},
>> 0, DBL_MAX, E},
>>  {"hls_list_size", "set maximum number of playlist entries",
>>  OFFSET(max_nb_segments),AV_OPT_TYPE_INT,{.i64 = 5}, 0,
>> INT_MAX, E},
>>  {"hls_ts_options","set hls mpegts list of options for the container
>> format used for hls", OFFSET(format_options_str), AV_OPT_TYPE_STRING,
>> {.str
>> = NULL},  0, 0,E},
>>  {"hls_vtt_options","set hls vtt list of options for the container
>> format used for hls", OFFSET(vtt_format_options_str), AV_OPT_TYPE_STRING,
>> {.str = NULL},  0, 0,E},
>> --
>> 2.10.0
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> patch broke by newline :(
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[Michael Niedermayer]

On Wed, Dec 21, 2016 at 09:45:39PM -0300, James Almer wrote:
> On 12/20/2016 11:36 PM, Michael Niedermayer wrote:
> > On Tue, Dec 20, 2016 at 04:53:51PM -0800, Thomas Turner wrote:
> >> Signed-off-by: Thomas Turner 
> >> ---
> >>  libavutil/Makefile   |   1 +
> >>  libavutil/tests/audio_fifo.c | 196 +
> >>  tests/fate/libavutil.mak |   4 +
> >>  tests/ref/fate/audio_fifo| 228 
> >> +++
> >>  4 files changed, 429 insertions(+)
> >>  create mode 100644 libavutil/tests/audio_fifo.c
> >>  create mode 100644 tests/ref/fate/audio_fifo
> > 
> > applied
> > 
> > thx
> 
> This is crashing on some fate clients.
> 
> I noticed it's using malloc and free instead of the av_malloc family,
> so maybe it's related to that?
> Patch attached in any case, it's proper even if not the reason behind
> the crashes.

i saw the malloc/free before applying. It seemed to make sense to
use libc functions in a test of our public API

why is it crashing ?

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

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


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

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[James Almer]

On 12/21/2016 10:49 PM, Michael Niedermayer wrote:
> On Wed, Dec 21, 2016 at 09:45:39PM -0300, James Almer wrote:
>> On 12/20/2016 11:36 PM, Michael Niedermayer wrote:
>>> On Tue, Dec 20, 2016 at 04:53:51PM -0800, Thomas Turner wrote:
 Signed-off-by: Thomas Turner 
 ---
  libavutil/Makefile   |   1 +
  libavutil/tests/audio_fifo.c | 196 +
  tests/fate/libavutil.mak |   4 +
  tests/ref/fate/audio_fifo| 228 
 +++
  4 files changed, 429 insertions(+)
  create mode 100644 libavutil/tests/audio_fifo.c
  create mode 100644 tests/ref/fate/audio_fifo
>>>
>>> applied
>>>
>>> thx
>>
>> This is crashing on some fate clients.
>>
>> I noticed it's using malloc and free instead of the av_malloc family,
>> so maybe it's related to that?
>> Patch attached in any case, it's proper even if not the reason behind
>> the crashes.
> 
> i saw the malloc/free before applying. It seemed to make sense to
> use libc functions in a test of our public API
> 
> why is it crashing ?

I don't know, i can't reproduce it. Seems to be OpenBSD FATE clients only
as far as i could see.

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

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[Michael Niedermayer]

On Wed, Dec 21, 2016 at 10:53:36PM -0300, James Almer wrote:
> On 12/21/2016 10:49 PM, Michael Niedermayer wrote:
> > On Wed, Dec 21, 2016 at 09:45:39PM -0300, James Almer wrote:
> >> On 12/20/2016 11:36 PM, Michael Niedermayer wrote:
> >>> On Tue, Dec 20, 2016 at 04:53:51PM -0800, Thomas Turner wrote:
>  Signed-off-by: Thomas Turner 
>  ---
>   libavutil/Makefile   |   1 +
>   libavutil/tests/audio_fifo.c | 196 +
>   tests/fate/libavutil.mak |   4 +
>   tests/ref/fate/audio_fifo| 228 
>  +++
>   4 files changed, 429 insertions(+)
>   create mode 100644 libavutil/tests/audio_fifo.c
>   create mode 100644 tests/ref/fate/audio_fifo
> >>>
> >>> applied
> >>>
> >>> thx
> >>
> >> This is crashing on some fate clients.
> >>
> >> I noticed it's using malloc and free instead of the av_malloc family,
> >> so maybe it's related to that?
> >> Patch attached in any case, it's proper even if not the reason behind
> >> the crashes.
> > 
> > i saw the malloc/free before applying. It seemed to make sense to
> > use libc functions in a test of our public API
> > 
> > why is it crashing ?
> 
> I don't know, i can't reproduce it. Seems to be OpenBSD FATE clients only
> as far as i could see.

no openbsd needed, valgrind shows these errors
Thomas, can you look at this ?

TEST: 1

written: 12
written: 12
remaining samples in audio_fifo: 24

==32011== Invalid write of size 8
==32011==at 0x4016B2: read_samples_from_audio_fifo (audio_fifo.c:92)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
==32011== Invalid read of size 8
==32011==at 0x401320: av_audio_fifo_read (audio_fifo.c:193)
==32011==by 0x4016DD: read_samples_from_audio_fifo (audio_fifo.c:95)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
read: 12
==32011== Invalid read of size 8
==32011==at 0x4015A8: print_audio_bytes (audio_fifo.c:74)
==32011==by 0x401900: test_function (audio_fifo.c:146)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
00 01 02 03 04 05 06 07 08 09 0a 0b
remaining samples in audio_fifo: 12

==32011== Invalid read of size 8
==32011==at 0x401164: av_audio_fifo_peek (audio_fifo.c:150)
==32011==by 0x401937: test_function (audio_fifo.c:150)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
peek:
==32011== Invalid read of size 8
==32011==at 0x4015A8: print_audio_bytes (audio_fifo.c:74)
==32011==by 0x401967: test_function (audio_fifo.c:155)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
00 01 02 03 04 05 06 07 08 09 0a 0b

peek_at:
==32011== Invalid read of size 8
==32011==at 0x401264: av_audio_fifo_peek_at (audio_fifo.c:174)
==32011==by 0x40199E: test_function (audio_fifo.c:161)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x401

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[Thomas Turner]

yeah, currently taking a look.


On 12/21/2016 06:08 PM, Michael Niedermayer wrote:

On Wed, Dec 21, 2016 at 10:53:36PM -0300, James Almer wrote:

On 12/21/2016 10:49 PM, Michael Niedermayer wrote:

On Wed, Dec 21, 2016 at 09:45:39PM -0300, James Almer wrote:

On 12/20/2016 11:36 PM, Michael Niedermayer wrote:

On Tue, Dec 20, 2016 at 04:53:51PM -0800, Thomas Turner wrote:

Signed-off-by: Thomas Turner 
---
  libavutil/Makefile   |   1 +
  libavutil/tests/audio_fifo.c | 196 +
  tests/fate/libavutil.mak |   4 +
  tests/ref/fate/audio_fifo| 228 +++
  4 files changed, 429 insertions(+)
  create mode 100644 libavutil/tests/audio_fifo.c
  create mode 100644 tests/ref/fate/audio_fifo

applied

thx

This is crashing on some fate clients.

I noticed it's using malloc and free instead of the av_malloc family,
so maybe it's related to that?
Patch attached in any case, it's proper even if not the reason behind
the crashes.

i saw the malloc/free before applying. It seemed to make sense to
use libc functions in a test of our public API

why is it crashing ?

I don't know, i can't reproduce it. Seems to be OpenBSD FATE clients only
as far as i could see.

no openbsd needed, valgrind shows these errors
Thomas, can you look at this ?

TEST: 1

written: 12
written: 12
remaining samples in audio_fifo: 24

==32011== Invalid write of size 8
==32011==at 0x4016B2: read_samples_from_audio_fifo (audio_fifo.c:92)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
==32011== Invalid read of size 8
==32011==at 0x401320: av_audio_fifo_read (audio_fifo.c:193)
==32011==by 0x4016DD: read_samples_from_audio_fifo (audio_fifo.c:95)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
read: 12
==32011== Invalid read of size 8
==32011==at 0x4015A8: print_audio_bytes (audio_fifo.c:74)
==32011==by 0x401900: test_function (audio_fifo.c:146)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
00 01 02 03 04 05 06 07 08 09 0a 0b
remaining samples in audio_fifo: 12

==32011== Invalid read of size 8
==32011==at 0x401164: av_audio_fifo_peek (audio_fifo.c:150)
==32011==by 0x401937: test_function (audio_fifo.c:150)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
peek:
==32011== Invalid read of size 8
==32011==at 0x4015A8: print_audio_bytes (audio_fifo.c:74)
==32011==by 0x401967: test_function (audio_fifo.c:155)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_samples_from_audio_fifo (audio_fifo.c:88)
==32011==by 0x4018C6: test_function (audio_fifo.c:141)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==
00 01 02 03 04 05 06 07 08 09 0a 0b

peek_at:
==32011== Invalid read of size 8
==32011==at 0x401264: av_audio_fifo_peek_at (audio_fifo.c:174)
==32011==by 0x40199E: test_function (audio_fifo.c:161)
==32011==by 0x401AE2: main (audio_fifo.c:193)
==32011==  Address 0x540f040 is 0 bytes after a block of size 0 alloc'd
==32011==at 0x4C2C66F: malloc (vg_replace_malloc.c:270)
==32011==by 0x4014E5: allocate_memory (audio_fifo.c:55)
==32011==by 0x401674: read_s

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[James Almer]

On 12/21/2016 11:22 PM, Thomas Turner wrote:
> yeah, currently taking a look.

int tot_elements   = !(planes = av_sample_fmt_is_planar(afifo->sample_fmt))
 ? samples : afifo->channels * samples;
void **data_planes = allocate_memory(sizeof(void*) * planes);

planes is zero when the sample_fmt is not planar, so you end up
calling malloc(0).
It should be channel count if planar, 1 otherwise. I think you
can just call malloc with afifo->nb_buffers * sizeof(void*) as
size.

This is also a good reason to use av_malloc_array() instead of
a plain malloc().

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

Re: [FFmpeg-devel] [PATCH] avutil: Added selftest for libavutil/audio_fifo.c

[Thomas Turner]

Yes, you're correct. I'll look over the test and make sure there isn't
anymore bugs before sending in the patch. Thanks

On Dec 21, 2016 6:28 PM, "James Almer"  wrote:

On 12/21/2016 11:22 PM, Thomas Turner wrote:
> yeah, currently taking a look.

int tot_elements   = !(planes = av_sample_fmt_is_planar(afifo->sample_fmt))
 ? samples : afifo->channels * samples;
void **data_planes = allocate_memory(sizeof(void*) * planes);

planes is zero when the sample_fmt is not planar, so you end up
calling malloc(0).
It should be channel count if planar, 1 otherwise. I think you
can just call malloc with afifo->nb_buffers * sizeof(void*) as
size.

This is also a good reason to use av_malloc_array() instead of
a plain malloc().

___
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