| Message ID | 1516869432-30961-1-git-send-email-ruiling.song@intel.com |
|---|---|
| State | New |
| Headers | show |
On 2018/1/25 16:37, 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. > > v2: > switch to using av_packet_unref(). > > 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 5e00673..d92a150 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); I think add a warning message more better than only drop the packet on the quiet. > return ret; > + } > > s->buffer_pkt.size -= ret; > s->buffer_pkt.data += ret;
On 2018/1/25 16:37, 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. > > v2: > switch to using av_packet_unref(). > > 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 5e00673..d92a150 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); I think add a warning message more better than only drop the packet on the quiet. > return ret; > + } > > s->buffer_pkt.size -= ret; > s->buffer_pkt.data += ret;
> -----Original Message----- > From: Jun Zhao [mailto:mypopydev@gmail.com] > Sent: Friday, January 26, 2018 1:02 PM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>; > Song, Ruiling <ruiling.song@intel.com> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsv: skip the packet if decoding failure. > > > > On 2018/1/25 16:37, 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. > > > > v2: > > switch to using av_packet_unref(). > > > > 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 5e00673..d92a150 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); > > I think add a warning message more better than only drop the packet on > the quiet. What do you think "packet decoding failed, skip it."? Usually some error message has already been printed out before skipping the packet. I am OK to make a new version to address it. And thanks for your feedback. Ruiling > > return ret; > > + } > > > > s->buffer_pkt.size -= ret; > > s->buffer_pkt.data += ret;
On 26/01/18 07:33, Song, Ruiling wrote: > > >> -----Original Message----- >> From: Jun Zhao [mailto:mypopydev@gmail.com] >> Sent: Friday, January 26, 2018 1:02 PM >> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>; >> Song, Ruiling <ruiling.song@intel.com> >> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsv: skip the packet if decoding failure. >> >> >> >> On 2018/1/25 16:37, 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. >>> >>> v2: >>> switch to using av_packet_unref(). >>> >>> 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 5e00673..d92a150 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); >> >> I think add a warning message more better than only drop the packet on >> the quiet. > What do you think "packet decoding failed, skip it."? > Usually some error message has already been printed out before skipping the packet. Yeah. I think the only ways we get here without having already logged the failure are either an internal bug (the setup is broken somehow) or out-of-memory. Neither of those would be improved by a generic "decoding failed" message here. Anyway, this was applied in libav and has already been applied here as a merge. Thanks, - Mark
diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c index 5e00673..d92a150 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;