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

[Stefano Sabatini]

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, or the user will forget to set it and memleak (and she can still
override that explicitely setting the free callback in the application
code).

Note that I don't have a strong preference for this and I'm not sure
this is the best choice, that's why I'm posting here for review.
-- 
FFmpeg = Fancy & Funny Mean Patchable Ecstatic Gadget