Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-10 Thread Paul B Mahol 
On 12/10/17, Nicolas George  wrote:
> Paul B Mahol (2017-12-10):
>> See vf_aspect.c, it changes inlink parameters.
>>
>> This patch is required for example if one change sar of first input
>> and that value is not propagated to next links.
>
> I had not noticed that. The code in vf_aspect.c looks invalid to me, I
> think the change should be done on outlink, and in the outpad's
> config_prop's callback.

Yes its looks fishy at best, will change and test it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-10 Thread Nicolas George 
Paul B Mahol (2017-12-10):
> See vf_aspect.c, it changes inlink parameters.
> 
> This patch is required for example if one change sar of first input
> and that value is not propagated to next links.

I had not noticed that. The code in vf_aspect.c looks invalid to me, I
think the change should be done on outlink, and in the outpad's
config_prop's callback.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-10 Thread Paul B Mahol 
On 12/10/17, Nicolas George  wrote:
> Paul B Mahol (2017-12-10):
>> > "May need" is not always a good way of designing changes. It would be
>> > much better if you actually changed these filters at the same time to
>> > use it. That way you can be more sure that you did not mess things up in
>> > a subtle way.
>> Is changing inlink parameters valid at all?
>
> I do not understand the question.

See vf_aspect.c, it changes inlink parameters.

This patch is required for example if one change sar of first input
and that value
is not propagated to next links.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-10 Thread Nicolas George 
Paul B Mahol (2017-12-10):
> > "May need" is not always a good way of designing changes. It would be
> > much better if you actually changed these filters at the same time to
> > use it. That way you can be more sure that you did not mess things up in
> > a subtle way.
> Is changing inlink parameters valid at all?

I do not understand the question.

Regards,

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

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-10 Thread Paul B Mahol 
On 12/10/17, Nicolas George  wrote:
> Paul B Mahol (2017-12-09):
>> No new patches require it, it is for case such stereo3d and setsar and
>> bunch
>> of others that may need to explicitly set sample aspect ratio to each
>> frame.
>
> "May need" is not always a good way of designing changes. It would be
> much better if you actually changed these filters at the same time to
> use it. That way you can be more sure that you did not mess things up in
> a subtle way.

Is changing inlink parameters valid at all?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-10 Thread Nicolas George 
Paul B Mahol (2017-12-09):
> No new patches require it, it is for case such stereo3d and setsar and bunch
> of others that may need to explicitly set sample aspect ratio to each frame.

"May need" is not always a good way of designing changes. It would be
much better if you actually changed these filters at the same time to
use it. That way you can be more sure that you did not mess things up in
a subtle way.

> Do you work in White House or in nuclear power plant?

You do not know anything about the use of my time, as I know nothing
about yours. Therefore, kindly refrain from making any assumptions about
me, as I refrain from making assumptions about you.

You frequently commit changes too soon. A newcomer asked about
reasonable delay not so long ago, someone answered quoting one week
before insisting, except if the patch has some kind of urgency. It seems
reasonable to me.

And it applies to people who have commit rights too.

One week.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-09 Thread Paul B Mahol 
On 12/9/17, Nicolas George  wrote:
> Paul B Mahol (2017-12-09):
>> >> It should not be needed for each filter that sets sample aspect ratio
>> >> to set it explicitly also for each and every frame, instead that is
>> >> automatically done in get_buffer call.
>> > Is this message ok? If yes then I will push this patch shortly.
>
> Ok, assuming you put it in the commit message. I think it would be
> better to accompany it with patches that actually require the change,
> though.

No new patches require it, it is for case such stereo3d and setsar and bunch
of others that may need to explicitly set sample aspect ratio to each frame.

>
>> Will push very soon.
>
> PLEASE STOP DOING THAT.
>
> Waiting less than 48 hours is unacceptably short for that kind of change
> when there is a discussion. I had to stop a more urgent task to answer
> right now, and that annoys me a lot.

Do you work in White House or in nuclear power plant?

>
> YOU ARE NOT ALONE IN THIS PROJECT, AND PEOPLE DO NOT HAVE THE SAME
> SCHEDULE AS YOU.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-09 Thread Nicolas George 
Paul B Mahol (2017-12-09):
> >> It should not be needed for each filter that sets sample aspect ratio
> >> to set it explicitly also for each and every frame, instead that is
> >> automatically done in get_buffer call.
> > Is this message ok? If yes then I will push this patch shortly.

Ok, assuming you put it in the commit message. I think it would be
better to accompany it with patches that actually require the change,
though.

> Will push very soon.

PLEASE STOP DOING THAT.

Waiting less than 48 hours is unacceptably short for that kind of change
when there is a discussion. I had to stop a more urgent task to answer
right now, and that annoys me a lot.

YOU ARE NOT ALONE IN THIS PROJECT, AND PEOPLE DO NOT HAVE THE SAME
SCHEDULE AS YOU.

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-09 Thread Paul B Mahol 
On 12/8/17, Paul B Mahol  wrote:
> On 12/7/17, Paul B Mahol  wrote:
>> On 12/7/17, Nicolas George  wrote:
>>> Paul B Mahol (2017-12-07):
 Signed-off-by: Paul B Mahol 
 ---
  libavfilter/video.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> Please explain (in the commit message) why you think this patch is
>>> needed.
>>
>> It should not be needed for each filter that sets sample aspect ratio
>> to set it explicitly also for each and every frame, instead that is
>> automatically done in get_buffer call.
>>
>
> Is this message ok? If yes then I will push this patch shortly.
>

Will push very soon.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-08 Thread Paul B Mahol 
On 12/7/17, Paul B Mahol  wrote:
> On 12/7/17, Nicolas George  wrote:
>> Paul B Mahol (2017-12-07):
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavfilter/video.c | 9 -
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> Please explain (in the commit message) why you think this patch is
>> needed.
>
> It should not be needed for each filter that sets sample aspect ratio
> to set it explicitly also for each and every frame, instead that is
> automatically done in get_buffer call.
>

Is this message ok? If yes then I will push this patch shortly.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-07 Thread Paul B Mahol 
On 12/7/17, Nicolas George  wrote:
> Paul B Mahol (2017-12-07):
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavfilter/video.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> Please explain (in the commit message) why you think this patch is
> needed.

It should not be needed for each filter that sets sample aspect ratio
to set it explicitly also for each and every frame, instead that is
automatically done in get_buffer call.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-07 Thread Nicolas George 
Paul B Mahol (2017-12-07):
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/video.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

Please explain (in the commit message) why you think this patch is
needed.

Regards,

-- 
  Nicolas George


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

[FFmpeg-devel] [PATCH] avfilter/video: pick sar from link

2017-12-07 Thread Paul B Mahol 
Signed-off-by: Paul B Mahol 
---
 libavfilter/video.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavfilter/video.c b/libavfilter/video.c
index 6f9020b9fe..7a8e587798 100644
--- a/libavfilter/video.c
+++ b/libavfilter/video.c
@@ -43,6 +43,7 @@ AVFrame *ff_null_get_video_buffer(AVFilterLink *link, int w, 
int h)
 
 AVFrame *ff_default_get_video_buffer(AVFilterLink *link, int w, int h)
 {
+AVFrame *frame = NULL;
 int pool_width = 0;
 int pool_height = 0;
 int pool_align = 0;
@@ -86,7 +87,13 @@ AVFrame *ff_default_get_video_buffer(AVFilterLink *link, int 
w, int h)
 }
 }
 
-return ff_frame_pool_get(link->frame_pool);
+frame = ff_frame_pool_get(link->frame_pool);
+if (!frame)
+return NULL;
+
+frame->sample_aspect_ratio = link->sample_aspect_ratio;
+
+return frame;
 }
 
 AVFrame *ff_get_video_buffer(AVFilterLink *link, int w, int h)
-- 
2.11.0

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