Re: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign check for hwupload

2019-04-11 Thread Song, Ruiling


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Fu, Linjie
> Sent: Thursday, April 11, 2019 3:59 PM
> To: Li, Zhong ; FFmpeg development discussions and
> patches 
> Subject: Re: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> check for hwupload
> 
> > -Original Message-
> > From: Li, Zhong
> > Sent: Thursday, April 11, 2019 10:51
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Cc: Fu, Linjie 
> > Subject: RE: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> > check for hwupload
> >
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > Behalf
> > > Of Linjie Fu
> > > Sent: Wednesday, April 10, 2019 7:56 PM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Fu, Linjie 
> > > Subject: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> > > check for hwupload
> > >
> > > Fix the aligned check in hwupload, input surface should be 16 aligned too.
> > >
> > > Fix #7830.
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > >
> > >  libavutil/hwcontext_qsv.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> > > b6d8bfe2bf..8b000fe636 100644
> > > --- a/libavutil/hwcontext_qsv.c
> > > +++ b/libavutil/hwcontext_qsv.c
> > > @@ -892,7 +892,8 @@ static int
> > > qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> > >  return ret;
> > >
> > >
> > > -if (src->height & 16 || src->linesize[0] & 16) {
> > > +if (src->height & 15 || src->width & 15 ||
> > > +src->linesize[0] & 15) {
> >
> > Should be better to use FFALIGN()
> >
> > Another question is it really necessary to check width alignment if we 
> > already
> > checked linesize to fix this issue?
> > (I guess it it not necessary, and if it is needed, many other places 
> > probably
> > needed to be changed too.)
> 
> Checked the code in qsvvpp.c and qsvenc.c, it has the same check using &:
> 
> libavcodec/qsvenc.c: if ((frame->height & 31 || frame->linesize[0] & 
> (q-
> >width_align - 1)) ||
> libavfilter/qsvvpp.c:if (picref->height & 31 || picref->linesize[0] & 
> 31) {
> 
> These should be matched, so the FFALIGN is better for all?
I think FFALIGN() is used to align a value not checking alignment.
> 
> For the width check, added for redundant check, can be removed to match the
> whole behavior.
only checking linesize[0] sounds good.
> 
> Thanks for comments.
> 
> ___
> 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 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, v2] lavu/hwcontext_qsv: Fix the realign check for hwupload

2019-04-11 Thread Fu, Linjie
> -Original Message-
> From: Li, Zhong
> Sent: Thursday, April 11, 2019 10:51
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: RE: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> check for hwupload
> 
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of Linjie Fu
> > Sent: Wednesday, April 10, 2019 7:56 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie 
> > Subject: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> > check for hwupload
> >
> > Fix the aligned check in hwupload, input surface should be 16 aligned too.
> >
> > Fix #7830.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >
> >  libavutil/hwcontext_qsv.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> > b6d8bfe2bf..8b000fe636 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -892,7 +892,8 @@ static int
> > qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> >  return ret;
> >
> >
> > -if (src->height & 16 || src->linesize[0] & 16) {
> > +if (src->height & 15 || src->width & 15 ||
> > +src->linesize[0] & 15) {
> 
> Should be better to use FFALIGN()
> 
> Another question is it really necessary to check width alignment if we already
> checked linesize to fix this issue?
> (I guess it it not necessary, and if it is needed, many other places probably
> needed to be changed too.)

Checked the code in qsvvpp.c and qsvenc.c, it has the same check using &:

libavcodec/qsvenc.c: if ((frame->height & 31 || frame->linesize[0] & 
(q->width_align - 1)) ||
libavfilter/qsvvpp.c:if (picref->height & 31 || picref->linesize[0] & 
31) {

These should be matched, so the FFALIGN is better for all?

For the width check, added for redundant check, can be removed to match the 
whole behavior.

Thanks for comments.
 
___
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, v2] lavu/hwcontext_qsv: Fix the realign check for hwupload

2019-04-11 Thread Hendrik Leppkes
On Thu, Apr 11, 2019 at 4:51 AM Li, Zhong  wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Linjie Fu
> > Sent: Wednesday, April 10, 2019 7:56 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie 
> > Subject: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> > check for hwupload
> >
> > Fix the aligned check in hwupload, input surface should be 16 aligned too.
> >
> > Fix #7830.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >
> >  libavutil/hwcontext_qsv.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> > b6d8bfe2bf..8b000fe636 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -892,7 +892,8 @@ static int
> > qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> >  return ret;
> >
> >
> > -if (src->height & 16 || src->linesize[0] & 16) {
> > +if (src->height & 15 || src->width & 15 ||
> > +src->linesize[0] & 15) {
>
> Should be better to use FFALIGN()

Since you are checking alignment here, the manual checks should be
fine. FFALIGN is for creating aligned values. You could use something
like "if (src->height != FFALIGN(src->height, 16))", but that doesn't
really seem clearer to me.

>
> Another question is it really necessary to check width alignment if we 
> already checked linesize to fix this issue?
> (I guess it it not necessary, and if it is needed, many other places probably 
> needed to be changed too.)

I agree, linesize should be enough, and width alignment probably not needed.

- Hendrik
___
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, v2] lavu/hwcontext_qsv: Fix the realign check for hwupload

2019-04-10 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Linjie Fu
> Sent: Wednesday, April 10, 2019 7:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie 
> Subject: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> check for hwupload
> 
> Fix the aligned check in hwupload, input surface should be 16 aligned too.
> 
> Fix #7830.
> 
> Signed-off-by: Linjie Fu 
> ---
> 
>  libavutil/hwcontext_qsv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> b6d8bfe2bf..8b000fe636 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -892,7 +892,8 @@ static int
> qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
>  return ret;
> 
> 
> -if (src->height & 16 || src->linesize[0] & 16) {
> +if (src->height & 15 || src->width & 15 ||
> +src->linesize[0] & 15) {

Should be better to use FFALIGN()

Another question is it really necessary to check width alignment if we already 
checked linesize to fix this issue?
(I guess it it not necessary, and if it is needed, many other places probably 
needed to be changed too.)
___
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, v2] lavu/hwcontext_qsv: Fix the realign check for hwupload

2019-04-10 Thread Linjie Fu
Fix the aligned check in hwupload, input surface should be 16 aligned
too.

Fix #7830.

Signed-off-by: Linjie Fu 
---

 libavutil/hwcontext_qsv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index b6d8bfe2bf..8b000fe636 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -892,7 +892,8 @@ static int qsv_transfer_data_to(AVHWFramesContext *ctx, 
AVFrame *dst,
 return ret;
 
 
-if (src->height & 16 || src->linesize[0] & 16) {
+if (src->height & 15 || src->width & 15 ||
+src->linesize[0] & 15) {
 realigned = 1;
 memset(_frame, 0, sizeof(tmp_frame));
 tmp_frame.format = src->format;
-- 
2.17.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".