[FFmpeg-devel] [PATCH] ffmpeg_opt: deprecate the hwaccel_lax_profile_check option

2017-10-13 Thread Jun Zhao

From 40c45849b5146688ce6f88ca4fe20b771fb2de46 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Fri, 13 Oct 2017 02:53:01 -0400
Subject: [PATCH] ffmpeg_opt: deprecate the hwaccel_lax_profile_check option

deprecate hwaccel_lax_profile_check. This only was used for VAAPI
hwaccel decoder, now use per-stream hwaccel_flags for all hwaccel
decoders.

Signed-off-by: Jun Zhao 
---
 fftools/ffmpeg_opt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 100fa76e46..f1c96ce08c 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -100,7 +100,7 @@ const HWAccel hwaccels[] = {
 #endif
 { 0 },
 };
-int hwaccel_lax_profile_check = 0;
+attribute_deprecated int hwaccel_lax_profile_check = 0;
 AVBufferRef *hw_device_ctx;
 HWDevice *filter_hw_device;
 
-- 
2.11.0

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


Re: [FFmpeg-devel] [PATCH] udp: added option to ignore empty UDP packets

2017-10-13 Thread Daniel Kučera
>
> The good approach is to fix the bugs. You do not need to fix all of
> them, just fix the one or two instances that break things for you: in
> your use case, there is one line of code in the whole project that gets
> ret=0 and thinks it means EOF: find it and fix it, and you should be
> good. Other instance, if any, can be fixed as needed.
>

I tried this approach but when I changed that one critical line, 10
other things got broken according to fate. Even if I fixed them and
posted patches here in list, I was sugested to thorougouhly test all
other protocol modules to make sure it doesn't break anything else but
that's not possible (at least I'm not able to).

So what other option I have than trying to submit a workaround?

-- 

S pozdravom / Best regards
Daniel Kucera.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 03/20] lavc: Add coded bitstream read/write API

2017-10-13 Thread Michael Niedermayer
On Thu, Oct 12, 2017 at 12:35:35AM +0100, Mark Thompson wrote:
> On 11/10/17 23:34, Michael Niedermayer wrote:
> > Hi
> > 
> > On Mon, Oct 09, 2017 at 08:04:47PM +0100, Mark Thompson wrote:
> > [...]
> > 
> >> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> >> new file mode 100644
> >> index 00..e35175fc74
> >> --- /dev/null
> >> +++ b/libavcodec/cbs.h
> >> @@ -0,0 +1,283 @@
> >> +/*
> >> + * 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
> >> + */
> >> +
> >> +#ifndef AVCODEC_CBS_H
> >> +#define AVCODEC_CBS_H
> >> +
> >> +#include 
> >> +#include 
> >> +
> >> +#include "avcodec.h"
> >> +
> 
> (Here)
> 
> >> +
> >> +struct CodedBitstreamType;
> >> +
> > 
> >> +/**
> >> + * The codec-specific type of a bitstream unit.
> >> + */
> >> +typedef uint32_t CodedBitstreamUnitType;
> > 
> > Whats a bitstream unit ? (no iam not asking you to explain me but it
> > should be documeted)
> 
> See the comment ten lines below this one.
> 
> > Is there some high level overview of this API and its components ?
> > I mean without having to search terms in dispersed documentation.
> > 
> > The high level documentation should also explain what this API does
> > in a terse and clear way.
> > Maybe something like: (if thats correct)
> > Converts between a bit/byte stream and object tree representation of
> > a frame.
> > The (serialized) bitstream representation is what is commonly stored
> > in containers and passed around on networks. The object tree
> > representation allows easier modification of individual elements.
> 
> Inserted at "Here" above:
> 
> /*
>  * This defines a framework for converting between a coded bitstream
>  * and structures defining all individual syntax elements found in
>  * such a stream.
>  *
>  * Conversion in both directions is possible.  Given a coded bitstream
>  * (any meaningful fragment), it can be parsed and decomposed into
>  * syntax elements stored in a set of codec-specific structures.
>  * Similarly, given a set of those same codec-specific structures the
>  * syntax elements can be serialised and combined to create a coded
>  * bitstream.
>  */
> 

> >> +
> >> +/**
> >> + * Coded bitstream unit structure.
> >> + *
> > 
> >> + * A bitstream unit the smallest element of a bitstream which
> >> + * is meaningful on its own.  For example, an H.264 NAL unit.
> > 
> >> + *
> >> + * See the codec-specific header for the meaning of this for any
> >> + * particular codec.
> >> + */
> >> +typedef struct CodedBitstreamUnit {
> >> +/**
> >> + * Codec-specific type of this unit.
> >> + */
> >> +CodedBitstreamUnitType type;
> >> +
> > 
> >> +/**
> >> + * Pointer to the bitstream form of this unit.
> >> + *
> >> + * May be NULL if the unit currently only exists in decomposed form.
> >> + */
> >> +uint8_t *data;
> > 
> > whats a "bitstream form" ?
> > is that the bitstream itself? if so why is it called "bitstream form"
> 
> It is a bitstream form appropriate for direct parsing (for H.264/H.265, rbsp, 
> so unescaped).

Thats a bit surprising.

Wouldnt a API that uses the unmodified bitstream be more clear and
simpler ?
Or an explicit parameter that specifies if the input has been
unescaped already.


[...]

> >> +#endif /* AVCODEC_CBS_H */
> 
> (I can't help but feel that refining this internal documentation is somewhat 
> futile.  It isn't a public API, and will never be used by anyone who isn't 
> highly familiar with the concepts involved.)

I do disagree here

Everyone is initially unfamiliar with the concepts, API and
implementation.

All the various internal documentation (and code) is all there is
to get one from "unfamiliar" to "familiar" to "highly familiar"

(and the specifications of various standards which often dont shine
 in terms of clarity either)

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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


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


Re: [FFmpeg-devel] [PATCH 03/20] lavc: Add coded bitstream read/write API

2017-10-13 Thread Michael Niedermayer
On Thu, Oct 12, 2017 at 12:35:35AM +0100, Mark Thompson wrote:
> On 11/10/17 23:34, Michael Niedermayer wrote:
> > Hi
> > 
> > On Mon, Oct 09, 2017 at 08:04:47PM +0100, Mark Thompson wrote:
> > [...]
> > 
> >> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> >> new file mode 100644
> >> index 00..e35175fc74
> >> --- /dev/null
> >> +++ b/libavcodec/cbs.h
> >> @@ -0,0 +1,283 @@
> >> +/*
> >> + * 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
> >> + */
> >> +
> >> +#ifndef AVCODEC_CBS_H
> >> +#define AVCODEC_CBS_H
> >> +
> >> +#include 
> >> +#include 
> >> +
> >> +#include "avcodec.h"
> >> +
> 
> (Here)
> 
> >> +
> >> +struct CodedBitstreamType;
> >> +
> > 
> >> +/**
> >> + * The codec-specific type of a bitstream unit.
> >> + */
> >> +typedef uint32_t CodedBitstreamUnitType;
> > 
> > Whats a bitstream unit ? (no iam not asking you to explain me but it
> > should be documeted)
> 
> See the comment ten lines below this one.
> 
> > Is there some high level overview of this API and its components ?
> > I mean without having to search terms in dispersed documentation.
> > 
> > The high level documentation should also explain what this API does
> > in a terse and clear way.
> > Maybe something like: (if thats correct)
> > Converts between a bit/byte stream and object tree representation of
> > a frame.
> > The (serialized) bitstream representation is what is commonly stored
> > in containers and passed around on networks. The object tree
> > representation allows easier modification of individual elements.
> 
> Inserted at "Here" above:
> 
> /*
>  * This defines a framework for converting between a coded bitstream
>  * and structures defining all individual syntax elements found in
>  * such a stream.
>  *
>  * Conversion in both directions is possible.  Given a coded bitstream
>  * (any meaningful fragment), it can be parsed and decomposed into
>  * syntax elements stored in a set of codec-specific structures.
>  * Similarly, given a set of those same codec-specific structures the
>  * syntax elements can be serialised and combined to create a coded
>  * bitstream.
>  */

[...]

> >> +/**
> >> + * Read the data bitstream from a packet into a fragment, then
> >> + * split into units and decompose.
> >> + */
> >> +int ff_cbs_read_packet(CodedBitstreamContext *ctx,
> >> +   CodedBitstreamFragment *frag,
> >> +   const AVPacket *pkt);

maybe ff_cbs_decompose_packet() would be a better name ?

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

No snowflake in an avalanche ever feels responsible. -- Voltaire


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


Re: [FFmpeg-devel] [PATCH 03/20] lavc: Add coded bitstream read/write API

2017-10-13 Thread Michael Niedermayer
On Fri, Oct 13, 2017 at 01:23:34PM +0200, Michael Niedermayer wrote:
> On Thu, Oct 12, 2017 at 12:35:35AM +0100, Mark Thompson wrote:
> > On 11/10/17 23:34, Michael Niedermayer wrote:
[...]
> > >> +
> > >> +/**
> > >> + * Coded bitstream unit structure.
> > >> + *
> > > 
> > >> + * A bitstream unit the smallest element of a bitstream which
> > >> + * is meaningful on its own.  For example, an H.264 NAL unit.
> > > 
> > >> + *
> > >> + * See the codec-specific header for the meaning of this for any
> > >> + * particular codec.
> > >> + */
> > >> +typedef struct CodedBitstreamUnit {
> > >> +/**
> > >> + * Codec-specific type of this unit.
> > >> + */
> > >> +CodedBitstreamUnitType type;
> > >> +
> > > 
> > >> +/**
> > >> + * Pointer to the bitstream form of this unit.
> > >> + *
> > >> + * May be NULL if the unit currently only exists in decomposed form.
> > >> + */
> > >> +uint8_t *data;
> > > 
> > > whats a "bitstream form" ?
> > > is that the bitstream itself? if so why is it called "bitstream form"
> > 
> > It is a bitstream form appropriate for direct parsing (for H.264/H.265, 
> > rbsp, so unescaped).
> 
> Thats a bit surprising.
> 
> Wouldnt a API that uses the unmodified bitstream be more clear and
> simpler ?
> Or an explicit parameter that specifies if the input has been
> unescaped already.

disregard this, i realize now you just spoke about the field in the
struct and not the interface, which i presume works with normal
data as stored in containers


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


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


Re: [FFmpeg-devel] [PATCH 03/20] lavc: Add coded bitstream read/write API

2017-10-13 Thread Mark Thompson
On 13/10/17 12:31, Michael Niedermayer wrote:
> On Fri, Oct 13, 2017 at 01:23:34PM +0200, Michael Niedermayer wrote:
>> On Thu, Oct 12, 2017 at 12:35:35AM +0100, Mark Thompson wrote:
>>> On 11/10/17 23:34, Michael Niedermayer wrote:
> [...]
> +
> +/**
> + * Coded bitstream unit structure.
> + *

> + * A bitstream unit the smallest element of a bitstream which
> + * is meaningful on its own.  For example, an H.264 NAL unit.

> + *
> + * See the codec-specific header for the meaning of this for any
> + * particular codec.
> + */
> +typedef struct CodedBitstreamUnit {
> +/**
> + * Codec-specific type of this unit.
> + */
> +CodedBitstreamUnitType type;
> +

> +/**
> + * Pointer to the bitstream form of this unit.
> + *
> + * May be NULL if the unit currently only exists in decomposed form.
> + */
> +uint8_t *data;

 whats a "bitstream form" ?
 is that the bitstream itself? if so why is it called "bitstream form"
>>>
>>> It is a bitstream form appropriate for direct parsing (for H.264/H.265, 
>>> rbsp, so unescaped).
>>
>> Thats a bit surprising.
>>
>> Wouldnt a API that uses the unmodified bitstream be more clear and
>> simpler ?
>> Or an explicit parameter that specifies if the input has been
>> unescaped already.
> 
> disregard this, i realize now you just spoke about the field in the
> struct and not the interface, which i presume works with normal
> data as stored in containers

Correct.  This is the intermediate form between the internal split/read or 
write/assemble - a user of the API is not expected to touch it unless they are 
doing something very sophisticated.

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


Re: [FFmpeg-devel] [PATCH] avdevice/decklink_dec: Added SCTE104 message decode from VANC

2017-10-13 Thread Devin Heitmueller
Hello Vishwanath,

> On Oct 13, 2017, at 1:42 AM, Dixit, Vishwanath  wrote:
> 
> Hi,
> 
> Please find the attached patch which adds support to decode SCTE-104 message 
> from VANC for decklink capture use case.
> 

I’ve got a couple of concerns about this approach.

Because I am doing a bunch of VANC related work, I looked at various VANC types 
and assessed their suitability for the use of AV_FRAME_SIDE_DATA versus 
generating new streams.  While side data is well suited for content that will 
ultimately be fed into the encoder (e.g. EIA-708 captions, AFD, etc), there are 
significant limitations for using side data for content types that may need to 
result in new stream creation.  Examples of this include SCTE-104 (which in 
many use cases will result in creation of an SCTE-35 stream), SMPTE 2038 (which 
again creates a corresponding elementary stream), and Teletext (which you see 
in the implementation today), etc.

Further, there are significant limitations in the ability to access side data 
from within a pipeline.  If tied to the video frame as side data, the only way 
to access it if not the target encoder codec is to use a video filter.  Video 
filters cannot create new streams, and hence creating pipelines which make use 
of SCTE-104 packets can be very difficult.

What you’ve submitted seems like half an actual use case - you’re ingesting the 
SCTE-104 packet and inserting it into as frame side data, but what are you 
actually planning to do with the data?  Do your use cases involve 
transformation to SCTE-35?  Do you care about the decklink input exclusively, 
or do you need the decklink output working as well?

I’ve got patches queueing which implement SCTE-104 and SCTE-35 for the 
following use cases:

- Ingest SCTE-104 on the decklink input, and create an SCTE-35 stream in the TS 
(including proper timing)
- Decode a TS containing SCTE-35 and output the resulting data on the decklink 
output as SCTE-104
- Be able to ingest SCTE-104 from network port (either TCP or UDP) and output 
the data into the decklink output (i.e. a SCTE-104 inserter)
- Be able to ingest SCTE-104 from a network port and insert the data into a TS 
as SCTE-35

The above cases require the flexibility to access the data from within the 
general pipeline as a stream, as opposed to the data being tied to the video 
frame as side-data.

It may be beneficial for the two of us to talk about use cases so we can avoid 
duplicated effort.  I already have patches which incorporate our mature VANC 
processing library for both the decklink input and output interfaces, and I’ve 
got trees which already implement EIA-708, AFD, SCTE-104, SCTE-35, and SMPTE 
2038.  I’ve been trickling the patches on the ML and the speed at which I’ve 
been providing them has been driven by how quickly they are reviewed and 
merged, not whether the work is already completed.

For your reference it may be worthwhile for you to look at our “libklvanc” 
library, which we are actively deploying in OBE and VLC and which already 
implements much of the stuff you’re in the process of writing from scratch.  My 
plan is to work on adding the glue in ffmpeg to leverage this library as well, 
given it’s code which is already in active use elsewhere and that work can be 
shared across multiple open source projects.

https://github.com/stoth68000/libklvanc 


I’m tied up working on multi-channel audio right now but I think there is 
definitely value in us syncing up to avoid duplicated effort and conflicting 
patches.  Let’s see where we can divide/conquer rather than re-implementing 
work the other may have already done.

Cheers,

Devin Heitmueller
LTN Global Communications


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


Re: [FFmpeg-devel] [PATCH 03/20] lavc: Add coded bitstream read/write API

2017-10-13 Thread Mark Thompson
On 13/10/17 12:28, Michael Niedermayer wrote:
> On Thu, Oct 12, 2017 at 12:35:35AM +0100, Mark Thompson wrote:
>> On 11/10/17 23:34, Michael Niedermayer wrote:
>>> Hi
>>>
>>> On Mon, Oct 09, 2017 at 08:04:47PM +0100, Mark Thompson wrote:
>>> [...]
>>>
 diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
 new file mode 100644
 index 00..e35175fc74
 --- /dev/null
 +++ b/libavcodec/cbs.h
 @@ -0,0 +1,283 @@
 +/*
 + * 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
 + */
 +
 +#ifndef AVCODEC_CBS_H
 +#define AVCODEC_CBS_H
 +
 +#include 
 +#include 
 +
 +#include "avcodec.h"
 +
>>
>> (Here)
>>
 +
 +struct CodedBitstreamType;
 +
>>>
 +/**
 + * The codec-specific type of a bitstream unit.
 + */
 +typedef uint32_t CodedBitstreamUnitType;
>>>
>>> Whats a bitstream unit ? (no iam not asking you to explain me but it
>>> should be documeted)
>>
>> See the comment ten lines below this one.
>>
>>> Is there some high level overview of this API and its components ?
>>> I mean without having to search terms in dispersed documentation.
>>>
>>> The high level documentation should also explain what this API does
>>> in a terse and clear way.
>>> Maybe something like: (if thats correct)
>>> Converts between a bit/byte stream and object tree representation of
>>> a frame.
>>> The (serialized) bitstream representation is what is commonly stored
>>> in containers and passed around on networks. The object tree
>>> representation allows easier modification of individual elements.
>>
>> Inserted at "Here" above:
>>
>> /*
>>  * This defines a framework for converting between a coded bitstream
>>  * and structures defining all individual syntax elements found in
>>  * such a stream.
>>  *
>>  * Conversion in both directions is possible.  Given a coded bitstream
>>  * (any meaningful fragment), it can be parsed and decomposed into
>>  * syntax elements stored in a set of codec-specific structures.
>>  * Similarly, given a set of those same codec-specific structures the
>>  * syntax elements can be serialised and combined to create a coded
>>  * bitstream.
>>  */
> 
> [...]
> 
 +/**
 + * Read the data bitstream from a packet into a fragment, then
 + * split into units and decompose.
 + */
 +int ff_cbs_read_packet(CodedBitstreamContext *ctx,
 +   CodedBitstreamFragment *frag,
 +   const AVPacket *pkt);
> 
> maybe ff_cbs_decompose_packet() would be a better name ?

It's more than decomposition, though, because it also reads the metadata to 
update the coded bitstream context state (e.g. saves parameter sets to be able 
to read subsequent packets).

Added to comment (as with the extradata one):

 * This also updates the internal state of the coded bitstream context
 * with any persistent data from the fragment which may be required to
 * read following fragments (e.g. parameter sets).

and similarly on write.

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


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread James Almer
On 10/12/2017 6:30 PM, James Almer wrote:
> Should prevent some options from being added to cflags when they
> don't exist and the compiler only warns about it.
> 
> Signed-off-by: James Almer 
> ---
> I figure this is safer than adding
> -Werror=unused-command-line-argument -Werror=unknown-warning-option
> as Ronald suggested.
> 
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index ade67a31bb..c7962665f1 100755
> --- a/configure
> +++ b/configure
> @@ -6370,7 +6370,7 @@ fi
>  
>  check_disable_warning(){
>  warning_flag=-W${1#-Wno-}
> -test_cflags $warning_flag && add_cflags $1
> +test_cflags -Werror $warning_flag && add_cflags $1
>  }
>  
>  check_disable_warning -Wno-parentheses

Ping. This or a similar solution has been annoying Clang users for some
days now and should be part of the 3.4 release.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread Hendrik Leppkes
On Fri, Oct 13, 2017 at 4:14 PM, James Almer  wrote:
> On 10/12/2017 6:30 PM, James Almer wrote:
>> Should prevent some options from being added to cflags when they
>> don't exist and the compiler only warns about it.
>>
>> Signed-off-by: James Almer 
>> ---
>> I figure this is safer than adding
>> -Werror=unused-command-line-argument -Werror=unknown-warning-option
>> as Ronald suggested.
>>
>>  configure | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index ade67a31bb..c7962665f1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -6370,7 +6370,7 @@ fi
>>
>>  check_disable_warning(){
>>  warning_flag=-W${1#-Wno-}
>> -test_cflags $warning_flag && add_cflags $1
>> +test_cflags -Werror $warning_flag && add_cflags $1
>>  }
>>
>>  check_disable_warning -Wno-parentheses
>
> Ping. This or a similar solution has been annoying Clang users for some
> days now and should be part of the 3.4 release.

I wonder if a general -Werror is really "safer", do these tests really
execute without any other warnings, which might trigger a false
negative? We're not exactly making it compile a proper source file, so
it might warn about random things.

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


Re: [FFmpeg-devel] [PATCH] libavformat/wtvdec: return AVERROR_EOF on EOF

2017-10-13 Thread Daniel Kučera
bump.

2017-06-15 10:22 GMT+02:00 Nicolas George :
> Le septidi 17 prairial, an CCXXV, Daniel Kucera a écrit :
>> Signed-off-by: Daniel Kucera 
>> ---
>>  libavformat/wtvdec.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
>> index 3ac4501306..ee19fd84da 100644
>> --- a/libavformat/wtvdec.c
>> +++ b/libavformat/wtvdec.c
>> @@ -65,7 +65,7 @@ static int64_t seek_by_sector(AVIOContext *pb, int64_t 
>> sector, int64_t offset)
>>  }
>>
>>  /**
>> - * @return bytes read, 0 on end of file, or <0 on error
>> + * @return bytes read, AVERROR_EOF on end of file, or <0 on error
>>   */
>>  static int wtvfile_read_packet(void *opaque, uint8_t *buf, int buf_size)
>>  {
>> @@ -76,7 +76,7 @@ static int wtvfile_read_packet(void *opaque, uint8_t *buf, 
>> int buf_size)
>>  if (wf->error || pb->error)
>>  return -1;
>>  if (wf->position >= wf->length || avio_feof(pb))
>> -return 0;
>> +return AVERROR_EOF;
>>
>>  buf_size = FFMIN(buf_size, wf->length - wf->position);
>>  while(nread < buf_size) {
>> --
>> 2.11.0
>
> LGTM, but I do not maintain that file.
>
> Regards,
>
> --
>   Nicolas George



-- 

S pozdravom / Best regards
Daniel Kucera.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavformat/wtvdec: return AVERROR_EOF on EOF

2017-10-13 Thread wm4
On Fri, 13 Oct 2017 16:46:31 +0200
Daniel Kučera  wrote:

> bump.

LGTM too. The maintainer is relatively inactive, and it's a small fix
that doesn't even require the maintainer's approval, so I pushed the
patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread wm4
On Fri, 13 Oct 2017 16:30:59 +0200
Hendrik Leppkes  wrote:

> On Fri, Oct 13, 2017 at 4:14 PM, James Almer  wrote:
> > On 10/12/2017 6:30 PM, James Almer wrote:  
> >> Should prevent some options from being added to cflags when they
> >> don't exist and the compiler only warns about it.
> >>
> >> Signed-off-by: James Almer 
> >> ---
> >> I figure this is safer than adding
> >> -Werror=unused-command-line-argument -Werror=unknown-warning-option
> >> as Ronald suggested.
> >>
> >>  configure | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/configure b/configure
> >> index ade67a31bb..c7962665f1 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -6370,7 +6370,7 @@ fi
> >>
> >>  check_disable_warning(){
> >>  warning_flag=-W${1#-Wno-}
> >> -test_cflags $warning_flag && add_cflags $1
> >> +test_cflags -Werror $warning_flag && add_cflags $1
> >>  }
> >>
> >>  check_disable_warning -Wno-parentheses  
> >
> > Ping. This or a similar solution has been annoying Clang users for some
> > days now and should be part of the 3.4 release.  
> 
> I wonder if a general -Werror is really "safer", do these tests really
> execute without any other warnings, which might trigger a false
> negative? We're not exactly making it compile a proper source file, so
> it might warn about random things.

The worst case is that warnings are not disabled, so the risk isn't too
high.

If you really wanted to, you could run the check _without_ any
arguments, to see whether this type of checking works. And if that
fails, dunno, explode?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread James Almer
On 10/13/2017 11:30 AM, Hendrik Leppkes wrote:
> On Fri, Oct 13, 2017 at 4:14 PM, James Almer  wrote:
>> On 10/12/2017 6:30 PM, James Almer wrote:
>>> Should prevent some options from being added to cflags when they
>>> don't exist and the compiler only warns about it.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>> I figure this is safer than adding
>>> -Werror=unused-command-line-argument -Werror=unknown-warning-option
>>> as Ronald suggested.
>>>
>>>  configure | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index ade67a31bb..c7962665f1 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -6370,7 +6370,7 @@ fi
>>>
>>>  check_disable_warning(){
>>>  warning_flag=-W${1#-Wno-}
>>> -test_cflags $warning_flag && add_cflags $1
>>> +test_cflags -Werror $warning_flag && add_cflags $1
>>>  }
>>>
>>>  check_disable_warning -Wno-parentheses
>>
>> Ping. This or a similar solution has been annoying Clang users for some
>> days now and should be part of the 3.4 release.
> 
> I wonder if a general -Werror is really "safer", do these tests really
> execute without any other warnings, which might trigger a false
> negative? We're not exactly making it compile a proper source file, so
> it might warn about random things.
> 
> - Hendrik

Ok, what about the attached patch, then?

Both -Werror=unused-command-line-argument and
-Werror=unknown-warning-option are not supported by gcc, so they
generate an error and would break every check.
This hopefully only uses them where they exist.
From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001
From: James Almer 
Date: Fri, 13 Oct 2017 12:34:34 -0300
Subject: [PATCH] configure: force erroring out in check_disable_warning() if
 an option doesn't exists

Should prevent some options from being added to cflags when they
don't exist and the compiler only warns about it.

Signed-off-by: James Almer 
---
 configure | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 64fd088208..87087f02e4 100755
--- a/configure
+++ b/configure
@@ -6370,9 +6370,14 @@ fi
 
 check_disable_warning(){
 warning_flag=-W${1#-Wno-}
-test_cflags $warning_flag && add_cflags $1
+test_cflags $unknown_warning_flags $warning_flag && add_cflags $1
 }
 
+test_cflags -Werror=unused-command-line-argument &&
+append unknown_warning_flags "-Werror=unused-command-line-argument"
+test_cflags -Werror=unknown-warning-option &&
+append unknown_warning_flags "-Werror=unknown-warning-option"
+
 check_disable_warning -Wno-parentheses
 check_disable_warning -Wno-switch
 check_disable_warning -Wno-format-zero-length
-- 
2.14.2

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


Re: [FFmpeg-devel] [FFmpeg-cvslog] Merge commit '7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63'

2017-10-13 Thread Reino Wijnsma
On 13-10-2017 0:27, James Almer  wrote:
> On 10/12/2017 6:42 PM, Reino Wijnsma wrote:
>> On 12-10-2017 20:11, Helmut K. C. Tessarek  wrote:
>>> I'm not sure why you guys have to make all these changes to the build
>>> process lately.
>>>
>>> All of a sudden ffmpeg does not compile anymore. During configure I get:
>>>
>>> ERROR: libbluray not found using pkg-config
>>>
>>> which worked perfectly 2 days ago. So, whatever changes you made, they
>>> screwed up other things.
>> Same here. I was about to compile another FFmpeg executable, but got
>> configure complaining about libbluray as well (note: I always use
>> --pkg-config-flags=--static).
>> I've worked around that by changing the pc-file
>>
>> prefix=/cygdrive/[...]
>> exec_prefix=${prefix}
>> libdir=${exec_prefix}/lib
>> includedir=${prefix}/include
>>
>> Name: libbluray
>> Description: library supporting Blu-ray playback
>> Version: 1.0.1
>> Libs: -L${libdir} -lbluray
>> Libs.private:  -L/cygdrive/[...]/lib -lxml2 -L/cygdrive/[...]/lib
>> -lfreetype -lgdi32
>> Cflags: -I${includedir}
>>
>> ...into:
>>
>> prefix=/cygdrive/[...]
>> exec_prefix=${prefix}
>> libdir=${exec_prefix}/lib
>> includedir=${prefix}/include
>>
>> Name: libbluray
>> Description: library supporting Blu-ray playback
>> Version: 1.0.1
>> Requires: libxml-2.0, freetype2
>> Libs: -L${libdir} -lbluray
>> Libs.private:  -lgdi32
>> Cflags: -I${includedir}
> Thanks a lot! You confirmed what i suspected.
>
> Guess we'll have to get libbluray to change their .pc file.
>
>> Next however was iLBC. I got "undefined reference to `pthread_once'" and
>> "ERROR: libilbc not found". I had to add --extra-libs=-lpthread to 'fix'
>> that.
>>
>> Now it's complaining about libmysofa:
>>
>> /cygdrive/[...]/lib/libmysofa.a(gunzip.c.obj):gunzip.c:(.text+0x5f):
>> undefined reference to `inflateInit_'
>> /cygdrive/[...]/lib/libmysofa.a(gunzip.c.obj):gunzip.c:(.text+0x76):
>> undefined reference to `inflate'
>> /cygdrive/[...]/lib/libmysofa.a(gunzip.c.obj):gunzip.c:(.text+0x8a):
>> undefined reference to `inflateEnd'
>> collect2: error: ld returned 1 exit status
>> ERROR: libmysofa not found
> Does libmysofa have a pkg-config file? Otherwise, guess i'll have to add
> -lz to its extralibs.
No, it does not. However,...

enabled libmysofa && require libmysofa "mysofa.h" mysofa_load
-lmysofa -lz

appending "-lz" here fixes the issue for me.


Next in line is libopenmpt. A LONG list of error-messages, saved and
attached as '/config_libopenmpt.log/'.
Also notice the warning on line 19 and the GCC 7.2.0
undefined-reference-messages on line 134 and 135 for instance.
My '/libopenmpt.pc/' looks like:

prefix=/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32
exec_prefix=${prefix}
includedir=${prefix}/include
libdir=${exec_prefix}/lib

Name: libopenmpt
Description: Tracker module player based on OpenMPT
Version: 0.3.1+release.autotools
Requires.private: zlib libmpg123 ogg vorbis vorbisfile
Libs: -L${libdir} -lopenmpt
Libs.private:
Cflags: -I${includedir}

-- Reino
[...]
require_pkg_config libopenmpt libopenmpt >= 0.2.6557 libopenmpt/libopenmpt.h 
openmpt_module_create
use_pkg_config libopenmpt libopenmpt >= 0.2.6557 libopenmpt/libopenmpt.h 
openmpt_module_create
check_pkg_config libopenmpt libopenmpt >= 0.2.6557 libopenmpt/libopenmpt.h 
openmpt_module_create
pkg-config --exists --print-errors libopenmpt >= 0.2.6557
check_func_headers libopenmpt/libopenmpt.h openmpt_module_create 
-I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
-L/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/lib -lopenmpt 
-lz -lmpg123 -lvorbisfile -lvorbis -lm -logg
check_ld cc 
-I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
-L/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/lib -lopenmpt 
-lz -lmpg123 -lvorbisfile -lvorbis -lm -logg
check_cc 
-I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
-L/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/lib
BEGIN /cygdrive/c/DOCUME~1/Admin/LOCALS~1/Temp/ffconf.f7SNCo4C/test.c
1   #include 
2   #include 
3   long check_openmpt_module_create(void) { return (long) 
openmpt_module_create; }
4   int main(void) { int ret = 0;
5ret |= ((intptr_t)check_openmpt_module_create) & 0x;
6   return ret; }
END /cygdrive/c/DOCUME~1/Admin/LOCALS~1/Temp/ffconf.f7SNCo4C/test.c
/cygdrive/[...]/cross_compilers/mingw-w64-i686/bin/i686-w64-mingw32-gcc 
-D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U__STRICT_ANSI__ 
-D__USE_MINGW_ANSI_STDIO=1 -D__printf__=__gnu_printf__ -D_POSIX_C_SOURCE=200112 
-D_XOPEN_SOURCE=600 -march=pentium3 -O2 -mfpmath=sse -msse -DCACA_STATIC 
-DLIBTWOLAME_STATIC -march=pentium3 -O2 -mfpmath=sse -msse -std=c11 
-fomit-frame-pointer -pthread 
-I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
-DLIBXML_STATIC 
-I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
-I/cygdrive/[...]

Re: [FFmpeg-devel] [FFmpeg-cvslog] Merge commit '7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63'

2017-10-13 Thread James Almer
On 10/13/2017 1:05 PM, Reino Wijnsma wrote:
> On 13-10-2017 0:27, James Almer  wrote:
>> On 10/12/2017 6:42 PM, Reino Wijnsma wrote:
>>> On 12-10-2017 20:11, Helmut K. C. Tessarek  wrote:
 I'm not sure why you guys have to make all these changes to the build
 process lately.

 All of a sudden ffmpeg does not compile anymore. During configure I get:

 ERROR: libbluray not found using pkg-config

 which worked perfectly 2 days ago. So, whatever changes you made, they
 screwed up other things.
>>> Same here. I was about to compile another FFmpeg executable, but got
>>> configure complaining about libbluray as well (note: I always use
>>> --pkg-config-flags=--static).
>>> I've worked around that by changing the pc-file
>>>
>>> prefix=/cygdrive/[...]
>>> exec_prefix=${prefix}
>>> libdir=${exec_prefix}/lib
>>> includedir=${prefix}/include
>>>
>>> Name: libbluray
>>> Description: library supporting Blu-ray playback
>>> Version: 1.0.1
>>> Libs: -L${libdir} -lbluray
>>> Libs.private:  -L/cygdrive/[...]/lib -lxml2 -L/cygdrive/[...]/lib
>>> -lfreetype -lgdi32
>>> Cflags: -I${includedir}
>>>
>>> ...into:
>>>
>>> prefix=/cygdrive/[...]
>>> exec_prefix=${prefix}
>>> libdir=${exec_prefix}/lib
>>> includedir=${prefix}/include
>>>
>>> Name: libbluray
>>> Description: library supporting Blu-ray playback
>>> Version: 1.0.1
>>> Requires: libxml-2.0, freetype2
>>> Libs: -L${libdir} -lbluray
>>> Libs.private:  -lgdi32
>>> Cflags: -I${includedir}
>> Thanks a lot! You confirmed what i suspected.
>>
>> Guess we'll have to get libbluray to change their .pc file.
>>
>>> Next however was iLBC. I got "undefined reference to `pthread_once'" and
>>> "ERROR: libilbc not found". I had to add --extra-libs=-lpthread to 'fix'
>>> that.
>>>
>>> Now it's complaining about libmysofa:
>>>
>>> /cygdrive/[...]/lib/libmysofa.a(gunzip.c.obj):gunzip.c:(.text+0x5f):
>>> undefined reference to `inflateInit_'
>>> /cygdrive/[...]/lib/libmysofa.a(gunzip.c.obj):gunzip.c:(.text+0x76):
>>> undefined reference to `inflate'
>>> /cygdrive/[...]/lib/libmysofa.a(gunzip.c.obj):gunzip.c:(.text+0x8a):
>>> undefined reference to `inflateEnd'
>>> collect2: error: ld returned 1 exit status
>>> ERROR: libmysofa not found
>> Does libmysofa have a pkg-config file? Otherwise, guess i'll have to add
>> -lz to its extralibs.
> No, it does not. However,...
> 
> enabled libmysofa && require libmysofa "mysofa.h" mysofa_load
> -lmysofa -lz
> 
> appending "-lz" here fixes the issue for me.

Yes, i fixed it this way earlier today.

> 
> 
> Next in line is libopenmpt. A LONG list of error-messages, saved and
> attached as '/config_libopenmpt.log/'.
> Also notice the warning on line 19 and the GCC 7.2.0
> undefined-reference-messages on line 134 and 135 for instance.
> My '/libopenmpt.pc/' looks like:
> 
> prefix=/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32
> exec_prefix=${prefix}
> includedir=${prefix}/include
> libdir=${exec_prefix}/lib
> 
> Name: libopenmpt
> Description: Tracker module player based on OpenMPT
> Version: 0.3.1+release.autotools
> Requires.private: zlib libmpg123 ogg vorbis vorbisfile
> Libs: -L${libdir} -lopenmpt
> Libs.private:
> Cflags: -I${includedir}
> 
> -- Reino
> 
> 
> config_libopenmpt.log
> 
> 
> [...]
> require_pkg_config libopenmpt libopenmpt >= 0.2.6557 libopenmpt/libopenmpt.h 
> openmpt_module_create
> use_pkg_config libopenmpt libopenmpt >= 0.2.6557 libopenmpt/libopenmpt.h 
> openmpt_module_create
> check_pkg_config libopenmpt libopenmpt >= 0.2.6557 libopenmpt/libopenmpt.h 
> openmpt_module_create
> pkg-config --exists --print-errors libopenmpt >= 0.2.6557
> check_func_headers libopenmpt/libopenmpt.h openmpt_module_create 
> -I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
> -L/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/lib 
> -lopenmpt -lz -lmpg123 -lvorbisfile -lvorbis -lm -logg
> check_ld cc 
> -I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
> -L/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/lib 
> -lopenmpt -lz -lmpg123 -lvorbisfile -lvorbis -lm -logg
> check_cc 
> -I/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/include 
> -L/cygdrive/[...]/cross_compilers/mingw-w64-i686/i686-w64-mingw32/lib
> BEGIN /cygdrive/c/DOCUME~1/Admin/LOCALS~1/Temp/ffconf.f7SNCo4C/test.c
> 1 #include 
> 2 #include 
> 3 long check_openmpt_module_create(void) { return (long) 
> openmpt_module_create; }
> 4 int main(void) { int ret = 0;
> 5  ret |= ((intptr_t)check_openmpt_module_create) & 0x;
> 6 return ret; }
> END /cygdrive/c/DOCUME~1/Admin/LOCALS~1/Temp/ffconf.f7SNCo4C/test.c
> /cygdrive/[...]/cross_compilers/mingw-w64-i686/bin/i686-w64-mingw32-gcc
> -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
> -U__STRICT_ANSI__ -D__USE_MINGW_ANSI_STDIO=1 -D__printf__=__gnu_printf__
> -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -march=pentium3 -O2
> -mfpmath=sse -msse -DCAC

[FFmpeg-devel] [PATCH 0/5] Cuvid/videotoolbox preparations

2017-10-13 Thread wm4
These commits are required to merge Libav's cuvid hwaccel, and to
fix videotoolbox operation if frame threading is enabled.

Anton Khirnov (4):
  decode: avoid leaks on failure in ff_get_buffer()
  decode: add a method for attaching lavc-internal data to frames
  decode: add a mechanism for performing delayed processing on the
decoded frames
  decode: add a per-frame private data for hwaccel use

wm4 (1):
  lavc/avrndec: remove AV_CODEC_CAP_DR1, as it's broken

 libavcodec/avrndec.c |   1 -
 libavcodec/decode.c  | 113 ++-
 libavcodec/decode.h  |  40 +++
 libavcodec/h264dec.c |   5 +-
 libavcodec/huffyuvdec.c  |   3 +-
 libavcodec/mpegutils.c   |   4 +-
 libavcodec/vp3.c |   3 +-
 libavcodec/wrapped_avframe.c |   7 +++
 8 files changed, 167 insertions(+), 9 deletions(-)

-- 
2.14.1

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


[FFmpeg-devel] [PATCH 3/5] decode: add a method for attaching lavc-internal data to frames

2017-10-13 Thread wm4
From: Anton Khirnov 

Use the AVFrame.opaque_ref field. The original user's opaque_ref is
wrapped in the lavc struct and then unwrapped before the frame is
returned to the caller.

This new struct will be useful in the following commits.

Merges Libav commit 359a8a3e2d1194b52b6c386f94fd0929567dfb67.

This adds proper handling of draw_horiz_band over Libav. Also, the
wrapped_avframe decoder is adjusted to return the wrapped frame's
opaque_ref field to the user (which may or may not be what the user
expects).
---
 libavcodec/decode.c  | 96 +++-
 libavcodec/decode.h  | 19 +
 libavcodec/h264dec.c |  5 ++-
 libavcodec/huffyuvdec.c  |  3 +-
 libavcodec/mpegutils.c   |  4 +-
 libavcodec/vp3.c |  3 +-
 libavcodec/wrapped_avframe.c |  7 
 7 files changed, 129 insertions(+), 8 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 437b848248..9395cfc43b 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -640,6 +640,26 @@ static int decode_receive_frame_internal(AVCodecContext 
*avctx, AVFrame *frame)
 if (ret == AVERROR_EOF)
 avci->draining_done = 1;
 
+/* unwrap the per-frame decode data and restore the original opaque_ref*/
+if (!ret) {
+/* the only case where decode data is not set should be decoders
+ * that do not call ff_get_buffer() */
+av_assert0((frame->opaque_ref && frame->opaque_ref->size == 
sizeof(FrameDecodeData)) ||
+   !(avctx->codec->capabilities & AV_CODEC_CAP_DR1));
+
+if (frame->opaque_ref) {
+FrameDecodeData *fdd;
+AVBufferRef *user_opaque_ref;
+
+fdd = (FrameDecodeData*)frame->opaque_ref->data;
+
+user_opaque_ref = fdd->user_opaque_ref;
+fdd->user_opaque_ref = NULL;
+av_buffer_unref(&frame->opaque_ref);
+frame->opaque_ref = user_opaque_ref;
+}
+}
+
 return ret;
 }
 
@@ -1612,6 +1632,37 @@ static void validate_avframe_allocation(AVCodecContext 
*avctx, AVFrame *frame)
 }
 }
 
+static void decode_data_free(void *opaque, uint8_t *data)
+{
+FrameDecodeData *fdd = (FrameDecodeData*)data;
+
+av_buffer_unref(&fdd->user_opaque_ref);
+
+av_freep(&fdd);
+}
+
+int ff_attach_decode_data(AVFrame *frame)
+{
+AVBufferRef *fdd_buf;
+FrameDecodeData *fdd;
+
+fdd = av_mallocz(sizeof(*fdd));
+if (!fdd)
+return AVERROR(ENOMEM);
+
+fdd_buf = av_buffer_create((uint8_t*)fdd, sizeof(*fdd), decode_data_free,
+   NULL, AV_BUFFER_FLAG_READONLY);
+if (!fdd_buf) {
+av_freep(&fdd);
+return AVERROR(ENOMEM);
+}
+
+fdd->user_opaque_ref = frame->opaque_ref;
+frame->opaque_ref= fdd_buf;
+
+return 0;
+}
+
 static int get_buffer_internal(AVCodecContext *avctx, AVFrame *frame, int 
flags)
 {
 const AVHWAccel *hwaccel = avctx->hwaccel;
@@ -1648,8 +1699,14 @@ static int get_buffer_internal(AVCodecContext *avctx, 
AVFrame *frame, int flags)
 avctx->sw_pix_fmt = avctx->pix_fmt;
 
 ret = avctx->get_buffer2(avctx, frame, flags);
-if (ret >= 0)
-validate_avframe_allocation(avctx, frame);
+if (ret < 0)
+goto end;
+
+validate_avframe_allocation(avctx, frame);
+
+ret = ff_attach_decode_data(frame);
+if (ret < 0)
+goto end;
 
 end:
 if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions &&
@@ -1757,3 +1814,38 @@ void ff_decode_bsfs_uninit(AVCodecContext *avctx)
 av_freep(&s->bsfs);
 s->nb_bsfs = 0;
 }
+
+void ff_call_draw_horiz_band(struct AVCodecContext *s,
+ const AVFrame *src, int 
offset[AV_NUM_DATA_POINTERS],
+ int y, int type, int height)
+{
+AVFrame *user_frame;
+
+if (!s->draw_horiz_band)
+return;
+
+user_frame = av_frame_clone(src);
+if (!user_frame) {
+av_log(s, AV_LOG_ERROR, "draw_horiz_band() failed\n");
+goto done;
+}
+
+av_buffer_unref(&user_frame->opaque_ref);
+
+if (src->opaque_ref) {
+FrameDecodeData *fdd = (FrameDecodeData*)src->opaque_ref->data;
+
+if (fdd->user_opaque_ref) {
+user_frame->opaque_ref = av_buffer_ref(fdd->user_opaque_ref);
+if (!user_frame->opaque_ref) {
+av_log(s, AV_LOG_ERROR, "draw_horiz_band() failed\n");
+goto done;
+}
+}
+}
+
+s->draw_horiz_band(s, user_frame, offset, y, type, height);
+
+done:
+av_frame_unref(user_frame);
+}
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index c9630228dc..9326f1d952 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -21,8 +21,21 @@
 #ifndef AVCODEC_DECODE_H
 #define AVCODEC_DECODE_H
 
+#include "libavutil/buffer.h"
+
 #include "avcodec.h"
 
+/**
+ * This struct stores per-frame lavc-internal data and is attached to it via
+ * opaque_ref.
+ */
+typedef struct F

[FFmpeg-devel] [PATCH 1/5] lavc/avrndec: remove AV_CODEC_CAP_DR1, as it's broken

2017-10-13 Thread wm4
In the is_mjpeg case, the user's get_buffer2 callback is not called,
thus completely breaking the API.
---
 libavcodec/avrndec.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libavcodec/avrndec.c b/libavcodec/avrndec.c
index c37f99661b..104ff2d904 100644
--- a/libavcodec/avrndec.c
+++ b/libavcodec/avrndec.c
@@ -168,7 +168,6 @@ AVCodec ff_avrn_decoder = {
 .init   = init,
 .close  = end,
 .decode = decode_frame,
-.capabilities   = AV_CODEC_CAP_DR1,
 .max_lowres = 3,
 .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };
-- 
2.14.1

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


[FFmpeg-devel] [PATCH 2/5] decode: avoid leaks on failure in ff_get_buffer()

2017-10-13 Thread wm4
From: Anton Khirnov 

If the get_buffer() call fails, the frame might have some side data
already set. Make sure it gets freed.

CC: libav-sta...@libav.org

Merges Libav commit de77671438c24ffea93398c8dc885d4dd04477de.
---
 libavcodec/decode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 1337ffb527..437b848248 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1658,6 +1658,9 @@ end:
 frame->height = avctx->height;
 }
 
+if (ret < 0)
+av_frame_unref(frame);
+
 return ret;
 }
 
-- 
2.14.1

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


[FFmpeg-devel] [PATCH 4/5] decode: add a mechanism for performing delayed processing on the decoded frames

2017-10-13 Thread wm4
From: Anton Khirnov 

This will be useful in the CUVID hwaccel.

Merges Libav commit badf0951f54c1332e77455dc40398f3512540c1b.
---
 libavcodec/decode.c | 11 +++
 libavcodec/decode.h | 15 +++
 2 files changed, 26 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 9395cfc43b..9878950c82 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -653,6 +653,14 @@ static int decode_receive_frame_internal(AVCodecContext 
*avctx, AVFrame *frame)
 
 fdd = (FrameDecodeData*)frame->opaque_ref->data;
 
+if (fdd->post_process) {
+ret = fdd->post_process(avctx, frame);
+if (ret < 0) {
+av_frame_unref(frame);
+return ret;
+}
+}
+
 user_opaque_ref = fdd->user_opaque_ref;
 fdd->user_opaque_ref = NULL;
 av_buffer_unref(&frame->opaque_ref);
@@ -1638,6 +1646,9 @@ static void decode_data_free(void *opaque, uint8_t *data)
 
 av_buffer_unref(&fdd->user_opaque_ref);
 
+if (fdd->post_process_opaque_free)
+fdd->post_process_opaque_free(fdd->post_process_opaque);
+
 av_freep(&fdd);
 }
 
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 9326f1d952..dcfc830296 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -22,6 +22,7 @@
 #define AVCODEC_DECODE_H
 
 #include "libavutil/buffer.h"
+#include "libavutil/frame.h"
 
 #include "avcodec.h"
 
@@ -34,6 +35,20 @@ typedef struct FrameDecodeData {
  * The original user-set opaque_ref.
  */
 AVBufferRef *user_opaque_ref;
+
+/**
+ * The callback to perform some delayed processing on the frame right
+ * before it is returned to the caller.
+ *
+ * @note This code is called at some unspecified point after the frame is
+ * returned from the decoder's decode/receive_frame call. Therefore it 
cannot rely
+ * on AVCodecContext being in any specific state, so it does not get to
+ * access AVCodecContext directly at all. All the state it needs must be
+ * stored in the post_process_opaque object.
+ */
+int (*post_process)(void *logctx, AVFrame *frame);
+void *post_process_opaque;
+void (*post_process_opaque_free)(void *opaque);
 } FrameDecodeData;
 
 /**
-- 
2.14.1

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


[FFmpeg-devel] [PATCH 5/5] decode: add a per-frame private data for hwaccel use

2017-10-13 Thread wm4
From: Anton Khirnov 

This will be useful in the CUVID hwaccel. It should also eventually
replace current decoder-specific mechanisms used by various other
hwaccels.

Merges Libav commit 704311b2946d74a80f65906961cd9baaa18683a3.
---
 libavcodec/decode.c | 3 +++
 libavcodec/decode.h | 6 ++
 2 files changed, 9 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 9878950c82..5e8660e091 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1649,6 +1649,9 @@ static void decode_data_free(void *opaque, uint8_t *data)
 if (fdd->post_process_opaque_free)
 fdd->post_process_opaque_free(fdd->post_process_opaque);
 
+if (fdd->hwaccel_priv_free)
+fdd->hwaccel_priv_free(fdd->hwaccel_priv);
+
 av_freep(&fdd);
 }
 
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index dcfc830296..5d532c615d 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -49,6 +49,12 @@ typedef struct FrameDecodeData {
 int (*post_process)(void *logctx, AVFrame *frame);
 void *post_process_opaque;
 void (*post_process_opaque_free)(void *opaque);
+
+/**
+ * Per-frame private data for hwaccels.
+ */
+void *hwaccel_priv;
+void (*hwaccel_priv_free)(void *priv);
 } FrameDecodeData;
 
 /**
-- 
2.14.1

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


Re: [FFmpeg-devel] [PATCH 2/7] decode: add a method for attaching lavc-internal data to frames

2017-10-13 Thread Michael Niedermayer
On Fri, Oct 06, 2017 at 01:48:14AM +0200, wm4 wrote:
> On Fri, 6 Oct 2017 00:01:30 +0200
> Michael Niedermayer  wrote:
> 
> > The opaque_ref wraping is a really bad design. Iam not sure why
> > people defend it.
> 
> FFmpeg is full of this design. There are plenty of structs with
> opaque/priv fields that change meaning depending on the context
> (basically how the struct is used or what uses it). It affects all
> decoders, encoders, filters, demuxers, muxers, the av_log() call,
> functions that work with AVClass, AVOption, and probably more.

what you write is not true

each decoder, demuxer, ... CLASS has its own type of private context
nothing outside code specific to that class messes with it.
A snow decoder has a snow context.
If the outside structure is moved around its still a snow decoder with
a snow private context. No amount of moving the structure around makes
it invalid.

OTOH, opaque_ref is defined by the user application.
There is a single user application in the address space.

Before the patch
all AVFrame opaque_ref have the same type, no amount of moving/passing
AVFrames around results in an invalid AVFrame.

after the patch this is very different.
Each AVFrame becomes tied to the code that created it, opaque_ref
has a type that outside decoders, is defined by the user application
inside decoders, its a internal structure.

AVFrames are no longer a universal structure that can simply be passed
around.
This is bad design and it is fragile

private and opaque are also intended to be very different things.
In FFmpeg private is a internal thing like the internal context of an
encoder. Opaque is a exteral thing, from the user application or caller


You may have misunderstood me but
I am against AVFrames depending on the code that created them.
This is not just about this implementation (which is in fact buggy too),


Pointing to the implementation issues and bugs was just intended
to show how fragile this is.
I do not understand how one can on one hand see all the problems this
causes yet not see that the API that causes this is what is at fault.
And it is MUCH easier to fix the API than to fix all the issues that
context specific AVFrames would cause


> 
> Are you saying that FFmpeg is really bad design? That's funny.
>
> There is nothing unclean about this use of opaque_ref, just that it
> somewhat collides with awful legacy hacks like draw_horiz_band (but
> which could be fixed anyway).
> 

> Also what you said about nested decoders is simply incorrect. The

It is not incorrect in case current implementations of decoders
dont trigger it. 


[...]

> 
> Now I'm very curious to hear what your "cleaner" solution to this
> problem is.

add a new field to AVFrame if you want a field with semantics that
are different.
you said you are against this IIRC. But thats the simple, robust and
easy maintainable solution
more so if that field is not void* it could provide type checking
which most people consider a good thing.

A system simiar to side data for opaque data could be used too, i
would say thats overkill but some people like side data

One entry for the users opaque structure
One entry for the libavcodec private structure
One entry for a future libavfilter private structure

And this way you need no wraping or unwraping, a AVFrame either
has some extra opaque fields or it doesnt. nothing could cast them to
the wrong type.
This is much more robust than putting all these things in opaque_ref
and spending time and more time and more time writing code wraping
and unwraping at every interface point and then cursing half the
interface and likely in the future fighting over every bit of added
interface as it massivly increases complexity with the wraping

[...]
-- 
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] [PATCH 2/7] decode: add a method for attaching lavc-internal data to frames

2017-10-13 Thread Michael Niedermayer
On Fri, Oct 06, 2017 at 12:38:44AM +0100, Mark Thompson wrote:
> On 05/10/17 23:59, Mark Thompson wrote:
> > On 05/10/17 23:01, Michael Niedermayer wrote:
> >> On Thu, Oct 05, 2017 at 09:03:40PM +0100, Mark Thompson wrote:
> >>> On 05/10/17 17:47, Michael Niedermayer wrote:
>  On Wed, Oct 04, 2017 at 02:04:54PM +0200, wm4 wrote:
> > On Wed, 4 Oct 2017 13:37:31 +0200
> > Tobias Rapp  wrote:
> >
> >> On 04.10.2017 11:34, wm4 wrote:
> >>> On Wed, 4 Oct 2017 11:22:37 +0200
> >>> Michael Niedermayer  wrote:
> >>>   
>  On Wed, Oct 04, 2017 at 09:12:29AM +0200, wm4 wrote:  
> > On Tue, 3 Oct 2017 21:40:58 +0200
> > Michael Niedermayer  wrote:
> >  
> >> On Tue, Oct 03, 2017 at 03:15:13PM +0200, wm4 wrote:  
> >>> From: Anton Khirnov 
> >>>
> >>  
> >>> Use the AVFrame.opaque_ref field. The original user's opaque_ref 
> >>> is
> >>> wrapped in the lavc struct and then unwrapped before the frame is
> >>> returned to the caller.  
> >>
> >> this is a ugly hack
> >>
> >> one and the same field should not be used to hold both the
> >> users opaque_ref as well as a structure which is itself not a user
> >> opaque_ref  
> >
> > While the AVFrame is within libavcodec, it's libavcodec's frame, not
> > the user's. Thus your claim doesn't make too much sense. libavcodec
> > fully controls the meaning for its own AVFrames' opaque_ref, but
> > reconstruct the user's value when returning it.  
> 
>  i disagree  
> >>>
> >>> Well, you're wrong anyway.
> >>>   
>  such hacks should not be added, we do have enough hacks already  
> >>>
> >>> It's not a hack.  
> >>
> >> Changing the semantics of a field during its lifetime, even when only 
> >> done within private code, is at least unexpected behavior.
> >
> > That's not done.
> 
>  The semantics are defined by the docs, which state:
>  "AVBufferRef for free use by the API user."
> 
>  And before the patch this is true, all instances of this field are
>  controled by the user application and are consistent.
> 
>  after the patch the AVFrames used by a codec have their opaque_ref
>  replaced by a wraped structure relative to what the outside of this
>  codec has.
> 
> 
> > Conceptually the AVFrame with the changed behavior is
> > a new reference. Internally, AVFrame.opaque_ref will always have the
> > same semantics, pointing to the FrameDecodeData struct. Only at points
> > where the AVFrame ref is converted to/from the user struct this is
> > changed.
> >
> >>> This is done strictly when returning a valid AVFrame, so there is no
> >>> room for mistakes.  
> >>
> >> The room for mistake might not increase for external developers but it 
> >> increases for internal developers (maintenance cost).
> >
> > Like where? There are only 2 places where the code needs to deal with
> > it, and these are in shared code, not individual decoders.
> 
>  just greping for AVFrame in the headers shows callbacks, a direct
>  pointer to a AVFrame and the API functions that interface the codec
> 
>  Just thinking of a codec that instanciates another codec and how
>  exactly the callback which may originate from the user or the outer
>  codec would unwrap the potential nested wraping.
>  I really dont think we want this in FFmpeg
>  And this is just one example ...
> >>>
> >>
> >>> I don't understand this discussion.
> >>
> >> yes, i realize this, but iam not sure why
> >>
> >> its pretty simple and clear but you seem to skip over parts of what
> >> i said.
> >>
> >>
> >>>
> >>> As far as I can tell, the sequence is this:
> >>>
> >>> * libavcodec allocates an AVFrame structure.
> >>> * libavcodec calls get_buffer2 with that AVFrame structure; the user 
> >>> fills its fields.
> >>> * libavcodec extracts the buffer references and metadata from the 
> >>> AVFrame, and maybe frees it (some codecs reuse a single AVFrame for the 
> >>> lifetime of the codec, others allocate them each time).
> >>> ~ decoding happens, the buffers are written to ~
> >>> * The user allocates a new AVFrame structure.
> >>> * The user calls receive_frame with that new AVFrame structure; 
> >>> libavcodec its fields with references to the decoded data and associated 
> >>> metadata.
> >>> * The user can then read the buffers of the frame, along with its 
> >>> metadata.
> >>>
> >>> Why would it matter what happens in the middle?  The AVFrame structure at 
> >>> the end is not the AVFrame structure at the start, and the user can't 
> >>> assume anything about it at all - if they try to dereference a pointer to 
> >>> the AVFrame supplied by libavcodec for get_buffer2 after get_buffer2 has 
> >>> return

[FFmpeg-devel] [PATCH] configure: use pkg-config for libilbc

2017-10-13 Thread James Almer
With this, the check will include the needed pthreads ldflags when
pkg-config is invoked with the --static flag.

Signed-off-by: James Almer 
---
Alternatively, i can keep the current check and add $pthreads_extralibs
to it.

 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index dc486ed756..749fc073a1 100755
--- a/configure
+++ b/configure
@@ -5997,7 +5997,7 @@ enabled libgme&& { use_pkg_config libgme 
libgme gme/gme.h gme_new_em
 enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
check_lib libgsm "${gsm_hdr}" gsm_create 
-lgsm && break;
done || die "ERROR: libgsm not found"; }
-enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
-lilbc
+enabled libilbc   && require_pkg_config libilbc libilbc ilbc.h 
WebRtcIlbcfix_InitDecode
 enabled libkvazaar&& require_pkg_config libkvazaar "kvazaar >= 0.8.1" 
kvazaar.h kvz_api_get
 # While it may appear that require is being used as a pkg-config
 # fallback for libmfx, it is actually being used to detect a different
-- 
2.14.2

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


Re: [FFmpeg-devel] [FFmpeg-cvslog] Merge commit '7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63'

2017-10-13 Thread James Almer
On 10/13/2017 1:48 PM, James Almer wrote:
> So in here it's including the libraries libopenmpt.pc said it needed
> (using pkg-config --static). Are "-lshlwapi -lpthread -lcrypt32"
> extralibs you added manually, or were they derived from dependencies
> like libmpg123, vorbis, etc by pkg-config?
> 
> Also, all the errors below mention functions from the c++ standard
> library, so it looks like libopenmpt.pc should be including -lstdc++
> in Libs.private but it doesn't? I see that libopempt depends on
> PortAudiocpp to build, so i'm guessing that's indeed the case.
> 
> See if adding -lstdc++ fixes it. If it does then it means that, before
> the commit that introduced these issues, a previous library you
> requested (maybe libgme) added -lstdc++ to the global ldflags.
> It would for that matter help a lot if you post the exact configure line
> you're using.

The attached patch adds the -lstdc++ flag to the check as i mentioned
above. See if it fixes it.
From 276ed55b9427aa21cd56b9511614b0d5df1081bc Mon Sep 17 00:00:00 2001
From: James Almer 
Date: Fri, 13 Oct 2017 15:16:55 -0300
Subject: [PATCH] configure: add a -lstdc++ flag to the libopenmpt check

It's missing in the pkg-config file, so add it here as a workaround.

Signed-off-by: James Almer 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 64fd088208..14b3b9f6c0 100755
--- a/configure
+++ b/configure
@@ -6030,7 +6030,7 @@ enabled libopenjpeg   && { { check_lib libopenjpeg 
openjpeg-2.3/openjpeg.h o
{ check_lib libopenjpeg openjpeg-1.5/openjpeg.h 
opj_version -lopenjpeg -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } ||
{ check_lib libopenjpeg openjpeg.h opj_version 
-lopenjpeg -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } ||
die "ERROR: libopenjpeg not found"; }
-enabled libopenmpt&& require_pkg_config libopenmpt "libopenmpt >= 
0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create
+enabled libopenmpt&& require_pkg_config libopenmpt "libopenmpt >= 
0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++
 enabled libopus   && {
 enabled libopus_decoder && {
 require_pkg_config libopus opus opus_multistream.h 
opus_multistream_decoder_create
-- 
2.14.2

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


Re: [FFmpeg-devel] [PATCH 2/7] decode: add a method for attaching lavc-internal data to frames

2017-10-13 Thread wm4
On Fri, 13 Oct 2017 20:03:12 +0200
Michael Niedermayer  wrote:

> I dont really know and as a maintainer of some of this code, i dont
> really want to have to keep track of how exactly AVFrames can pass
> through the code.

You have to do that anyway.

> And as the one taking care of a lot of security i totally have to
> reject this. It turns a robust system into something thats really
> fragile. This is only secure if every way a AVFrame can pass through
> the code has each interface point carefully convert the opaque_ref type.
> Thats a large source of bugs and exploits.

The security argument is bullshit. It's very clearly defined how the
field behaves. If you pass it to libavcodec you have to wrap it, if you
return it you have to unwrap it.

> Just look at how hard it is to get this patch work initially.

The only hard thing was "discussing" this with you. I even gave up my
resistance to release ffmpeg 3.4 without it, and took care of your
considerations (see updated patches I've sent before).

Normally you're quick to apply whatever fragile and complicated BS
there is, only here you're making an exception. Why the special
treatment.

> How many future commits will change the way AVFrames move and incorrectly
> update all the wraping. developers are not machines this suprising
> requirement of (un)wraping on interfaces will not be done correctly
> in a few years from now even if you get it all correct now.

Again, this is not complicated. Even if it is, it's less complicated
than all the tricky things you insist on and defend.

> Now add libavformat, libavdevice, libavfilter specific opaque_ref
> wraping
> 
> you really consider this to be good design by any definition of "good"?

It's probably better than whatever you could come up with. But you
didn't even make a suggestion.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 3/3] libavformat/aviobuf: don't treat 0 as EOF

2017-10-13 Thread Daniel Kucera
Signed-off-by: Daniel Kucera 
---
 libavformat/aviobuf.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 636cb46161..0d4eb051e1 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -572,13 +572,14 @@ static void fill_buffer(AVIOContext *s)
 if (s->read_packet)
 len = s->read_packet(s->opaque, dst, len);
 else
-len = 0;
-if (len <= 0) {
+len = AVERROR_EOF;
+if (len == AVERROR_EOF) {
 /* do not modify buffer if EOF reached so that a seek back can
be done without rereading data */
 s->eof_reached = 1;
-if (len < 0)
-s->error = len;
+} else if (len < 0) {
+s->eof_reached = 1;
+s->error= len;
 } else {
 s->pos += len;
 s->buf_ptr = dst;
@@ -646,13 +647,16 @@ int avio_read(AVIOContext *s, unsigned char *buf, int 
size)
 // bypass the buffer and read data directly into buf
 if(s->read_packet)
 len = s->read_packet(s->opaque, buf, size);
-
-if (len <= 0) {
+else
+len = AVERROR_EOF;
+if (len == AVERROR_EOF) {
 /* do not modify buffer if EOF reached so that a seek back 
can
 be done without rereading data */
 s->eof_reached = 1;
-if(len<0)
-s->error= len;
+break;
+} else if (len < 0) {
+s->eof_reached = 1;
+s->error= len;
 break;
 } else {
 s->pos += len;
-- 
2.11.0

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


[FFmpeg-devel] [PATCH 2/3] libavformat/cache: don't treat 0 as EOF

2017-10-13 Thread Daniel Kucera
Signed-off-by: Daniel Kucera 
---
 libavformat/cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/cache.c b/libavformat/cache.c
index 6aabca2e78..66bbbf54c9 100644
--- a/libavformat/cache.c
+++ b/libavformat/cache.c
@@ -201,7 +201,7 @@ static int cache_read(URLContext *h, unsigned char *buf, 
int size)
 }
 
 r = ffurl_read(c->inner, buf, size);
-if (r == 0 && size>0) {
+if (r == AVERROR_EOF && size>0) {
 c->is_true_eof = 1;
 av_assert0(c->end >= c->logical_pos);
 }
@@ -263,7 +263,7 @@ resolve_eof:
 if (whence == SEEK_SET)
 size = FFMIN(sizeof(tmp), pos - c->logical_pos);
 ret = cache_read(h, tmp, size);
-if (ret == 0 && whence == SEEK_END) {
+if (ret == AVERROR_EOF && whence == SEEK_END) {
 av_assert0(c->is_true_eof);
 goto resolve_eof;
 }
-- 
2.11.0

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


[FFmpeg-devel] [PATCH 1/3] libavformat/avio: retry_transfer_wrapper: don't treat 0 as EOF

2017-10-13 Thread Daniel Kucera
transfer_func variable passed to retry_transfer_wrapper
are h->prot->url_read and h->prot->url_write functions.
These need to return EOF or other error properly.
In case of returning >= 0, url_read/url_write is retried
until error is returned.

Signed-off-by: Daniel Kucera 
---
 libavformat/avio.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/avio.c b/libavformat/avio.c
index 64248e098b..d3004c007f 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -391,8 +391,10 @@ static inline int retry_transfer_wrapper(URLContext *h, 
uint8_t *buf,
 }
 av_usleep(1000);
 }
-} else if (ret < 1)
-return (ret < 0 && ret != AVERROR_EOF) ? ret : len;
+} else if (ret == AVERROR_EOF)
+   return (len > 0) ? len : AVERROR_EOF;
+else if (ret < 0)
+return ret;
 if (ret) {
 fast_retries = FFMAX(fast_retries, 2);
 wait_since = 0;
-- 
2.11.0

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


Re: [FFmpeg-devel] [PATCH 2/7] decode: add a method for attaching lavc-internal data to frames

2017-10-13 Thread wm4
On Fri, 13 Oct 2017 19:41:28 +0200
Michael Niedermayer  wrote:

> On Fri, Oct 06, 2017 at 01:48:14AM +0200, wm4 wrote:
> > On Fri, 6 Oct 2017 00:01:30 +0200
> > Michael Niedermayer  wrote:
> >   
> > > The opaque_ref wraping is a really bad design. Iam not sure why
> > > people defend it.  
> > 
> > FFmpeg is full of this design. There are plenty of structs with
> > opaque/priv fields that change meaning depending on the context
> > (basically how the struct is used or what uses it). It affects all
> > decoders, encoders, filters, demuxers, muxers, the av_log() call,
> > functions that work with AVClass, AVOption, and probably more.  
> 
> what you write is not true
> 
> each decoder, demuxer, ... CLASS has its own type of private context
> nothing outside code specific to that class messes with it.
> A snow decoder has a snow context.
> If the outside structure is moved around its still a snow decoder with
> a snow private context. No amount of moving the structure around makes
> it invalid.
> 
> OTOH, opaque_ref is defined by the user application.
> There is a single user application in the address space.

You misunderstand how AVFrame works. AVFrame has an owner, and this
owner decides how certain fields are handled. This includes for example
the pts fields, whose meaning entirely depend on an undefined timebase.
opaque_ref is merely a more advanced case of this.

> Before the patch
> all AVFrame opaque_ref have the same type, no amount of moving/passing
> AVFrames around results in an invalid AVFrame.

Wrong. Even a user application could have multiple uses of opaque_ref,
all with their own meaning. You can't interpret this field without
context about where its value came from.

> after the patch this is very different.
> Each AVFrame becomes tied to the code that created it, opaque_ref
> has a type that outside decoders, is defined by the user application
> inside decoders, its a internal structure.
> 
> AVFrames are no longer a universal structure that can simply be passed
> around.
> This is bad design and it is fragile
> 
> private and opaque are also intended to be very different things.
> In FFmpeg private is a internal thing like the internal context of an
> encoder. Opaque is a exteral thing, from the user application or caller

Why can you access the private fields anyway? This is fragile and bad
design.

> You may have misunderstood me but
> I am against AVFrames depending on the code that created them.

Suggest something better.

> This is not just about this implementation (which is in fact buggy too),

Is it.

> Pointing to the implementation issues and bugs was just intended
> to show how fragile this is.
> I do not understand how one can on one hand see all the problems this
> causes yet not see that the API that causes this is what is at fault.
> And it is MUCH easier to fix the API than to fix all the issues that
> context specific AVFrames would cause

Fix what API? Do you even understand for what this is needed?

> add a new field to AVFrame if you want a field with semantics that
> are different.

HAHAHAHAHAHAHAAHAHAHAHA

This is a load of badly designed bullshit. This would add another
AVFrame field for something that is INTERNAL and IMPLEMENTATION DEFINED
to libavcodec. Do you know that we have a field for this, which is
supposed to be used by anything that requires INTERNAL and
IMPLEMENTATION DEFINED values by the AVFrame users? It's called
opaque_ref.

> you said you are against this IIRC. But thats the simple, robust and
> easy maintainable solution

It's not simple, because you STILL can forget to unwrap or wrap the
AVFrame properly. Forgetting to unwrap it would be "harmless" in your
opinion because the user isn't allowed to read it, while with
opaque_ref it would be "dangerous". But the truth is that an
"unwrapped" frame doesn't contain the correct data anyway - post
processing was not run, and returning the frame in this state to the
user is undefined behavior.

Skipping unwrapping is a bug. Unwrapping will restore opaque_ref to the
value it should be.

Please come up with something that is actually better.

Anyway, in my opinion, it would be better to disallow opaque_ref being
set by the user's get_buffer2 callback. That would solve most of the
issues you have with it, but surely you would come up with some crap to
block my work anyway.

> more so if that field is not void* it could provide type checking
> which most people consider a good thing.
> 
> A system simiar to side data for opaque data could be used too, i
> would say thats overkill but some people like side data

Side data has literally nothing to do with this. Side data types can't
be defined by the user anyway. Why do codec/filter/demuxer private
fields have no type checking? Or av_log? Or the whole AVOption API?
Please answer this.

> One entry for the users opaque structure
> One entry for the libavcodec private structure
> One entry for a future libavfilter private structure

One entry for application 1.
..

Re: [FFmpeg-devel] [PATCH] ffmpeg.c: Fallback to duration_dts, when duration_pts can't be determined.

2017-10-13 Thread Michael Niedermayer
On Thu, Oct 12, 2017 at 10:43:54AM +0200, Thomas Mundt wrote:
> Hi Michael,
> 
> 2017-10-12 1:28 GMT+02:00 Michael Niedermayer :
> 
> > On Tue, Oct 10, 2017 at 10:26:13PM +0200, Thomas Mundt wrote:
> > > 2017-10-10 19:36 GMT+02:00 Sasi Inguva :
> > >
> > > > This is required for FLV files, for which duration_pts comes out to be
> > > > zero.
> > > >
> > > > Signed-off-by: Sasi Inguva 
> > > > ---
> > > >  fftools/ffmpeg.c | 9 +++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > > > index 6d64bc1043..3ee31473dc 100644
> > > > --- a/fftools/ffmpeg.c
> > > > +++ b/fftools/ffmpeg.c
> > > > @@ -2665,8 +2665,13 @@ static int process_input_packet(InputStream
> > *ist,
> > > > const AVPacket *pkt, int no_eo
> > > >  ist->next_dts = AV_NOPTS_VALUE;
> > > >  }
> > > >
> > > > -if (got_output)
> > > > -ist->next_pts += av_rescale_q(duration_pts,
> > > > ist->st->time_base, AV_TIME_BASE_Q);
> > > > +if (got_output) {
> > > > +if (duration_pts > 0) {
> > > > +ist->next_pts += av_rescale_q(duration_pts,
> > > > ist->st->time_base, AV_TIME_BASE_Q);
> > > > +} else {
> > > > +ist->next_pts += duration_dts;
> > > > +}
> > > > +}
> > > >  break;
> > > >  case AVMEDIA_TYPE_SUBTITLE:
> > > >  if (repeating)
> > > > --
> > > >
> > >
> > > Patch LGTM.
> >
> > will apply
> >
> 
> would you mind pushing this patch to the 3.4 branch also?

ok, will be part of by next push to release/3.4 unless someone wants
this to be omitted 

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire


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


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread James Almer
On 10/13/2017 12:37 PM, James Almer wrote:
> On 10/13/2017 11:30 AM, Hendrik Leppkes wrote:
>> On Fri, Oct 13, 2017 at 4:14 PM, James Almer  wrote:
>>> On 10/12/2017 6:30 PM, James Almer wrote:
 Should prevent some options from being added to cflags when they
 don't exist and the compiler only warns about it.

 Signed-off-by: James Almer 
 ---
 I figure this is safer than adding
 -Werror=unused-command-line-argument -Werror=unknown-warning-option
 as Ronald suggested.

  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/configure b/configure
 index ade67a31bb..c7962665f1 100755
 --- a/configure
 +++ b/configure
 @@ -6370,7 +6370,7 @@ fi

  check_disable_warning(){
  warning_flag=-W${1#-Wno-}
 -test_cflags $warning_flag && add_cflags $1
 +test_cflags -Werror $warning_flag && add_cflags $1
  }

  check_disable_warning -Wno-parentheses
>>> Ping. This or a similar solution has been annoying Clang users for some
>>> days now and should be part of the 3.4 release.
>> I wonder if a general -Werror is really "safer", do these tests really
>> execute without any other warnings, which might trigger a false
>> negative? We're not exactly making it compile a proper source file, so
>> it might warn about random things.
>>
>> - Hendrik
> Ok, what about the attached patch, then?
> 
> Both -Werror=unused-command-line-argument and
> -Werror=unknown-warning-option are not supported by gcc, so they
> generate an error and would break every check.
> This hopefully only uses them where they exist.
> 
> 
> 0001-configure-force-erroring-out-in-check_disable_warnin.patch
> 
> 
> From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001
> From: James Almer 
> Date: Fri, 13 Oct 2017 12:34:34 -0300
> Subject: [PATCH] configure: force erroring out in check_disable_warning() if
>  an option doesn't exists
> 
> Should prevent some options from being added to cflags when they
> don't exist and the compiler only warns about it.
> 
> Signed-off-by: James Almer 
> ---
>  configure | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 64fd088208..87087f02e4 100755
> --- a/configure
> +++ b/configure
> @@ -6370,9 +6370,14 @@ fi
>  
>  check_disable_warning(){
>  warning_flag=-W${1#-Wno-}
> -test_cflags $warning_flag && add_cflags $1
> +test_cflags $unknown_warning_flags $warning_flag && add_cflags $1
>  }
>  
> +test_cflags -Werror=unused-command-line-argument &&
> +append unknown_warning_flags "-Werror=unused-command-line-argument"
> +test_cflags -Werror=unknown-warning-option &&
> +append unknown_warning_flags "-Werror=unknown-warning-option"
> +
>  check_disable_warning -Wno-parentheses
>  check_disable_warning -Wno-switch
>  check_disable_warning -Wno-format-zero-length
> -- 2.14.2

I'll push this version later today if nobody comments.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [FFmpeg-cvslog] Merge commit '7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63'

2017-10-13 Thread Reino Wijnsma
On 13-10-2017 18:48, James Almer  wrote:
> Are "-lshlwapi -lpthread -lcrypt32"
> extralibs you added manually, or were they derived from dependencies
> like libmpg123, vorbis, etc by pkg-config?
My configure line up until now with the manually added extra-libs:

# ./configure --arch=x86 --target-os=mingw32
--cross-prefix=/cygdrive/m/ffmpeg-windows-build-helpers-master/native_build/windows/ffmpeg_local_builds/sandbox/cross_compilers/mingw-w64-i686/bin/i686-w64-mingw32-
--pkg-config=pkg-config --pkg-config-flags=--static
--extra-version=Reino --enable-gray --enable-version3 --disable-debug
--disable-doc --disable-htmlpages --disable-manpages --disable-podpages
--disable-schannel --disable-txtpages --disable-w32threads
--enable-avisynth --enable-avresample --enable-fontconfig
--enable-frei0r --enable-filter=frei0r --enable-gmp --enable-gnutls
*--extra-libs=-lcrypt32* --enable-gpl --enable-libass --enable-libbluray
--enable-libbs2b --enable-libcaca --extra-cflags=-DCACA_STATIC
--enable-libfdk-aac --enable-libflite --enable-libfreetype
--enable-libfribidi --enable-libgme --enable-libgsm --enable-libilbc
*--extra-libs=-lpthread* --enable-libmp3lame --enable-libmysofa
--enable-libopencore-amrnb --enable-libopencore-amrwb
--enable-libopenh264 --enable-libopenmpt *--extra-libs=-lshlwapi*
--enable-libopus --enable-librubberband --enable-libsnappy
--enable-libsoxr --enable-libspeex --enable-libtheora
--enable-libtwolame --extra-cflags=-DLIBTWOLAME_STATIC
--enable-libvidstab --enable-libvo-amrwbenc --enable-libvorbis
--enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265
--enable-libxavs --enable-libxml2 --enable-libxvid --enable-libzimg
--enable-libzvbi --extra-cflags='-march=pentium3' --extra-cflags=-O2
--extra-cflags='-mfpmath=sse' --extra-cflags=-msse --enable-static
--disable-shared
--prefix=/cygdrive/m/ffmpeg-windows-build-helpers-master/native_build/windows/ffmpeg_local_builds/sandbox/cross_compilers/mingw-w64-i686/i686-w64-mingw32

> See if adding -lstdc++ fixes it.
It does. And since we're in the process of fixing the external library
checks in 'configure', I've removed all --extra-libs and --extra-cflags
entries from the configure line and I've made some changes to
'configure' until no more errors showed up.

_Libcaca_

-enabled libcaca   && require_pkg_config libcaca caca caca.h
caca_create_canvas
+enabled libcaca   && require_pkg_config libcaca caca caca.h
caca_create_canvas -DCACA_STATIC && add_cppflags -DCACA_STATIC
(see
https://github.com/Reino17/ffmpeg-windows-build-helpers/blob/master/cross_compile_ffmpeg.sh#L1652-L1653
)

Otherwise you'll get "undefined reference to `_imp__caca_create_canvas'"
and "ERROR: caca not found using pkg-config".

_Libilbc_

On 13-10-2017 20:09, James Almer  wrote:
> With this, the check will include the needed pthreads ldflags when
> pkg-config is invoked with the --static flag.
My 'libilbc.pc' doesn't include -lpthread at all. Adding it here
manually works for me.

-enabled libilbc   && require libilbc ilbc.h
WebRtcIlbcfix_InitDecode -lilbc
+enabled libilbc   && require_pkg_config libilbc libilbc ilbc.h
WebRtcIlbcfix_InitDecode -lpthread

_Libmodplug_

-enabled libmodplug&& require_pkg_config libmodplug libmodplug
libmodplug/modplug.h ModPlug_Load
+enabled libmodplug&& require_pkg_config libmodplug libmodplug
libmodplug/modplug.h ModPlug_Load -DMODPLUG_STATIC && add_cppflags
-DMODPLUG_STATIC

Although I've replaced libmodplug for libopenmpt, I know FFmpeg's
'configure' needs this. I don't know the "undefined reference"-message
anymore.

_Libopenmpt_

-enabled libopenmpt&& require_pkg_config libopenmpt "libopenmpt
>= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create
+enabled libopenmpt&& require_pkg_config libopenmpt "libopenmpt
>= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++
-lshlwapi

Adding -lstdc++ fixes the 12000 errors I mentioned earlier, but without
-lshlwapi you'd still get:

/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0xcb):
undefined reference to `_imp__PathIsRelativeW@4'
/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0x224):
undefined reference to `_imp__PathIsUNCW@4'
/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0x4e9):
undefined reference to `_imp__PathIsRelativeW@4'
/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0x670):
undefined reference to `_imp__PathIsUNCW@4'
/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0xd4b):
undefined reference to `_imp__PathCombineW@12'
/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0xf1b):
undefined reference to `_imp__PathIsRelativeW@4'
/cygdrive/[...]/lib/libmpg123.a(compat.o):compat.c:(.text+0x10c4):
undefined reference to `_imp__PathIsUNCW@4'

_Librubberband_

-enabled librubberband && require_pkg_config librubberband
"rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new
+enabled librubberband && require_pkg_config librubberband
"rubberband >

Re: [FFmpeg-devel] [PATCH 3/3] libavformat/aviobuf: don't treat 0 as EOF

2017-10-13 Thread Michael Niedermayer
On Fri, Oct 13, 2017 at 09:03:49PM +0200, Daniel Kucera wrote:
> Signed-off-by: Daniel Kucera 
> ---
>  libavformat/aviobuf.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)

with this

make -j12 fate-wtv-demux

infinite loops

[...]
-- 
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] [PATCH 1/3] libavformat/avio: retry_transfer_wrapper: don't treat 0 as EOF

2017-10-13 Thread Michael Niedermayer
On Fri, Oct 13, 2017 at 09:03:47PM +0200, Daniel Kucera wrote:
> transfer_func variable passed to retry_transfer_wrapper
> are h->prot->url_read and h->prot->url_write functions.
> These need to return EOF or other error properly.
> In case of returning >= 0, url_read/url_write is retried
> until error is returned.
> 
> Signed-off-by: Daniel Kucera 
> ---
>  libavformat/avio.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

breaks fate-indeo3-2

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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact


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] Merge commit '7cb1d9e2dbbe5bf4652be5d78cdd68e956fa3d63'

2017-10-13 Thread James Almer
On 10/13/2017 6:30 PM, Reino Wijnsma wrote:
> On 13-10-2017 18:48, James Almer  wrote:
>> Are "-lshlwapi -lpthread -lcrypt32"
>> extralibs you added manually, or were they derived from dependencies
>> like libmpg123, vorbis, etc by pkg-config?
> My configure line up until now with the manually added extra-libs:
> 
> # ./configure --arch=x86 --target-os=mingw32
> --cross-prefix=/cygdrive/m/ffmpeg-windows-build-helpers-master/native_build/windows/ffmpeg_local_builds/sandbox/cross_compilers/mingw-w64-i686/bin/i686-w64-mingw32-
> --pkg-config=pkg-config --pkg-config-flags=--static
> --extra-version=Reino --enable-gray --enable-version3 --disable-debug
> --disable-doc --disable-htmlpages --disable-manpages --disable-podpages
> --disable-schannel --disable-txtpages --disable-w32threads
> --enable-avisynth --enable-avresample --enable-fontconfig
> --enable-frei0r --enable-filter=frei0r --enable-gmp --enable-gnutls
> *--extra-libs=-lcrypt32* --enable-gpl --enable-libass --enable-libbluray
> --enable-libbs2b --enable-libcaca --extra-cflags=-DCACA_STATIC
> --enable-libfdk-aac --enable-libflite --enable-libfreetype
> --enable-libfribidi --enable-libgme --enable-libgsm --enable-libilbc
> *--extra-libs=-lpthread* --enable-libmp3lame --enable-libmysofa
> --enable-libopencore-amrnb --enable-libopencore-amrwb
> --enable-libopenh264 --enable-libopenmpt *--extra-libs=-lshlwapi*
> --enable-libopus --enable-librubberband --enable-libsnappy
> --enable-libsoxr --enable-libspeex --enable-libtheora
> --enable-libtwolame --extra-cflags=-DLIBTWOLAME_STATIC
> --enable-libvidstab --enable-libvo-amrwbenc --enable-libvorbis
> --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265
> --enable-libxavs --enable-libxml2 --enable-libxvid --enable-libzimg
> --enable-libzvbi --extra-cflags='-march=pentium3' --extra-cflags=-O2
> --extra-cflags='-mfpmath=sse' --extra-cflags=-msse --enable-static
> --disable-shared
> --prefix=/cygdrive/m/ffmpeg-windows-build-helpers-master/native_build/windows/ffmpeg_local_builds/sandbox/cross_compilers/mingw-w64-i686/i686-w64-mingw32
> 
>> See if adding -lstdc++ fixes it.
> It does.

Ok, fixing this way then.

Consider reporting this bug to libopenmpt, while at it. They should add
-lstdc++ to their pkg-config file for static builds, instead of projects
using the library having to workaround it.

> And since we're in the process of fixing the external library
> checks in 'configure', I've removed all --extra-libs and --extra-cflags
> entries from the configure line and I've made some changes to
> 'configure' until no more errors showed up.

Thanks a lot for that!

> 
> _Libcaca_
> 
> -enabled libcaca   && require_pkg_config libcaca caca caca.h
> caca_create_canvas
> +enabled libcaca   && require_pkg_config libcaca caca caca.h
> caca_create_canvas -DCACA_STATIC && add_cppflags -DCACA_STATIC
> (see
> https://github.com/Reino17/ffmpeg-windows-build-helpers/blob/master/cross_compile_ffmpeg.sh#L1652-L1653
> )
> 
> Otherwise you'll get "undefined reference to `_imp__caca_create_canvas'"
> and "ERROR: caca not found using pkg-config".

This i will not touch. It's unrelated to the commit that revamped the
dependency checking code and is something the maintainer of the libcaca
wrapper should solve.

> 
> _Libilbc_
> 
> On 13-10-2017 20:09, James Almer  wrote:
>> With this, the check will include the needed pthreads ldflags when
>> pkg-config is invoked with the --static flag.
> My 'libilbc.pc' doesn't include -lpthread at all. Adding it here
> manually works for me.

It doesn't? That's weird, seeing "Libs.private: @PTHREAD_LIBS@" is
listed in the source package.

> 
> -enabled libilbc   && require libilbc ilbc.h
> WebRtcIlbcfix_InitDecode -lilbc
> +enabled libilbc   && require_pkg_config libilbc libilbc ilbc.h
> WebRtcIlbcfix_InitDecode -lpthread

This doesn't work as is. -lpthread will be used as part of the check,
but afterwards it will not be added to libilbc's ldflags.
require_pkg_config() doesn't work the same as require(), for some reason.

I'll keep the current non pkg-config check then and add the pthreads
ldflag to it.

> 
> _Libmodplug_
> 
> -enabled libmodplug    && require_pkg_config libmodplug libmodplug
> libmodplug/modplug.h ModPlug_Load
> +enabled libmodplug    && require_pkg_config libmodplug libmodplug
> libmodplug/modplug.h ModPlug_Load -DMODPLUG_STATIC && add_cppflags
> -DMODPLUG_STATIC

Again, i'll not change this. Needing project specific pre-processor
flags is unrelated.

> 
> Although I've replaced libmodplug for libopenmpt, I know FFmpeg's
> 'configure' needs this. I don't know the "undefined reference"-message
> anymore.
> 
> _Libopenmpt_
> 
> -enabled libopenmpt    && require_pkg_config libopenmpt "libopenmpt
>>= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create
> +enabled libopenmpt    && require_pkg_config libopenmpt "libopen

Re: [FFmpeg-devel] [PATCH] configure: use pkg-config for libilbc

2017-10-13 Thread James Almer
On 10/13/2017 3:09 PM, James Almer wrote:
> With this, the check will include the needed pthreads ldflags when
> pkg-config is invoked with the --static flag.
> 
> Signed-off-by: James Almer 
> ---
> Alternatively, i can keep the current check and add $pthreads_extralibs
> to it.

Patch dropped. It seems the pkg-config file is not guaranteed to always
include the pthreads ldflag.
Added $pthreads_extralibs instead and pushed.

> 
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index dc486ed756..749fc073a1 100755
> --- a/configure
> +++ b/configure
> @@ -5997,7 +5997,7 @@ enabled libgme&& { use_pkg_config libgme 
> libgme gme/gme.h gme_new_em
>  enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
> check_lib libgsm "${gsm_hdr}" gsm_create 
> -lgsm && break;
> done || die "ERROR: libgsm not found"; }
> -enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
> -lilbc
> +enabled libilbc   && require_pkg_config libilbc libilbc ilbc.h 
> WebRtcIlbcfix_InitDecode
>  enabled libkvazaar&& require_pkg_config libkvazaar "kvazaar >= 
> 0.8.1" kvazaar.h kvz_api_get
>  # While it may appear that require is being used as a pkg-config
>  # fallback for libmfx, it is actually being used to detect a different
> 

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


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread Michael Niedermayer
On Fri, Oct 13, 2017 at 12:37:18PM -0300, James Almer wrote:
> On 10/13/2017 11:30 AM, Hendrik Leppkes wrote:
> > On Fri, Oct 13, 2017 at 4:14 PM, James Almer  wrote:
> >> On 10/12/2017 6:30 PM, James Almer wrote:
> >>> Should prevent some options from being added to cflags when they
> >>> don't exist and the compiler only warns about it.
> >>>
> >>> Signed-off-by: James Almer 
> >>> ---
> >>> I figure this is safer than adding
> >>> -Werror=unused-command-line-argument -Werror=unknown-warning-option
> >>> as Ronald suggested.
> >>>
> >>>  configure | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/configure b/configure
> >>> index ade67a31bb..c7962665f1 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -6370,7 +6370,7 @@ fi
> >>>
> >>>  check_disable_warning(){
> >>>  warning_flag=-W${1#-Wno-}
> >>> -test_cflags $warning_flag && add_cflags $1
> >>> +test_cflags -Werror $warning_flag && add_cflags $1
> >>>  }
> >>>
> >>>  check_disable_warning -Wno-parentheses
> >>
> >> Ping. This or a similar solution has been annoying Clang users for some
> >> days now and should be part of the 3.4 release.
> > 
> > I wonder if a general -Werror is really "safer", do these tests really
> > execute without any other warnings, which might trigger a false
> > negative? We're not exactly making it compile a proper source file, so
> > it might warn about random things.
> > 
> > - Hendrik
> 
> Ok, what about the attached patch, then?
> 
> Both -Werror=unused-command-line-argument and
> -Werror=unknown-warning-option are not supported by gcc, so they
> generate an error and would break every check.
> This hopefully only uses them where they exist.

>  configure |7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 11cd9cc220b675d4cbe171b61c969e024458afad  
> 0001-configure-force-erroring-out-in-check_disable_warnin.patch
> From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001
> From: James Almer 
> Date: Fri, 13 Oct 2017 12:34:34 -0300
> Subject: [PATCH] configure: force erroring out in check_disable_warning() if
>  an option doesn't exists
> 
> Should prevent some options from being added to cflags when they
> don't exist and the compiler only warns about it.
> 
> Signed-off-by: James Almer 
> ---
>  configure | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

iam a bit surprised this has not been fixed long ago
but LGTM

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


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


Re: [FFmpeg-devel] [PATCH] configure: force erroring out in check_disable_warning() if an option doesn't exists

2017-10-13 Thread James Almer
On 10/13/2017 8:54 PM, Michael Niedermayer wrote:
> On Fri, Oct 13, 2017 at 12:37:18PM -0300, James Almer wrote:
>> On 10/13/2017 11:30 AM, Hendrik Leppkes wrote:
>>> On Fri, Oct 13, 2017 at 4:14 PM, James Almer  wrote:
 On 10/12/2017 6:30 PM, James Almer wrote:
> Should prevent some options from being added to cflags when they
> don't exist and the compiler only warns about it.
>
> Signed-off-by: James Almer 
> ---
> I figure this is safer than adding
> -Werror=unused-command-line-argument -Werror=unknown-warning-option
> as Ronald suggested.
>
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index ade67a31bb..c7962665f1 100755
> --- a/configure
> +++ b/configure
> @@ -6370,7 +6370,7 @@ fi
>
>  check_disable_warning(){
>  warning_flag=-W${1#-Wno-}
> -test_cflags $warning_flag && add_cflags $1
> +test_cflags -Werror $warning_flag && add_cflags $1
>  }
>
>  check_disable_warning -Wno-parentheses

 Ping. This or a similar solution has been annoying Clang users for some
 days now and should be part of the 3.4 release.
>>>
>>> I wonder if a general -Werror is really "safer", do these tests really
>>> execute without any other warnings, which might trigger a false
>>> negative? We're not exactly making it compile a proper source file, so
>>> it might warn about random things.
>>>
>>> - Hendrik
>>
>> Ok, what about the attached patch, then?
>>
>> Both -Werror=unused-command-line-argument and
>> -Werror=unknown-warning-option are not supported by gcc, so they
>> generate an error and would break every check.
>> This hopefully only uses them where they exist.
> 
>>  configure |7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 11cd9cc220b675d4cbe171b61c969e024458afad  
>> 0001-configure-force-erroring-out-in-check_disable_warnin.patch
>> From 0e305676e6b30ed3274fcbb4f31e318d41cac66d Mon Sep 17 00:00:00 2001
>> From: James Almer 
>> Date: Fri, 13 Oct 2017 12:34:34 -0300
>> Subject: [PATCH] configure: force erroring out in check_disable_warning() if
>>  an option doesn't exists
>>
>> Should prevent some options from being added to cflags when they
>> don't exist and the compiler only warns about it.
>>
>> Signed-off-by: James Almer 
>> ---
>>  configure | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> iam a bit surprised this has not been fixed long ago
> but LGTM

Applied and backported, thanks!
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/3] libavformat/avio: retry_transfer_wrapper: don't treat 0 as EOF

2017-10-13 Thread Daniel Kučera
breaks fate-indeo3-2

[...]
--


You need to apply the whole set. It passed all fate tests on my machine.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel