vc1: Print debug message if a b frame without reference is skipped

Message ID 1398600806-22310-1-git-send-email-alessandro@ghedini.me
State New
Headers show

Commit Message

Alessandro Ghedini April 27, 2014, 12:13 p.m.
From: Michael Niedermayer <michaelni@gmx.at>

---
 libavcodec/vc1dec.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Luca Barbato April 27, 2014, 12:36 p.m. | #1
On 27/04/14 14:13, Alessandro Ghedini wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
> 
> ---
>  libavcodec/vc1dec.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> index 9825924..ccba35b 100644
> --- a/libavcodec/vc1dec.c
> +++ b/libavcodec/vc1dec.c
> @@ -5941,6 +5941,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
>  
>      /* skip B-frames if we don't have reference frames */
>      if (s->last_picture_ptr == NULL && (s->pict_type == AV_PICTURE_TYPE_B || s->droppable)) {
> +        av_log(v->s.avctx, AV_LOG_DEBUG, "Skipping B frame without reference frames\n");
>          goto end;
>      }
>      if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||
> 

There is another similar condition below w/out any message and I'm not
so sure DEBUG is the correct level anyway.

lu
Alessandro Ghedini April 27, 2014, 1:47 p.m. | #2
On dom, apr 27, 2014 at 02:36:41 +0200, Luca Barbato wrote:
> On 27/04/14 14:13, Alessandro Ghedini wrote:
> > From: Michael Niedermayer <michaelni@gmx.at>
> > 
> > ---
> >  libavcodec/vc1dec.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> > index 9825924..ccba35b 100644
> > --- a/libavcodec/vc1dec.c
> > +++ b/libavcodec/vc1dec.c
> > @@ -5941,6 +5941,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
> >  
> >      /* skip B-frames if we don't have reference frames */
> >      if (s->last_picture_ptr == NULL && (s->pict_type == AV_PICTURE_TYPE_B || s->droppable)) {
> > +        av_log(v->s.avctx, AV_LOG_DEBUG, "Skipping B frame without reference frames\n");
> >          goto end;
> >      }
> >      if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||
> > 
> 
> There is another similar condition below w/out any message

Do you mean the if just below, or the if (s->next_p_frame_damaged) one?

> and I'm not so sure DEBUG is the correct level anyway.

What about VERBOSE?
Anton Khirnov April 28, 2014, 5:44 p.m. | #3
On Sun, 27 Apr 2014 15:47:25 +0200, Alessandro Ghedini <alessandro@ghedini.me> wrote:
> On dom, apr 27, 2014 at 02:36:41 +0200, Luca Barbato wrote:
> > On 27/04/14 14:13, Alessandro Ghedini wrote:
> > > From: Michael Niedermayer <michaelni@gmx.at>
> > > 
> > > ---
> > >  libavcodec/vc1dec.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> > > index 9825924..ccba35b 100644
> > > --- a/libavcodec/vc1dec.c
> > > +++ b/libavcodec/vc1dec.c
> > > @@ -5941,6 +5941,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
> > >  
> > >      /* skip B-frames if we don't have reference frames */
> > >      if (s->last_picture_ptr == NULL && (s->pict_type == AV_PICTURE_TYPE_B || s->droppable)) {
> > > +        av_log(v->s.avctx, AV_LOG_DEBUG, "Skipping B frame without reference frames\n");
> > >          goto end;
> > >      }
> > >      if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||
> > > 
> > 
> > There is another similar condition below w/out any message
> 
> Do you mean the if just below, or the if (s->next_p_frame_damaged) one?
> 
> > and I'm not so sure DEBUG is the correct level anyway.
> 
> What about VERBOSE?

Maybe warning. It is a kind of an error after all.
Alessandro Ghedini April 29, 2014, 2:11 p.m. | #4
On lun, apr 28, 2014 at 07:44:32 +0200, Anton Khirnov wrote:
> 
> On Sun, 27 Apr 2014 15:47:25 +0200, Alessandro Ghedini <alessandro@ghedini.me> wrote:
> > On dom, apr 27, 2014 at 02:36:41 +0200, Luca Barbato wrote:
> > > On 27/04/14 14:13, Alessandro Ghedini wrote:
> > > > From: Michael Niedermayer <michaelni@gmx.at>
> > > > 
> > > > ---
> > > >  libavcodec/vc1dec.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> > > > index 9825924..ccba35b 100644
> > > > --- a/libavcodec/vc1dec.c
> > > > +++ b/libavcodec/vc1dec.c
> > > > @@ -5941,6 +5941,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
> > > >  
> > > >      /* skip B-frames if we don't have reference frames */
> > > >      if (s->last_picture_ptr == NULL && (s->pict_type == AV_PICTURE_TYPE_B || s->droppable)) {
> > > > +        av_log(v->s.avctx, AV_LOG_DEBUG, "Skipping B frame without reference frames\n");
> > > >          goto end;
> > > >      }
> > > >      if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||
> > > > 
> > > 
> > > There is another similar condition below w/out any message
> > 
> > Do you mean the if just below, or the if (s->next_p_frame_damaged) one?
> > 
> > > and I'm not so sure DEBUG is the correct level anyway.
> > 
> > What about VERBOSE?
> 
> Maybe warning. It is a kind of an error after all.

Well the point is that it's not really an error (seeking is a valid action).

In any case, I'm not against dropping the patch altogether, I just forwarded it
from ffmpeg because it seemed like a good idea, but I don't have that much
interest in it.

Cheers

Patch

diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 9825924..ccba35b 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -5941,6 +5941,7 @@  static int vc1_decode_frame(AVCodecContext *avctx, void *data,
 
     /* skip B-frames if we don't have reference frames */
     if (s->last_picture_ptr == NULL && (s->pict_type == AV_PICTURE_TYPE_B || s->droppable)) {
+        av_log(v->s.avctx, AV_LOG_DEBUG, "Skipping B frame without reference frames\n");
         goto end;
     }
     if ((avctx->skip_frame >= AVDISCARD_NONREF && s->pict_type == AV_PICTURE_TYPE_B) ||