[FFmpeg-devel] [PATCH 1/4] Implement avfilter_ref_video_buffer().

[Stefano Sabatini]

On date Wednesday 2010-11-24 17:03:41 +0100, Michael Niedermayer encoded:
> On Tue, Nov 23, 2010 at 11:23:58PM +0100, Stefano Sabatini wrote:
> > On date Tuesday 2010-11-23 23:11:46 +0100, Michael Niedermayer encoded:
> > > On Tue, Nov 23, 2010 at 12:56:33AM +0100, Stefano Sabatini wrote:
> > > > On date Monday 2010-11-22 03:48:08 +0100, Michael Niedermayer encoded:
> > > > > On Sun, Nov 21, 2010 at 11:36:44PM +0100, Stefano Sabatini wrote:
> > > > > > On date Sunday 2010-11-21 17:57:34 +0100, Michael Niedermayer encoded:
> > > > > > > On Sat, Nov 20, 2010 at 12:59:30PM +0100, Stefano Sabatini wrote:
> > > > > > > > On date Saturday 2010-11-20 03:39:44 +0100, Michael Niedermayer encoded:
> > > > > > > > > On Sat, Nov 13, 2010 at 03:02:50PM +0100, Stefano Sabatini wrote:
> > > > > > > > > > On date Friday 2010-11-12 20:19:08 +0100, Michael Niedermayer encoded:
> > > > > > > > > > > On Fri, Nov 12, 2010 at 03:49:38PM +0100, Stefano Sabatini wrote:
> > > > > > > > > > [...]
> > > > > > > > > > > > Other proposals:
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > > avfilter_get_video_buffer_ref_from_arrays
> > > > > > > > > > > 
> > > > > > > > > > > just longer than my suggestion otherwise its the same
> > > > > > > > > > 
> > > > > > > > > > Your suggestion:
> > > > > > > > > > avfilter_arrays_to_video_buffer_ref()
> > > > > > > > > > 
> > > > > > > > > > I slightly dislike it as it lacks a verb.
> > > > > > > > > 
> > > > > > > > > If you associate a function with doing something then yes that makes sense
> > > > > > > > > doxy then should also be writen to indicate so which leads to 3rd person ...
> > > > > > > > > 
> > > > > > > > > the use of to or 2 is quite common to indicate "convert to" though
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > >  
> > > > > > > > > > > > avfilter_ref_video_buffer_arrays
> > > > > > > > > > > 
> > > > > > > > > > > ambigous
> > > > > > > > > > 
> > > > > > > > > > "Reference video buffer arrays", that is get a reference for the given
> > > > > > > > > > video buffer arrays (data+linesizes), I don't see that so much
> > > > > > > > > > ambiguous and a name of a function has not to be necessarily
> > > > > > > > > > *completely* explicative.
> > > > > > > > > 
> > > > > > > > > avfilter_ref_video_buffer_arrays, does not contain a verb
> > > > > > > > > either. More precissely ref is not a english word at all.
> > > > > > > > 
> > > > > > > > "ref" clearly stands for "reference".
> > > > > > > > 
> > > > > > > > > > > > avfilter_ref_buffer_from_video_buffer_arrays
> > > > > > > > > > > > avfilter_get_video_buffer_ref_from_video_buffer_arrays
> > > > > > > > > > > > avfilter_get_buffer_ref_from_video_buffer_arrays
> > > > > > > > > > > 
> > > > > > > > > > > we have no video anything array, as the thing just represents a single frame
> > > > > > > > > > 
> > > > > > > > > > AVFilterVideoBufferRef references just an image anyway, so if this is
> > > > > > > > > > amniguous the same is for the name of that struct.
> > > > > > > > > 
> > > > > > > > > Thats not untrue
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > Alternatively we could use the term "frame" in place of image (which
> > > > > > > > > > is more consistent with the names already used in the rest of the
> > > > > > > > > > lavfi API, start/end_frame).
> > > > > > > > > 
> > > > > > > > > To quote h264
> > > > > > > > > 3.104 picture: A collective term for a field or a frame.
> > > > > > > > > 
> > > > > > > > > So while you might think this is bikeshed, its not the terms you switch between
> > > > > > > > > have subtiley different meaning
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > > avfilter_ref_buffer_from_image_arrays
> > > > > > > > > > > > avfilter_get_video_buffer_ref_from_image_arrays
> > > > > > > > > > > > avfilter_get_buffer_ref_from_image_arrays
> > > > > > > > > > > 
> > > > > > > > > > > image arrays implicates to me that there are several images
> > > > > > > > > > 
> > > > > > > > > > That's unfortunately a quirk of the English language, image arrays ~
> > > > > > > > > > images arrays.
> > > > > > > > > > 
> > > > > > > > > > Resuming:
> > > > > > > > > > avfilter_get_video_buffer_ref_from_frame
> > > > > > > > > 
> > > > > > > > > > avfilter_get_video_buffer_ref_from_arrays
> > > > > > > > > 
> > > > > > > > > from your list this is the only that is not totally nonsense
> > > > > > > > > still it is just longer than what i suggested with zero additional information
> > > > > > > > > added
> > > > > > > > > avfilter_arrays_to_video_buffer_ref
> > > > > > > > > vs.
> > > > > > > > > avfilter_get_video_buffer_ref_from_arrays
> > > > > > > > 
> > > > > > > > So we have the candidates:
> > > > > > > > avfilter_arrays_to_video_buffer_ref
> > > > > > > > avfilter_get_video_buffer_ref_from_arrays
> > > > > > > > 
> > > > > > > > Other proposals:
> > > > > > > > avfilter_ref_video_buffer_from_arrays => reference video buffer from arrays 
> > > > > > > > avfilter_ref_video_buffer_arrays      => reference video buffer arrays
> > > > > > > > 
> > > > > > > > otherwise I'll stick with one of the candidates.
> > > > > > > 
> > > > > > > pick what you want, iam too busy for this
> > > > > > 
> > > > > > I picked avfilter_get_video_buffer_ref_from_arrays, patch updated.
> > > > > > -- 
> > > > > > FFmpeg = Faithless & Furious Magnificient Pitiless Evangelical Gigant
> > > > > 
> > > > > >  avfilter.c |   39 +++++++++++++++++++++++++++++++++++++++
> > > > > >  avfilter.h |   15 +++++++++++++++
> > > > > >  defaults.c |   50 ++++++++++++++++----------------------------------
> > > > > >  3 files changed, 70 insertions(+), 34 deletions(-)
> > > > > > fbcb4c355440c30c1c240425092e08359e3cfa88  0001-Implement-avfilter_get_video_buffer_ref_from_arrays.patch
> > > > > > From e91d3a586e80f7174c7645b5bbb393b698b456cc Mon Sep 17 00:00:00 2001
> > > > > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > > > > Date: Mon, 1 Nov 2010 15:31:50 +0100
> > > > > > Subject: [PATCH] Implement avfilter_get_video_buffer_ref_from_arrays().
> > > > > 
> > > > > fine with me if it has been tested and works
> > > > 
> > > > Updated with a simple change, I'm setting:
> > > >     picref->buf->free = avfilter_default_free_buffer;
> > > > 
> > > > in avfilter_get_video_buffer_ref_from_arrays() which seems more useful.
> > > > 
> > > 
> > > > This change also requires to make public the
> > > > avfilter_default_free_buffer() function, as in the attached patch.
> > > 
> > > why?
> > 
> > I prefer to place the callback for freeing the buffer in the library
> > code,
> 
> my question was "why make it public" not "why in the lib" sorry if that was
> unclear

That's completely indifferent to me, and since it is currently
non-used in application code maybe it's even better to declare them
internally in libavfilter/internal.h, but looked more consistent to
place its declaration togheter with the other avfilter_default*
declarations.
-- 
FFmpeg = Fishing and Fast Meaningful Practical Evanescent Guru