[FFmpeg-devel] [PATCH] avfilter/internal: Improve docs about ff_request_frame()

[James Darnley]

On 2015-07-14 21:47, Michael Niedermayer wrote:
> On Tue, Jul 14, 2015 at 08:42:40PM +0200, James Darnley wrote:
>> On 2015-07-14 19:50, Michael Niedermayer wrote:
>>> + * One example of a source returning AVERROR(EAGAIN) is a buffer into which the
>>> + * user application pushes new data, it cannot fullfill a request
>>> + * or know its at EOF until the user application has given it this
>>> + * information
>>
>> I don't find this clear.
> 
> ok
> so please help me reword this in a way that is clear

I do want to but me knowledge of how filters are supposed to operate is
limited.

> you have a filter A which can contain a frame, or no frame
> it has a single buffer (it could have more but single is simpler as an
> example)
> if it has a frame in its buffer and the next filter does a
> ff_request_frame() on it it returns that frame and returns success
> (0) from the ff_request_frame() call
> if it does not have a frame in its buffer its screwed and has to
> return AVERROR(EAGAIN)
> until the user application gives it a frame by external means or
> tells it that EOF has been reached

Okay.  Don't make the doxy comment too verbose with examples.  I'll look
again at writing_filters.txt and my example and see if I can explain
things better there.

>> This is internal, isn't it?
> 
> yes, it is, it should be public API though its required for
> implementing filters

Sorry if I wasn't clear.  When I meant "internal" I meant internal to
lavfi, as in only filters need to use or care about this function.  Not
external users of lavfi, say a player or a video encoder.

>> What does a "user
>> application" matter to a filter, or someone writing a filter?
> 
> Derek complained on IRC that the "why" isnt documented, so
> i tried to document it.
> 
> 
>>
>> Perhaps something about filters rather than applications?
>>
>> One example of a source returning AVERROR(EAGAIN) is a filter which
>> buffers frames and cannot fulfil a request until "some event happens" or
>> the filter cannot know it is as EOF until "some other event happens".
> 
> This misses the main point and it is possibly why derek was confused
> about the API
> 
> EAGAIN here means "please return all the way back to the user
> application so it can perform the next transcode step and refill
> the filter graphs source _IF_ that is how the application interfaces
> to the filergraph"

Fair enough.  I didn't understand how requests were supposed to work
either and I think my example filter is wrong about them.  This latest
round of attention is helping.

> new patch:
> commit 6628dd2aa001ec23de306840a41f05947abdf326
> Author: Michael Niedermayer <michael at niedermayer.cc>
> Date:   Tue Jul 14 19:44:59 2015 +0200
> 
>     avfilter/internal: Improve docs about ff_request_frame()
> 
>     Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> 
> diff --git a/libavfilter/internal.h b/libavfilter/internal.h
> index a7ec751..f1938f9 100644
> --- a/libavfilter/internal.h
> +++ b/libavfilter/internal.h
> @@ -305,8 +305,24 @@ int ff_poll_frame(AVFilterLink *link);
>  /**
>   * Request an input frame from the filter at the other end of the link.
>   *
> + * The input filter may pass the request on to its inputs, fulfill the
> + * request from an internal buffer or any other means specific to its function.

Can I suggest a blank line here between the above line and the one below?

> + * When the end of a stream is reached AVERROR_EOF is returned and no further frames.

The line above is longer than 80 characters.  Are we supposed to stick
to that for comments?  Can I also ask for another blank line here, just
to clearly separate these two return conditions.

> + * When a filter is unable to output a frame for example due to its sources
> + * being unable to do so or because it depends on external means pushing data
> + * into it then AVERROR(EAGAIN) is returned.
> + * It is important that a AVERROR(EAGAIN) return is returned all the way to the
> + * caller (generally eventually a user application) as this step may (but does
> + * not have to be) neccessary to provide the input with the next frame.

Much better in my opinion, except for necessary (one 'c').


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 603 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150714/f0795313/attachment.sig>