Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-30 Thread Yan Wang 


On 5/30/2019 7:39 AM, Mark Thompson wrote:

On 28/05/2019 08:46, Yan Wang wrote:

On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:

On Tue, May 28, 2019 at 8:57 AM Yan Wang  wrote:

When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.

Signed-off-by: Yan Wang 
---
   libavcodec/decode.c | 6 ++
   1 file changed, 6 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)

   for (;;) {
   // Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
   hwaccel_uninit(avctx);
+#endif

   user_choice = avctx->get_format(avctx, choices);
   if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
  "missing configuration.\n", desc->name);
   goto try_again;
   }
+#if CONFIG_VP9_VAAPI_HWACCEL
+    if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
   if (hw_config->hwaccel) {
+#endif
   av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
  "initialisation.\n", desc->name);
   err = hwaccel_init(avctx, hw_config);
--
2.17.2


This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.

Sorry for this. I am new guy for ffmpeg development. What way should be

better? I can refine it.


Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?

Based on VP9 spec, the frame resolution can be changed per frame. But current

frame will need refer to previous frame still. So if destroy the VAAPI context, 
it

will cause reference frame surface in VAAPI driver lost.

In fact, this patch is for the issue:

https://github.com/intel/media-driver/issues/629

its 2nd frame (128x128) will refer to the 1st frame (256x256).

Can you explain exactly what is going wrong here?  The surface is definitely 
still present - the reference frame list entry holds a reference to it.


IMHO,

for one VP9 clips with dynamic resolution changing which is legal based 
on VP9 spec.


After the 1st frame (256x256) is decoded with vaapi-hw acceleration, 
ffmpeg will parse and decode the frame header of the 2nd frame (128x128).


It will call update_size() when ffmpeg finds the resolution is changed 
and destroy/recreate vaapi-hw context which will also destroy and 
release the previous decoded 1st frame (256x256) saved in driver. The 
saved 1st frame surface should be as reference frame normally.


So when ffmpeg ask vaapi-hw to decode the 2nd frame, it cannot find the 
the 1st frame surface as reference frame and has to use dummy reference 
frame although the reference frame index is right.


I am newbie for ffmpeg code. I am studying ffmpeg related code and hope 
to compare libvpx path for looking for a better solution.


Thanks.

Yan Wang



- Mark
___
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] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-29 Thread Mark Thompson 
On 28/05/2019 08:46, Yan Wang wrote:
> 
> On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:
>> On Tue, May 28, 2019 at 8:57 AM Yan Wang  wrote:
>>> When the format change, the VAAPI context cannot be destroyed.
>>> Otherwise, the reference frame surface will lost.
>>>
>>> Signed-off-by: Yan Wang 
>>> ---
>>>   libavcodec/decode.c | 6 ++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>>> index 6c31166ec2..3eda1dc42c 100644
>>> --- a/libavcodec/decode.c
>>> +++ b/libavcodec/decode.c
>>> @@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
>>> AVPixelFormat *fmt)
>>>
>>>   for (;;) {
>>>   // Remove the previous hwaccel, if there was one.
>>> +#if !CONFIG_VP9_VAAPI_HWACCEL
>>>   hwaccel_uninit(avctx);
>>> +#endif
>>>
>>>   user_choice = avctx->get_format(avctx, choices);
>>>   if (user_choice == AV_PIX_FMT_NONE) {
>>> @@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
>>> AVPixelFormat *fmt)
>>>  "missing configuration.\n", desc->name);
>>>   goto try_again;
>>>   }
>>> +#if CONFIG_VP9_VAAPI_HWACCEL
>>> +    if (hw_config->hwaccel && !avctx->hwaccel) {
>>> +#else
>>>   if (hw_config->hwaccel) {
>>> +#endif
>>>   av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
>>>  "initialisation.\n", desc->name);
>>>   err = hwaccel_init(avctx, hw_config);
>>> -- 
>>> 2.17.2
>>>
>> This change feels just wrong. First of all, preprocessors are
>> absolutely the wrong way to go about this.
> 
> Sorry for this. I am new guy for ffmpeg development. What way should be
> 
> better? I can refine it.
> 
>> Secondly, if the frames need to change size, or surface format, then
>> this absolutely needs to be called, doesn't it?
> 
> Based on VP9 spec, the frame resolution can be changed per frame. But current
> 
> frame will need refer to previous frame still. So if destroy the VAAPI 
> context, it
> 
> will cause reference frame surface in VAAPI driver lost.
> 
> In fact, this patch is for the issue:
> 
> https://github.com/intel/media-driver/issues/629
> 
> its 2nd frame (128x128) will refer to the 1st frame (256x256).

Can you explain exactly what is going wrong here?  The surface is definitely 
still present - the reference frame list entry holds a reference to it.

- Mark
___
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] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-28 Thread Yan Wang 


On 5/28/2019 4:43 PM, Hendrik Leppkes wrote:

On Tue, May 28, 2019 at 9:46 AM Yan Wang  wrote:


On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:

On Tue, May 28, 2019 at 8:57 AM Yan Wang  wrote:

When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.

Signed-off-by: Yan Wang 
---
   libavcodec/decode.c | 6 ++
   1 file changed, 6 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)

   for (;;) {
   // Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
   hwaccel_uninit(avctx);
+#endif

   user_choice = avctx->get_format(avctx, choices);
   if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
  "missing configuration.\n", desc->name);
   goto try_again;
   }
+#if CONFIG_VP9_VAAPI_HWACCEL
+if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
   if (hw_config->hwaccel) {
+#endif
   av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
  "initialisation.\n", desc->name);
   err = hwaccel_init(avctx, hw_config);
--
2.17.2


This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.

Sorry for this. I am new guy for ffmpeg development. What way should be

better? I can refine it.


Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?

Based on VP9 spec, the frame resolution can be changed per frame. But
current

frame will need refer to previous frame still. So if destroy the VAAPI
context, it

will cause reference frame surface in VAAPI driver lost.

In fact, this patch is for the issue:

https://github.com/intel/media-driver/issues/629

its 2nd frame (128x128) will refer to the 1st frame (256x256).


This may work if the frame size decreases, but what if it increases?
Then the frame buffers in the pool are too small, and anything could
go wrong.
This won't be an easy issue to solve, and needs very careful design.


Agree. I should add [RFC] for this patch.

I can investigate frame buffer management of ffmpeg and submit new patch 
for covering this situation.


Thanks for comments.

Yan Wang



- 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".

___
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] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-28 Thread Hendrik Leppkes 
On Tue, May 28, 2019 at 9:46 AM Yan Wang  wrote:
>
>
> On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:
> > On Tue, May 28, 2019 at 8:57 AM Yan Wang  wrote:
> >> When the format change, the VAAPI context cannot be destroyed.
> >> Otherwise, the reference frame surface will lost.
> >>
> >> Signed-off-by: Yan Wang 
> >> ---
> >>   libavcodec/decode.c | 6 ++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> >> index 6c31166ec2..3eda1dc42c 100644
> >> --- a/libavcodec/decode.c
> >> +++ b/libavcodec/decode.c
> >> @@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
> >> AVPixelFormat *fmt)
> >>
> >>   for (;;) {
> >>   // Remove the previous hwaccel, if there was one.
> >> +#if !CONFIG_VP9_VAAPI_HWACCEL
> >>   hwaccel_uninit(avctx);
> >> +#endif
> >>
> >>   user_choice = avctx->get_format(avctx, choices);
> >>   if (user_choice == AV_PIX_FMT_NONE) {
> >> @@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
> >> AVPixelFormat *fmt)
> >>  "missing configuration.\n", desc->name);
> >>   goto try_again;
> >>   }
> >> +#if CONFIG_VP9_VAAPI_HWACCEL
> >> +if (hw_config->hwaccel && !avctx->hwaccel) {
> >> +#else
> >>   if (hw_config->hwaccel) {
> >> +#endif
> >>   av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
> >>  "initialisation.\n", desc->name);
> >>   err = hwaccel_init(avctx, hw_config);
> >> --
> >> 2.17.2
> >>
> > This change feels just wrong. First of all, preprocessors are
> > absolutely the wrong way to go about this.
>
> Sorry for this. I am new guy for ffmpeg development. What way should be
>
> better? I can refine it.
>
> > Secondly, if the frames need to change size, or surface format, then
> > this absolutely needs to be called, doesn't it?
>
> Based on VP9 spec, the frame resolution can be changed per frame. But
> current
>
> frame will need refer to previous frame still. So if destroy the VAAPI
> context, it
>
> will cause reference frame surface in VAAPI driver lost.
>
> In fact, this patch is for the issue:
>
> https://github.com/intel/media-driver/issues/629
>
> its 2nd frame (128x128) will refer to the 1st frame (256x256).
>

This may work if the frame size decreases, but what if it increases?
Then the frame buffers in the pool are too small, and anything could
go wrong.
This won't be an easy issue to solve, and needs very careful design.

- 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] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-28 Thread Yan Wang 


On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:

On Tue, May 28, 2019 at 8:57 AM Yan Wang  wrote:

When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.

Signed-off-by: Yan Wang 
---
  libavcodec/decode.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)

  for (;;) {
  // Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
  hwaccel_uninit(avctx);
+#endif

  user_choice = avctx->get_format(avctx, choices);
  if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
 "missing configuration.\n", desc->name);
  goto try_again;
  }
+#if CONFIG_VP9_VAAPI_HWACCEL
+if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
  if (hw_config->hwaccel) {
+#endif
  av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
 "initialisation.\n", desc->name);
  err = hwaccel_init(avctx, hw_config);
--
2.17.2


This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.


Sorry for this. I am new guy for ffmpeg development. What way should be

better? I can refine it.


Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?


Based on VP9 spec, the frame resolution can be changed per frame. But 
current


frame will need refer to previous frame still. So if destroy the VAAPI 
context, it


will cause reference frame surface in VAAPI driver lost.

In fact, this patch is for the issue:

https://github.com/intel/media-driver/issues/629

its 2nd frame (128x128) will refer to the 1st frame (256x256).

Thanks.

Yan Wang



- 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".

___
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] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-28 Thread Hendrik Leppkes 
On Tue, May 28, 2019 at 8:57 AM Yan Wang  wrote:
>
> When the format change, the VAAPI context cannot be destroyed.
> Otherwise, the reference frame surface will lost.
>
> Signed-off-by: Yan Wang 
> ---
>  libavcodec/decode.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 6c31166ec2..3eda1dc42c 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
> AVPixelFormat *fmt)
>
>  for (;;) {
>  // Remove the previous hwaccel, if there was one.
> +#if !CONFIG_VP9_VAAPI_HWACCEL
>  hwaccel_uninit(avctx);
> +#endif
>
>  user_choice = avctx->get_format(avctx, choices);
>  if (user_choice == AV_PIX_FMT_NONE) {
> @@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
> AVPixelFormat *fmt)
> "missing configuration.\n", desc->name);
>  goto try_again;
>  }
> +#if CONFIG_VP9_VAAPI_HWACCEL
> +if (hw_config->hwaccel && !avctx->hwaccel) {
> +#else
>  if (hw_config->hwaccel) {
> +#endif
>  av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
> "initialisation.\n", desc->name);
>  err = hwaccel_init(avctx, hw_config);
> --
> 2.17.2
>

This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.
Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?

- 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".

[FFmpeg-devel] [PATCH] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.

2019-05-27 Thread Yan Wang 
When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.

Signed-off-by: Yan Wang 
---
 libavcodec/decode.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
 
 for (;;) {
 // Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
 hwaccel_uninit(avctx);
+#endif
 
 user_choice = avctx->get_format(avctx, choices);
 if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum 
AVPixelFormat *fmt)
"missing configuration.\n", desc->name);
 goto try_again;
 }
+#if CONFIG_VP9_VAAPI_HWACCEL
+if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
 if (hw_config->hwaccel) {
+#endif
 av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
"initialisation.\n", desc->name);
 err = hwaccel_init(avctx, hw_config);
-- 
2.17.2

___
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".