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

[Michael Niedermayer]

On Sat, Jul 21, 2012 at 12:39:13PM +0200, Nicolas George wrote:
> Le quartidi 4 thermidor, an CCXX, Michael Niedermayer a écrit :
> > i dont think access permissions have been considered/implemented
> > very well in most filters. So some existing filters are
> > maybe not a good example and work more by luck than correctness/
> > consistency
> 
> 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"?
It certainly does not work with the current API / current definition
of the permissions.
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.
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


> 
> > The problem i see is that the permissions no longer are well
> > defined as in that they are no longer sufficient on their own to say
> > what can be done with a reference
> > But maybe iam misunderstanding 
> 
> That is absolutely true, but I do not think this is a problem. Permissions
> in lavfi must not be seen as an access control mechanism:

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


> 
> [ drawtext @ 0xdeadbeef ] No WRITE permissions, too bad
> 
> Permissions in lavfi are a mechanism to ensure that filters have always what
> they need without walking on each other's feet.
> 
> The only problem I see with that is that it is very surprising and
> counterintuitive, and as such requires careful documentation and will
> confuse beginners even so. That would probably a good reason to redesign it
> if it were not for the compatibility issues (both backward and with the
> fork).

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



> > A filter may receive a "PRESERVE" reference without requesting one.
> 
> I do not see a case where it would be useful.

consider a mpeg1/2/4 decoder

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.


> 
> > also a filter might request one and later no longer need it
> > (for example a decoder might have used it as reference frame but
> > knows it wont be used as reference frame again) In this case
> > it could pass the reference on with PRESERVE attribute.
> 
> 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"

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- 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/20120722/c753feb3/attachment.asc>