Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On date Tuesday 2023-09-05 12:43:35 +, ffmpeg-devel Mailing List wrote: [...] > On date Saturday 2023-09-02 09:20:08 +, Carotti, Elias wrote: > > On Thu, 2023-08-31 at 19:09 +0200, Stefano Sabatini wrote: > > > > > > > > > > > > In particular why are you turning on fast_pskip silently based on > > > > a completely different setting? > > > > > > The patch is fixing the regression introduced by the unconditional > > > setting of b_fast_pskip. > > > > > > Now the question is if it makes sense to set mb_info without > > > b_fast_pskip (in both case this should be probably documented). > > > > > > @Elias can you comment about the mb_info/b_fast_pskip use case? > > > > Sorry again for the delay in responding. > > We can safely remove it altogether. It's true we don't need to set it > > along with mb_info. > > However, it doesn't do any harm, since fast_pskip is by default set to > > true by libx264 and only turned off either explicitly by the user, or > > when using the placebo preset, or when doing lossless encoding > > (constant QP == 0.) So, I agree, let's remove these three lines. > > > > Thanks, updated. > > Ok for me Applied, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
-Original Message- From: ffmpeg-devel On Behalf Of Stefano Sabatini Sent: Saturday, September 2, 2023 5:45 PM To: ffmpeg-devel@ffmpeg.org Cc: kier...@obe.tv; Carotti, Elias Subject: RE: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. On date Saturday 2023-09-02 09:20:08 +, Carotti, Elias wrote: > On Thu, 2023-08-31 at 19:09 +0200, Stefano Sabatini wrote: > > > > > > > > In particular why are you turning on fast_pskip silently based on > > > a completely different setting? > > > > The patch is fixing the regression introduced by the unconditional > > setting of b_fast_pskip. > > > > Now the question is if it makes sense to set mb_info without > > b_fast_pskip (in both case this should be probably documented). > > > > @Elias can you comment about the mb_info/b_fast_pskip use case? > > Sorry again for the delay in responding. > We can safely remove it altogether. It's true we don't need to set it > along with mb_info. > However, it doesn't do any harm, since fast_pskip is by default set to > true by libx264 and only turned off either explicitly by the user, or > when using the placebo preset, or when doing lossless encoding > (constant QP == 0.) So, I agree, let's remove these three lines. Thanks, updated. Ok for me Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On date Saturday 2023-09-02 09:20:08 +, Carotti, Elias wrote: > On Thu, 2023-08-31 at 19:09 +0200, Stefano Sabatini wrote: > > > > > > > > In particular why are you turning on fast_pskip silently based on a > > > completely different setting? > > > > The patch is fixing the regression introduced by the unconditional > > setting of b_fast_pskip. > > > > Now the question is if it makes sense to set mb_info without > > b_fast_pskip (in both case this should be probably documented). > > > > @Elias can you comment about the mb_info/b_fast_pskip use case? > > Sorry again for the delay in responding. > We can safely remove it altogether. It's true we don't need to set it > along with mb_info. > However, it doesn't do any harm, since fast_pskip is by default set to > true by libx264 and only turned off either explicitly by the user, or > when using the placebo preset, or when doing lossless encoding > (constant QP == 0.) > So, I agree, let's remove these three lines. Thanks, updated. >From ceb00a939ab2cd0fe146020b0e4e0d80b5d16a5d Mon Sep 17 00:00:00 2001 From: Stefano Sabatini Date: Fri, 25 Aug 2023 11:35:01 +0200 Subject: [PATCH] lavc/libx264: do not unconditionally set x4->params.analyse.b_fast_pskip Fix output change regression introduced in 418c954e318. --- libavcodec/libx264.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index ce849d6c9a..cc5e1ba5b1 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -1190,7 +1190,6 @@ FF_ENABLE_DEPRECATION_WARNINGS } x4->params.analyse.b_mb_info = x4->mb_info; -x4->params.analyse.b_fast_pskip = 1; // update AVCodecContext with x264 parameters avctx->has_b_frames = x4->params.i_bframe ? -- 2.34.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On Thu, 2023-08-31 at 19:09 +0200, Stefano Sabatini wrote: > > > > In particular why are you turning on fast_pskip silently based on a > > completely different setting? > > The patch is fixing the regression introduced by the unconditional > setting of b_fast_pskip. > > Now the question is if it makes sense to set mb_info without > b_fast_pskip (in both case this should be probably documented). > > @Elias can you comment about the mb_info/b_fast_pskip use case? Sorry again for the delay in responding. We can safely remove it altogether. It's true we don't need to set it along with mb_info. However, it doesn't do any harm, since fast_pskip is by default set to true by libx264 and only turned off either explicitly by the user, or when using the placebo preset, or when doing lossless encoding (constant QP == 0.) So, I agree, let's remove these three lines. Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On date Thursday 2023-08-31 15:38:09 +0100, Kieran Kunhya wrote:
> On Thu, 31 Aug 2023 at 15:30, Kieran Kunhya wrote:
>
> >
> >
> > On Thu, 31 Aug 2023 at 15:12, Carotti, Elias via ffmpeg-devel <
> > ffmpeg-devel@ffmpeg.org> wrote:
> >
> >> Hi
> >>
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of
> >> Stefano Sabatini
> >> Sent: Friday, August 25, 2023 12:01 PM
> >> To: FFmpeg development discussions and patches
> >> Cc: Stefano Sabatini
> >> Subject: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable
> >> x4->params.analyse.b_fast_pskip if mb_info is set
> >>
> >> CAUTION: This email originated from outside of the organization. Do not
> >> click links or open attachments unless you can confirm the sender and know
> >> the content is safe.
> >>
> >>
> >>
> >> x4->params.analyse.b_fast_pskip should only be forced in case mb_info
> >> is set.
> >>
> >> Fix output change introduced in 418c954e318.
> >> ---
> >> libavcodec/libx264.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
> >> 1a7dc7bdd5..a2877d7f75 100644
> >> --- a/libavcodec/libx264.c
> >> +++ b/libavcodec/libx264.c
> >> @@ -1190,7 +1190,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >> }
> >>
> >> x4->params.analyse.b_mb_info = x4->mb_info;
> >> -x4->params.analyse.b_fast_pskip = 1;
> >> +if (x4->mb_info) {
> >> +x4->params.analyse.b_fast_pskip = x4->mb_info;
> >> +}
> >>
> >> // update AVCodecContext with x264 parameters
> >> avctx->has_b_frames = x4->params.i_bframe ?
> >> --
> >> 2.25.1
> >>
> >>
> >> Sorry for the delay. I agree, this was missing in the patch.
> >> Best
> >> Elias
> >>
> >
> > What does this patch actually do?
> >
> > Kieran
> >
>
> In particular why are you turning on fast_pskip silently based on a
> completely different setting?
The patch is fixing the regression introduced by the unconditional
setting of b_fast_pskip.
Now the question is if it makes sense to set mb_info without
b_fast_pskip (in both case this should be probably documented).
@Elias can you comment about the mb_info/b_fast_pskip use case?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On Thu, 31 Aug 2023 at 15:30, Kieran Kunhya wrote:
>
>
> On Thu, 31 Aug 2023 at 15:12, Carotti, Elias via ffmpeg-devel <
> ffmpeg-devel@ffmpeg.org> wrote:
>
>> Hi
>>
>> -Original Message-
>> From: ffmpeg-devel On Behalf Of
>> Stefano Sabatini
>> Sent: Friday, August 25, 2023 12:01 PM
>> To: FFmpeg development discussions and patches
>> Cc: Stefano Sabatini
>> Subject: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable
>> x4->params.analyse.b_fast_pskip if mb_info is set
>>
>> CAUTION: This email originated from outside of the organization. Do not
>> click links or open attachments unless you can confirm the sender and know
>> the content is safe.
>>
>>
>>
>> x4->params.analyse.b_fast_pskip should only be forced in case mb_info
>> is set.
>>
>> Fix output change introduced in 418c954e318.
>> ---
>> libavcodec/libx264.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
>> 1a7dc7bdd5..a2877d7f75 100644
>> --- a/libavcodec/libx264.c
>> +++ b/libavcodec/libx264.c
>> @@ -1190,7 +1190,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> }
>>
>> x4->params.analyse.b_mb_info = x4->mb_info;
>> -x4->params.analyse.b_fast_pskip = 1;
>> +if (x4->mb_info) {
>> +x4->params.analyse.b_fast_pskip = x4->mb_info;
>> +}
>>
>> // update AVCodecContext with x264 parameters
>> avctx->has_b_frames = x4->params.i_bframe ?
>> --
>> 2.25.1
>>
>>
>> Sorry for the delay. I agree, this was missing in the patch.
>> Best
>> Elias
>>
>
> What does this patch actually do?
>
> Kieran
>
In particular why are you turning on fast_pskip silently based on a
completely different setting?
Kieran
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On Thu, 31 Aug 2023 at 15:12, Carotti, Elias via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:
> Hi
>
> -Original Message-
> From: ffmpeg-devel On Behalf Of Stefano
> Sabatini
> Sent: Friday, August 25, 2023 12:01 PM
> To: FFmpeg development discussions and patches
> Cc: Stefano Sabatini
> Subject: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable
> x4->params.analyse.b_fast_pskip if mb_info is set
>
> CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and know
> the content is safe.
>
>
>
> x4->params.analyse.b_fast_pskip should only be forced in case mb_info
> is set.
>
> Fix output change introduced in 418c954e318.
> ---
> libavcodec/libx264.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
> 1a7dc7bdd5..a2877d7f75 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -1190,7 +1190,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> }
>
> x4->params.analyse.b_mb_info = x4->mb_info;
> -x4->params.analyse.b_fast_pskip = 1;
> +if (x4->mb_info) {
> +x4->params.analyse.b_fast_pskip = x4->mb_info;
> +}
>
> // update AVCodecContext with x264 parameters
> avctx->has_b_frames = x4->params.i_bframe ?
> --
> 2.25.1
>
>
> Sorry for the delay. I agree, this was missing in the patch.
> Best
> Elias
>
What does this patch actually do?
Kieran
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
Hi
-Original Message-
From: ffmpeg-devel On Behalf Of Stefano
Sabatini
Sent: Friday, August 25, 2023 12:01 PM
To: FFmpeg development discussions and patches
Cc: Stefano Sabatini
Subject: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable
x4->params.analyse.b_fast_pskip if mb_info is set
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
x4->params.analyse.b_fast_pskip should only be forced in case mb_info
is set.
Fix output change introduced in 418c954e318.
---
libavcodec/libx264.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
1a7dc7bdd5..a2877d7f75 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -1190,7 +1190,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
x4->params.analyse.b_mb_info = x4->mb_info;
-x4->params.analyse.b_fast_pskip = 1;
+if (x4->mb_info) {
+x4->params.analyse.b_fast_pskip = x4->mb_info;
+}
// update AVCodecContext with x264 parameters
avctx->has_b_frames = x4->params.i_bframe ?
--
2.25.1
Sorry for the delay. I agree, this was missing in the patch.
Best
Elias
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with
subject "unsubscribe".
NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese
di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR
i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
x4->params.analyse.b_fast_pskip should only be forced in case mb_info
is set.
Fix output change introduced in 418c954e318.
---
libavcodec/libx264.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 1a7dc7bdd5..a2877d7f75 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -1190,7 +1190,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
x4->params.analyse.b_mb_info = x4->mb_info;
-x4->params.analyse.b_fast_pskip = 1;
+if (x4->mb_info) {
+x4->params.analyse.b_fast_pskip = x4->mb_info;
+}
// update AVCodecContext with x264 parameters
avctx->has_b_frames = x4->params.i_bframe ?
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
