Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[Michael Niedermayer]

On Sun, Mar 18, 2018 at 01:40:15PM +, j...@itanimul.li wrote:
> From: Josh de Kock 
> 
> ---
>  tests/checkasm/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 0520e264e2..ae7e810d25 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -61,7 +61,7 @@ tests/checkasm/checkasm.o: CFLAGS += -Umain
>  CHECKASM := tests/checkasm/checkasm$(EXESUF)
>  
>  $(CHECKASM): $(CHECKASMOBJS) $(FF_STATIC_DEP_LIBS)
> - $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS) 
> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter) 
> $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)
> + $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS) 
> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter) 
> $(EXTRALIBS-avformat) $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)

These additional dependancies should not be needed.
It does not entirely feel right to me that adding a new API would cause
additional dependancies between libraries to appear


thx

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

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk



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

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[Josh de Kock]

On Sun, Mar 18, 2018 at 01:42:12PM -0300, James Almer wrote:
> On 3/18/2018 1:06 PM, Paul B Mahol wrote:
> > [...]
> > And how do you know that?, without even providing alternative?
> 
> I provided an alternative. Look at my comment about the *_FFLIBS
> variables. The build system already uses it when linking the libraries
> themselves.
> The build system has a lot of measures to only build the required
> objects and only link the required libraries based on configure time
> options. Hardcoding avformat and swresample extralibs for this
> executable when avcodec and avfilter may be built without the modules
> that require said libraries is doing the exact opposite of that.

I will look into the *_FFLIBS variables. I figured it would be an OK
patch because of the swresample one recently added.

> Sure, we can commit this as a "temporary" fix, seeing that something
> needs to be done in a timely manner so the iterate() api can be
> committed, but the result will be that nobody will bother to implement
> it right afterwards.

This would be preferred, or at least looking at the other patches first
as they are more complex. This is an issue which can be looked at after
all the other major issues are discussed, the main one being:

How avdevice and avformat interact going forward (see the v2 of the patch
which removes a bunch of register_all() functions for an explanation
of this issue).

-- 
Josh

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

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[James Almer]

On 3/18/2018 1:06 PM, Paul B Mahol wrote:
> On 3/18/18, James Almer  wrote:
>> On 3/18/2018 12:17 PM, Paul B Mahol wrote:
>>> On 3/18/18, James Almer  wrote:
 On 3/18/2018 10:40 AM, j...@itanimul.li wrote:
> From: Josh de Kock 
>
> ---
>  tests/checkasm/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 0520e264e2..ae7e810d25 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -61,7 +61,7 @@ tests/checkasm/checkasm.o: CFLAGS += -Umain
>  CHECKASM := tests/checkasm/checkasm$(EXESUF)
>
>  $(CHECKASM): $(CHECKASMOBJS) $(FF_STATIC_DEP_LIBS)
> - $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
> $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)
> + $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
> $(EXTRALIBS-avformat) $(EXTRALIBS-avutil) $(EXTRALIBS-swresample)
> $(EXTRALIBS)

 This is not the correct fix. We currently only need to link to avcodec,
 avfilter and avutil here, which may or may not depend on other fflibs.
 Seeing swresample there already hints that this line got duct tape fixes
 as linking failures popped up. How long until someone gets a linking
 failure pointing to swscale or postproc, seeing avfilter may link to it
 depending on enabled filters, much like it does with avformat?

 The *_FFLIBS variables in config.mak list these dependencies per lib,
 and should be used to assemble a proper linking command here.

>
>  checkasm: $(CHECKASM)
>
>

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

>>>
>>> Try patchset without it and than complain instead.
>>
>> I'm not saying it's not needed, I'm saying it's not the correct way to
>> fix the linking failure the following patches would introduce.
> 
> And how do you know that?, without even providing alternative?

I provided an alternative. Look at my comment about the *_FFLIBS
variables. The build system already uses it when linking the libraries
themselves.
The build system has a lot of measures to only build the required
objects and only link the required libraries based on configure time
options. Hardcoding avformat and swresample extralibs for this
executable when avcodec and avfilter may be built without the modules
that require said libraries is doing the exact opposite of that.

Sure, we can commit this as a "temporary" fix, seeing that something
needs to be done in a timely manner so the iterate() api can be
committed, but the result will be that nobody will bother to implement
it right afterwards.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[Paul B Mahol]

On 3/18/18, James Almer  wrote:
> On 3/18/2018 12:17 PM, Paul B Mahol wrote:
>> On 3/18/18, James Almer  wrote:
>>> On 3/18/2018 10:40 AM, j...@itanimul.li wrote:
 From: Josh de Kock 

 ---
  tests/checkasm/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
 index 0520e264e2..ae7e810d25 100644
 --- a/tests/checkasm/Makefile
 +++ b/tests/checkasm/Makefile
 @@ -61,7 +61,7 @@ tests/checkasm/checkasm.o: CFLAGS += -Umain
  CHECKASM := tests/checkasm/checkasm$(EXESUF)

  $(CHECKASM): $(CHECKASMOBJS) $(FF_STATIC_DEP_LIBS)
 -  $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
 $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
 $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)
 +  $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
 $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
 $(EXTRALIBS-avformat) $(EXTRALIBS-avutil) $(EXTRALIBS-swresample)
 $(EXTRALIBS)
>>>
>>> This is not the correct fix. We currently only need to link to avcodec,
>>> avfilter and avutil here, which may or may not depend on other fflibs.
>>> Seeing swresample there already hints that this line got duct tape fixes
>>> as linking failures popped up. How long until someone gets a linking
>>> failure pointing to swscale or postproc, seeing avfilter may link to it
>>> depending on enabled filters, much like it does with avformat?
>>>
>>> The *_FFLIBS variables in config.mak list these dependencies per lib,
>>> and should be used to assemble a proper linking command here.
>>>

  checkasm: $(CHECKASM)


>>>
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> Try patchset without it and than complain instead.
>
> I'm not saying it's not needed, I'm saying it's not the correct way to
> fix the linking failure the following patches would introduce.

And how do you know that?, without even providing alternative?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[James Almer]

On 3/18/2018 12:17 PM, Paul B Mahol wrote:
> On 3/18/18, James Almer  wrote:
>> On 3/18/2018 10:40 AM, j...@itanimul.li wrote:
>>> From: Josh de Kock 
>>>
>>> ---
>>>  tests/checkasm/Makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
>>> index 0520e264e2..ae7e810d25 100644
>>> --- a/tests/checkasm/Makefile
>>> +++ b/tests/checkasm/Makefile
>>> @@ -61,7 +61,7 @@ tests/checkasm/checkasm.o: CFLAGS += -Umain
>>>  CHECKASM := tests/checkasm/checkasm$(EXESUF)
>>>
>>>  $(CHECKASM): $(CHECKASMOBJS) $(FF_STATIC_DEP_LIBS)
>>> -   $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
>>> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
>>> $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)
>>> +   $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
>>> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
>>> $(EXTRALIBS-avformat) $(EXTRALIBS-avutil) $(EXTRALIBS-swresample)
>>> $(EXTRALIBS)
>>
>> This is not the correct fix. We currently only need to link to avcodec,
>> avfilter and avutil here, which may or may not depend on other fflibs.
>> Seeing swresample there already hints that this line got duct tape fixes
>> as linking failures popped up. How long until someone gets a linking
>> failure pointing to swscale or postproc, seeing avfilter may link to it
>> depending on enabled filters, much like it does with avformat?
>>
>> The *_FFLIBS variables in config.mak list these dependencies per lib,
>> and should be used to assemble a proper linking command here.
>>
>>>
>>>  checkasm: $(CHECKASM)
>>>
>>>
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> Try patchset without it and than complain instead.

I'm not saying it's not needed, I'm saying it's not the correct way to
fix the linking failure the following patches would introduce.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[Paul B Mahol]

On 3/18/18, James Almer  wrote:
> On 3/18/2018 10:40 AM, j...@itanimul.li wrote:
>> From: Josh de Kock 
>>
>> ---
>>  tests/checkasm/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
>> index 0520e264e2..ae7e810d25 100644
>> --- a/tests/checkasm/Makefile
>> +++ b/tests/checkasm/Makefile
>> @@ -61,7 +61,7 @@ tests/checkasm/checkasm.o: CFLAGS += -Umain
>>  CHECKASM := tests/checkasm/checkasm$(EXESUF)
>>
>>  $(CHECKASM): $(CHECKASMOBJS) $(FF_STATIC_DEP_LIBS)
>> -$(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
>> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
>> $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)
>> +$(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS)
>> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter)
>> $(EXTRALIBS-avformat) $(EXTRALIBS-avutil) $(EXTRALIBS-swresample)
>> $(EXTRALIBS)
>
> This is not the correct fix. We currently only need to link to avcodec,
> avfilter and avutil here, which may or may not depend on other fflibs.
> Seeing swresample there already hints that this line got duct tape fixes
> as linking failures popped up. How long until someone gets a linking
> failure pointing to swscale or postproc, seeing avfilter may link to it
> depending on enabled filters, much like it does with avformat?
>
> The *_FFLIBS variables in config.mak list these dependencies per lib,
> and should be used to assemble a proper linking command here.
>
>>
>>  checkasm: $(CHECKASM)
>>
>>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Try patchset without it and than complain instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[James Almer]

On 3/18/2018 10:40 AM, j...@itanimul.li wrote:
> From: Josh de Kock 
> 
> ---
>  tests/checkasm/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 0520e264e2..ae7e810d25 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -61,7 +61,7 @@ tests/checkasm/checkasm.o: CFLAGS += -Umain
>  CHECKASM := tests/checkasm/checkasm$(EXESUF)
>  
>  $(CHECKASM): $(CHECKASMOBJS) $(FF_STATIC_DEP_LIBS)
> - $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS) 
> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter) 
> $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)
> + $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $(CHECKASMOBJS) 
> $(FF_STATIC_DEP_LIBS) $(EXTRALIBS-avcodec) $(EXTRALIBS-avfilter) 
> $(EXTRALIBS-avformat) $(EXTRALIBS-avutil) $(EXTRALIBS-swresample) $(EXTRALIBS)

This is not the correct fix. We currently only need to link to avcodec,
avfilter and avutil here, which may or may not depend on other fflibs.
Seeing swresample there already hints that this line got duct tape fixes
as linking failures popped up. How long until someone gets a linking
failure pointing to swscale or postproc, seeing avfilter may link to it
depending on enabled filters, much like it does with avformat?

The *_FFLIBS variables in config.mak list these dependencies per lib,
and should be used to assemble a proper linking command here.

>  
>  checkasm: $(CHECKASM)
>  
> 

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

Re: [FFmpeg-devel] [PATCH 1/4] checkasm/Makefile: add EXTRALIBS-libavformat

[Carl Eugen Hoyos]

2018-03-18 14:40 GMT+01:00, j...@itanimul.li :
> From: Josh de Kock 

You could add a line that explains for which configure-option this
fixes something.
(I guess there is a configure option that needs this.)

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