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

[Stefano Sabatini]

On date Thursday 2016-12-01 22:21:17 +0100, Michael Niedermayer encoded:
> On Tue, Jun 14, 2016 at 05:55:47PM +0200, Stefano Sabatini wrote:
> > 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
> [...]
> > > > 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.
> 
> has anyone found a more general design ?
> 
> 
> > 
> > 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.
> 
> when decoding a frame triggers a log message then its caused by the
> frame.
> It may come ultimately from the experssion evaluator for example but
> knowig that the message comes from the experssion evaluator is of
> little use, a grep would tell one that already.
> Knowing which frame triggers it seems more usefull to me
> 
> 
> > 
> > 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.
> 

> its certainly possible to extend this patch to also cover demuxer
> and other areas and then add a -show_logs to select what should be
> enabled
> is that what you meant ?
> 
> it seems thats not a argument against the patch as it is, just something
> that can be done as additional work afterwards
> 
> but quite possibly i misunderstand

Feel free to push the patch as is. The other thing I don't like is the
threading dependency but probably there is not much we can do about
it.
-- 
FFmpeg = Fostering and Free Mystic Patchable Egregious Ghost