[FFmpeg-devel] [PATCH] lavfi/audio: fix size of copied samples.

[Nicolas George]

[ TL;DR: My goal is to understand how things currently work, not change the
  API. PRESERVE is like a reader's lock and REUSE2 is like a writer's lock.
  Since locks need to be shared by all references and the permissions are in
  the references, they can only be applied when the buffer is created. ]

Le quintidi 5 thermidor, an CCXX, Michael Niedermayer a écrit :
> > That may be so, and indeed I can spot a few inconsistencies. But do you
> > agree the mechanism I described works? For reference:
> > 
> > * If a filter wants WRITE, it rejects PRESERVE; if a frame with PRESERVE
> >   arrives, it is copied because of that.
> > 
> > Also, if the filters that want PRESERVE just remove WRITE when sending the
> > frame, I do not see the point of the PRESERVE at all: removing WRITE is
> > enough.
> 
> I dont know what you mean by "works"?

I mean that without changing the framework code nor most of the filters
(only those that are inconsistent), it achieves its goal, which is to ensure
that all filters can do what they need on their buffers without stepping on
each other's feet and with a minimum amount of copying.

> It certainly does not work with the current API / current definition
> of the permissions.

The current definition of the permissions are quite vague.

> a filter might want WRITE+PRESERVE (mpeg1/2 decoder would be an
> example). It cant request a frame with write but not preserve because
> that wouldnt be what it needs.

If the buffer is obtained from get_*_buffer, the permission can be requested
at that time. Same if the buffer is constructed around private memory. That
is what ffmpeg and src_movie do.

I wrote the following paragraphs, until I realized it does not work due to
the split filter (or any similar filter):

# If the buffer comes from an input, I do not think we currently have any
# filter that do that, but I think this would work: .min_perms = READ|WRITE,
# .rej_perms = PRESERVE|REUSE2, and as soon as we get it, ref->perms |=
# PRESERVE.
# 
# Explanation: We intend to write, so we do not accept a buffer that someone
# wants to preserve for himself, so reject PRESERVE. We do not the buffer to
# change under our feet, so we do not accept a buffer that someone else intend
# to change, so reject REUSE2. We do not want the next filters to change the
# buffer either, so we add PRESERVE.
# 
# Adding a permission may seem strange, until you realize that PRESERVE is
# really an interdiction: we forbid all other filters to write on the buffer.
# Adding interdictions is removing permissions, and removing permissions is
# acceptable.

So with the current design, PRESERVE can only be set when a buffer is
created. Fortunately, the situations where PRESERVE would be needed for an
already existing filter are few (a frame interpolation filter maybe?).

In fact, it may be more intuitive to think of PRESERVE as a reader's lock,
and to REUSE2 as a writer's lock. Since locks should be shared, and the
permissions are on the references, they can only be established when there
is no concurrency, i.e. at the beginning.

> It may be that you could redefine the flags in some way in which things
> work out but i do not know the definition you base things on so i cant
> argue about it much beyond that it feels very odd and doesnt feel like
> a good choice

Note that I am not trying to redefine things or change anything: I am trying
to understand how things currently work, so I can document them and figure
out how to do things that are not currently done.

> that then also means its not only counterintuitive but also cannot be
> tested anymore automatically when the permissions no longer mean
> what access is allowed

That is true.

> Whatever API we would choose the fork would likely choose a different
> one.

True too. But, again, I do not want to choose an API, just understand the
current one.

> when decoding, a frame may be a B frame (needs write but no preserve
> as it is no reference in mpeg1 ever) or a P frame which needs read+
> write+preserve)
> this also means the needed permissions are not static but depend on
> the header of each frame. Static flags in the filter pad wont work
> here.
> Now a following filter may draw subtitles into the frame, whichever
> way one defines permissions. It is possible with B frames in above case
> but with P frames a copy is needed.

The decoder can request a buffer with PRESERVE and then remove the PRESERVE
when it realizes the frame is not a reference frame.

> > It could also downgrade the permissions to remove PRESERVE, in that case.
> It could but then 1 out of 3 filters would need to copy it.
> That is every that needs to keep the reference for more than this
> "iteration"

Why?

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120723/5c02492c/attachment.asc>