[FFmpeg-devel] [PATCH] Add debug facilities for avfilter.c

Michael Niedermayer michaelni
Tue Oct 6 00:30:58 CEST 2009


On Mon, Oct 05, 2009 at 11:46:28PM +0200, Stefano Sabatini wrote:
> On date Monday 2009-10-05 22:42:04 +0200, Michael Niedermayer encoded:
> > On Mon, Oct 05, 2009 at 10:37:10PM +0200, Michael Niedermayer wrote:
> > > On Mon, Oct 05, 2009 at 09:34:50PM +0200, Stefano Sabatini wrote:
> > > > On date Monday 2009-10-05 00:32:40 +0200, Michael Niedermayer encoded:
> > > > > On Sun, Oct 04, 2009 at 06:40:12PM +0200, Stefano Sabatini wrote:
> > [...]
> > > > > the buffers and snprintf() are useless, a single av_log call is enough
> > > > 
> > > > How? I'd like to keep the dprint_STUFF functions, so it's easy to
> > > > add/customize other traces in other places if necessary.
> > > 
> > > you have a single function that prints it all and you call that single
> > > function from various places
> > > that function is #defined to ; if DEBUG is not set
> > > you pass NULL if you dont have some link or picref
> > > the fuction just calls av_log() or dprintf()
> > > 
> > > this is simpler than your code
> > 
> > it also could be 2 functions, one for picref and one for link
> > but the snprintf + buffers + av_log is unneeded they can print directly
> 
> What about:
> request_frame   : link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ]
> request_frame   : link[0xa056200 s:100x200 scale           ->hflip           ]
> request_frame   : link[0xa034040 s:464x348 crop            ->scale           ]
> request_frame   : link[0xa0340a0 s:464x348 vflip           ->crop            ]
> request_frame   : link[0xa031d60 s:464x348 ffplay_input    ->vflip           ]
> get_video_buffer: link[0xa031d60 s:464x348 ffplay_input    ->vflip           ] perms:2 w:464 h:348
> get_video_buffer: link[0xa0340a0 s:464x348 vflip           ->crop            ] perms:2 w:464 h:348
> get_video_buffer: link[0xa034040 s:464x348 crop            ->scale           ] perms:2 w:464 h:348
> get_video_buffer: link[0xa034040 s:464x348 crop            ->scale           ] returning picref[0xa033fd0 data[0xa1f9750, 0xa220e10, 0xa22b130, (nil)] linesize[464, 240, 240, 0] pts:0 s:464x348]
> get_video_buffer: link[0xa0340a0 s:464x348 vflip           ->crop            ] returning picref[0xa033fd0 data[0xa1f9750, 0xa220e10, 0xa22b130, (nil)] linesize[464, 240, 240, 0] pts:0 s:464x348]
> get_video_buffer: link[0xa031d60 s:464x348 ffplay_input    ->vflip           ] returning picref[0xa033fd0 data[0xa220c40, 0xa22b040, 0xa235360, (nil)] linesize[-464, -240, -240, 0] pts:0 s:464x348]
> start_frame     : link[0xa031d60 s:464x348 ffplay_input    ->vflip           ] picref[0xa006ea0 data[0xa220c40, 0xa22b040, 0xa235360, (nil)] linesize[-464, -240, -240, 0] pts:900000 s:464x348]
> start_frame     : link[0xa0340a0 s:464x348 vflip           ->crop            ] picref[0xa057440 data[0xa1f9750, 0xa220e10, 0xa22b130, (nil)] linesize[464, 240, 240, 0] pts:900000 s:464x348]
> start_frame     : link[0xa034040 s:464x348 crop            ->scale           ] picref[0xa08b230 data[0xa1f9750, 0xa220e10, 0xa22b130, (nil)] linesize[464, 240, 240, 0] pts:900000 s:464x348]
> get_video_buffer: link[0xa056200 s:100x200 scale           ->hflip           ] perms:2 w:100 h:200
> get_video_buffer: link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ] perms:2 w:100 h:200
> get_video_buffer: link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ] returning picref[0xa031e40 data[0xa235480, 0xa23ac00, 0xa23c500, (nil)] linesize[112, 64, 64, 0] pts:0 s:100x200]
> get_video_buffer: link[0xa056200 s:100x200 scale           ->hflip           ] returning picref[0xa031e40 data[0xa235480, 0xa23ac00, 0xa23c500, (nil)] linesize[112, 64, 64, 0] pts:0 s:100x200]
> start_frame     : link[0xa056200 s:100x200 scale           ->hflip           ] picref[0xa031fb0 data[0xa235480, 0xa23ac00, 0xa23c500, (nil)] linesize[112, 64, 64, 0] pts:900000 s:100x200]
> get_video_buffer: link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ] perms:2 w:100 h:200
> get_video_buffer: link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ] returning picref[0xa032020 data[0xa23de30, 0xa2435b0, 0xa244eb0, (nil)] linesize[112, 64, 64, 0] pts:0 s:100x200]
> start_frame     : link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ] picref[0xa057290 data[0xa23de30, 0xa2435b0, 0xa244eb0, (nil)] linesize[112, 64, 64, 0] pts:900000 s:100x200]
> draw_slice      : link[0xa031d60 s:464x348 ffplay_input    ->vflip           ] y:0 h:348
> draw_slice      : link[0xa0340a0 s:464x348 vflip           ->crop            ] y:0 h:348
> draw_slice      : link[0xa034040 s:464x348 crop            ->scale           ] y:0 h:348
> draw_slice      : link[0xa056200 s:100x200 scale           ->hflip           ] y:0 h:200
> draw_slice      : link[0xa0562b0 s:100x200 hflip           ->ffplay_output   ] y:0 h:200
> ...
> 
> Now I wonder if keeping some void function would affect performance,
> ideally a clever compiler should avoid to even call such functions (an
> empty function which calls an empty function), but in practice things
> may be different. In that case a macro is still preferable.
> 
> Regards.
> -- 
> FFmpeg = Faboulous and Faithful Marvellous Plastic Enhanced Game

>  avfilter.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 11407ca4edfeaaf834cb3fb0674625284bbf3988  debug-system.patch
> Index: ffmpeg-vfilters/ffmpeg/libavfilter/avfilter.c
> ===================================================================
> --- ffmpeg-vfilters.orig/ffmpeg/libavfilter/avfilter.c	2009-10-05 21:45:42.000000000 +0200
> +++ ffmpeg-vfilters/ffmpeg/libavfilter/avfilter.c	2009-10-05 23:43:19.000000000 +0200
> @@ -22,6 +22,8 @@
>  #include "libavcodec/imgconvert.h"
>  #include "avfilter.h"
>  
> +/* #define DEBUG */
> +
>  unsigned avfilter_version(void) {
>      return LIBAVFILTER_VERSION_INT;
>  }
> @@ -160,10 +162,43 @@
>      return 0;
>  }
>  
> +static void dlog(const char *fmt, ...)
> +{
> +#ifdef DEBUG
> +    va_list vl;
> +    va_start(vl, fmt);
> +    av_vlog(NULL, AV_LOG_DEBUG, fmt, vl);
> +    va_end(vl);
> +#endif
> +}

i really do not understand you.
why did you add this?
lets return to the previous revission please, that was cleaner.

from the previous revission please remove the snprintf() and use dprintf()
directly, do not add functions like above please.
This is simple debug code that is not important enough for such obfuscated
indirection tricks whatever the reason may be for you to have added them,
also you did not explain at all why such uncommon ways of printing would
be usefull.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091006/d8d75548/attachment.pgp>



More information about the ffmpeg-devel mailing list