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

[Michael Niedermayer]

On Mon, Jul 23, 2012 at 08:27:40PM +0200, Nicolas George wrote:
> [ 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.

yes


> 
> > 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.

I think we maybe have a different understaning of "my permissions" vs.
"the previous filters permissions"

if the previous filter holds a PRESERVE+WRITE (random example) ref
then this is kind of a exclusive locked write buffer. If now the
previous filter passes this to us it becomes our reference and our
lock. so rej_perms= PRESERVE would be quite odd.

maybe theres a problem with my view above but i dont see any ATM ...


> # 
> # 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.

yes, but considering PRESERVE cannot be added back later (there could
be others having write references at that point in the general case)
it would be preferable not to do that IMHO


> 
> > > 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?

if we assume WRITE is defined by allowing the holder of the reference
WRITE access
and we assume PRESERVE is defined by giving the holder exclusive
WRITE access or noone write access (if the holder doesnt have write)

then droping PRESERVE is equivalent to droping a write lock and
allowing everyone to write, but theres no way to obtain a new
write lock so a filter that wants to keep the reference for longer
has to make a copy, otherwise some other filter could "legally"
write into it for its own purposes (maybe a static buffer of a
conditional replenishment decoder)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/53660148/attachment.asc>