[FFmpeg-devel] [PATCH] ffprobe: Support adding av_log output to frames

[Stefano Sabatini]

On date Wednesday 2016-06-08 18:20:39 +0200, Michael Niedermayer encoded:
> On Sun, Jun 05, 2016 at 12:56:08PM +0200, Stefano Sabatini wrote:
> > On date Tuesday 2016-05-31 21:23:27 +0200, Michael Niedermayer encoded:
> > > adding demuxer and other logs should be easy
> > > This forces single threaded decoding for simplicity
> > > It also requires pthreads, this could be avoided either with
> > > some lockless tricks or simply by assuming av_log would never be called from
> > > another thread.
> > > 
> > > doc/ffprobe.xsd update missing (TODO & help welcome)
> > > 
> > > Fixes Ticket5521
> > > 
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  ffprobe.c |  154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 153 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/ffprobe.c b/ffprobe.c
> > > index a7e329c..07b61ec 100644
> > > --- a/ffprobe.c
> > > +++ b/ffprobe.c
> > > @@ -49,6 +49,19 @@
> > >  #include "libpostproc/postprocess.h"
> > >  #include "cmdutils.h"
> > >  
> > 
> > > +#if HAVE_PTHREADS
> > > +#  include <pthread.h>
> > > +#else
> > > +#  ifdef pthread_mutex_lock
> > > +#    undef pthread_mutex_lock
> > > +#  endif
> > > +#  define pthread_mutex_lock(a)
> > > +#  ifdef pthread_mutex_unlock
> > > +#    undef pthread_mutex_unlock
> > > +#  endif
> > > +#  define pthread_mutex_unlock(a)
> > > +#endif
> > > +
> > 
> > any chance to move this ifdeffery to lavu?
> 
> iam not sure what you suggest exactly ?

Forget about it.

[...]
> > > +#if HAVE_PTHREADS
> > > +    ret = pthread_mutex_init(&log_mutex, NULL);
> > > +    if (ret != 0) {
> > > +        goto end;
> > > +    }
> > > +#endif
> > > +    av_log_set_callback(log_callback);
> > > +
> > >      av_log_set_flags(AV_LOG_SKIP_REPEATED);
> > >      register_exit(ffprobe_cleanup);
> > 
> > I'm not really sure about storing the log in frame, since they could
> > come from any context, not necessarily related to decoding.
> > 
> > OTOH, I'm not really sure how such feature could be implemented in a
> > really clean way.
> > 
> > About frame decoding errors, we could store an error in the decoded
> > frame itself.
> 

> I dont know what exactly is wanted ?
> 

I mean, we already have the AVFrame.decode_error_flags we could expose
to the user.

> What i found interresting was to partition all av_log into
> closest related sections like decoder/demuxer/filter/...
> and add them there
> so that detailed information, not limited to errors about frame and
> packet related information can be associated with them
> this patch was just a step toward that
> if that feature isnt wanted we can drop the patch

I think this patch is interesting, but I don't want to clutter the
code too much, especially if we found a more general design which
could deprecate this one.

The problem with this approach is that the log is contained in the
frame element, when it could come from other parts of the code as
well.

One option could be to define a -show_logs options, and optionally add
a filter specifying the contexts. Just storing the log in the frame
elements sounds a bit weird to me.
-- 
FFmpeg = Fanciful and Friendly Merciless Plastic EnGine