[V2] lavc/qsv: skip the packet if decoding failure.

Message ID 1516760091-4555-1-git-send-email-ruiling.song@intel.com
State New
Headers show
Series
  • [V2] lavc/qsv: skip the packet if decoding failure.
Related show

Commit Message

Ruiling Song Jan. 24, 2018, 2:14 a.m.
From: "Ruiling, Song" <ruiling.song@intel.com>

MediaSDK may fail to decode some frame, just skip it.
Otherwise, it will keep decoding the failure packet repeatedly
without processing any packet afterwards.

Signed-off-by: Ruiling Song <ruiling.song@intel.com>
---
 libavcodec/qsvdec_h2645.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Luca Barbato Jan. 24, 2018, 10:36 a.m. | #1
On 24/01/2018 03:14, Ruiling Song wrote:
> From: "Ruiling, Song" <ruiling.song@intel.com>
> 
> MediaSDK may fail to decode some frame, just skip it.
> Otherwise, it will keep decoding the failure packet repeatedly
> without processing any packet afterwards.
> 
> Signed-off-by: Ruiling Song <ruiling.song@intel.com>
> ---
>   libavcodec/qsvdec_h2645.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
> index 83880dc..78a7b61 100644
> --- a/libavcodec/qsvdec_h2645.c
> +++ b/libavcodec/qsvdec_h2645.c
> @@ -153,8 +153,12 @@ static int qsv_decode_frame(AVCodecContext *avctx, void *data,
>           }
>   
>           ret = ff_qsv_process_data(avctx, &s->qsv, frame, got_frame, &s->buffer_pkt);
> -        if (ret < 0)
> +        if (ret < 0){
> +            /* Drop buffer_pkt when failed to decode the packet. Otherwise,
> +               the decoder will keep decoding the failure packet. */
> +            av_packet_unref(&s->buffer_pkt);
>               return ret;
> +        }
>   
>           s->buffer_pkt.size -= ret;
>           s->buffer_pkt.data += ret;
> 

Looks better, do you have a specific test sample?

lu
Ruiling Song Jan. 25, 2018, 1:21 a.m. | #2
> -----Original Message-----
> From: libav-devel [mailto:libav-devel-bounces@libav.org] On Behalf Of Luca
> Barbato
> Sent: Wednesday, January 24, 2018 6:36 PM
> To: libav-devel@libav.org
> Subject: Re: [libav-devel] [PATCH V2] lavc/qsv: skip the packet if decoding failure.
> 
> On 24/01/2018 03:14, Ruiling Song wrote:
> > From: "Ruiling, Song" <ruiling.song@intel.com>
> >
> > MediaSDK may fail to decode some frame, just skip it.
> > Otherwise, it will keep decoding the failure packet repeatedly
> > without processing any packet afterwards.
> >
> > Signed-off-by: Ruiling Song <ruiling.song@intel.com>
> > ---
> >   libavcodec/qsvdec_h2645.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
> > index 83880dc..78a7b61 100644
> > --- a/libavcodec/qsvdec_h2645.c
> > +++ b/libavcodec/qsvdec_h2645.c
> > @@ -153,8 +153,12 @@ static int qsv_decode_frame(AVCodecContext *avctx,
> void *data,
> >           }
> >
> >           ret = ff_qsv_process_data(avctx, &s->qsv, frame, got_frame, &s-
> >buffer_pkt);
> > -        if (ret < 0)
> > +        if (ret < 0){
> > +            /* Drop buffer_pkt when failed to decode the packet. Otherwise,
> > +               the decoder will keep decoding the failure packet. */
> > +            av_packet_unref(&s->buffer_pkt);
> >               return ret;
> > +        }
> >
> >           s->buffer_pkt.size -= ret;
> >           s->buffer_pkt.data += ret;
> >
> 
> Looks better, do you have a specific test sample?
Sorry I don't have the original clip to hit the issue.
But I tried to decode fate-suite/h264/attachment631-small.mp4 (The video stream has some broken frames)
QSV keeps trying to decode one broken frame. With this patch, qsv could move on to process all the frames.
Even with this patch, MediaSDK still fails to decode out any frame in that stream, which I think the issue comes from MediaSDK itself.

Thanks!
Ruiling
> 
> lu
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
Ruiling Song Jan. 25, 2018, 6:28 a.m. | #3
> >
> > Looks better, do you have a specific test sample?
> Sorry I don't have the original clip to hit the issue.
> But I tried to decode fate-suite/h264/attachment631-small.mp4 (The video
> stream has some broken frames)
Sorry that the video stream I mentioned is inside FFmpeg fate instead of Libav fate.

Ruiling
> QSV keeps trying to decode one broken frame. With this patch, qsv could move
> on to process all the frames.
> Even with this patch, MediaSDK still fails to decode out any frame in that stream,
> which I think the issue comes from MediaSDK itself.
> 
> Thanks!
> Ruiling
> >
> > lu
> > _______________________________________________
> > libav-devel mailing list
> > libav-devel@libav.org
> > https://lists.libav.org/mailman/listinfo/libav-devel
Luca Barbato Jan. 25, 2018, 7:59 a.m. | #4
On 25/01/2018 02:21, Song, Ruiling wrote:
> 
> 
>> -----Original Message----- From: libav-devel
>> [mailto:libav-devel-bounces@libav.org] On Behalf Of Luca Barbato 
>> Sent: Wednesday, January 24, 2018 6:36 PM To:
>> libav-devel@libav.org Subject: Re: [libav-devel] [PATCH V2]
>> lavc/qsv: skip the packet if decoding failure.
>> 
>> On 24/01/2018 03:14, Ruiling Song wrote:
>>> From: "Ruiling, Song" <ruiling.song@intel.com>
>>> 
>>> MediaSDK may fail to decode some frame, just skip it. Otherwise,
>>> it will keep decoding the failure packet repeatedly without
>>> processing any packet afterwards.
>>> 
>>> Signed-off-by: Ruiling Song <ruiling.song@intel.com> --- 
>>> libavcodec/qsvdec_h2645.c | 6 +++++- 1 file changed, 5
>>> insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/libavcodec/qsvdec_h2645.c
>>> b/libavcodec/qsvdec_h2645.c index 83880dc..78a7b61 100644 ---
>>> a/libavcodec/qsvdec_h2645.c +++ b/libavcodec/qsvdec_h2645.c @@
>>> -153,8 +153,12 @@ static int qsv_decode_frame(AVCodecContext
>>> *avctx,
>> void *data,
>>> }
>>> 
>>> ret = ff_qsv_process_data(avctx, &s->qsv, frame, got_frame, &s- 
>>> buffer_pkt); -        if (ret < 0) +        if (ret < 0){ +
>>> /* Drop buffer_pkt when failed to decode the packet. Otherwise, +
>>> the decoder will keep decoding the failure packet. */ +
>>> av_packet_unref(&s->buffer_pkt); return ret; +        }
>>> 
>>> s->buffer_pkt.size -= ret; s->buffer_pkt.data += ret;
>>> 
>> 
>> Looks better, do you have a specific test sample?
> Sorry I don't have the original clip to hit the issue. But I tried to
> decode fate-suite/h264/attachment631-small.mp4 (The video stream has
> some broken frames) QSV keeps trying to decode one broken frame. With
> this patch, qsv could move on to process all the frames. Even with
> this patch, MediaSDK still fails to decode out any frame in that
> stream, which I think the issue comes from MediaSDK itself.

Ok, I'd merge this if nobody is against then.

lu
Maxym Dmytrychenko Jan. 25, 2018, 8:32 a.m. | #5
just fine, thanks

On Thu, Jan 25, 2018 at 8:59 AM, Luca Barbato <lu_zero@gentoo.org> wrote:

> On 25/01/2018 02:21, Song, Ruiling wrote:
>
>>
>>
>> -----Original Message----- From: libav-devel
>>> [mailto:libav-devel-bounces@libav.org] On Behalf Of Luca Barbato Sent:
>>> Wednesday, January 24, 2018 6:36 PM To:
>>> libav-devel@libav.org Subject: Re: [libav-devel] [PATCH V2]
>>> lavc/qsv: skip the packet if decoding failure.
>>>
>>> On 24/01/2018 03:14, Ruiling Song wrote:
>>>
>>>> From: "Ruiling, Song" <ruiling.song@intel.com>
>>>>
>>>> MediaSDK may fail to decode some frame, just skip it. Otherwise,
>>>> it will keep decoding the failure packet repeatedly without
>>>> processing any packet afterwards.
>>>>
>>>> Signed-off-by: Ruiling Song <ruiling.song@intel.com> ---
>>>> libavcodec/qsvdec_h2645.c | 6 +++++- 1 file changed, 5
>>>> insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/qsvdec_h2645.c
>>>> b/libavcodec/qsvdec_h2645.c index 83880dc..78a7b61 100644 ---
>>>> a/libavcodec/qsvdec_h2645.c +++ b/libavcodec/qsvdec_h2645.c @@
>>>> -153,8 +153,12 @@ static int qsv_decode_frame(AVCodecContext
>>>> *avctx,
>>>>
>>> void *data,
>>>
>>>> }
>>>>
>>>> ret = ff_qsv_process_data(avctx, &s->qsv, frame, got_frame, &s-
>>>> buffer_pkt); -        if (ret < 0) +        if (ret < 0){ +
>>>> /* Drop buffer_pkt when failed to decode the packet. Otherwise, +
>>>> the decoder will keep decoding the failure packet. */ +
>>>> av_packet_unref(&s->buffer_pkt); return ret; +        }
>>>>
>>>> s->buffer_pkt.size -= ret; s->buffer_pkt.data += ret;
>>>>
>>>>
>>> Looks better, do you have a specific test sample?
>>>
>> Sorry I don't have the original clip to hit the issue. But I tried to
>> decode fate-suite/h264/attachment631-small.mp4 (The video stream has
>> some broken frames) QSV keeps trying to decode one broken frame. With
>> this patch, qsv could move on to process all the frames. Even with
>> this patch, MediaSDK still fails to decode out any frame in that
>> stream, which I think the issue comes from MediaSDK itself.
>>
>
> Ok, I'd merge this if nobody is against then.
>
> lu
>
> _______________________________________________
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel

Patch

diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
index 83880dc..78a7b61 100644
--- a/libavcodec/qsvdec_h2645.c
+++ b/libavcodec/qsvdec_h2645.c
@@ -153,8 +153,12 @@  static int qsv_decode_frame(AVCodecContext *avctx, void *data,
         }
 
         ret = ff_qsv_process_data(avctx, &s->qsv, frame, got_frame, &s->buffer_pkt);
-        if (ret < 0)
+        if (ret < 0){
+            /* Drop buffer_pkt when failed to decode the packet. Otherwise,
+               the decoder will keep decoding the failure packet. */
+            av_packet_unref(&s->buffer_pkt);
             return ret;
+        }
 
         s->buffer_pkt.size -= ret;
         s->buffer_pkt.data += ret;