[vlc-devel] [PATCH 2/2] vlc_block: store per block ancillary

[Rémi Denis-Courmont]

Le keskiviikkona 27. kesäkuuta 2018, 15.54.31 EEST Denis Charmet a écrit :
> Isn't VLC all about handling corner cases :). I think we all agree that
> we need a way to handle that and the debate is just about the how.

I don't call getting blamed for design decisions that I did not make, and 
getting all objections rejected on the sole basis of sloth a debate.

> So do you think that we should re-split block into more specific data
> structures?

That is one of at least three possible alternatives proposed in this thread. 
Four if you count new ES category (but I would not).

> I would conceptually agree with this, I'm just afraid of the amount of
> work it implies with the duplication/evolution of all the helpers.

There are _no_ options that does not involve significant helper changes.

As much as François likes to pretend that his patch works magically, it does 
not. It's missing a truck load of updates and is heavily under-specified, some 
of which were already mentioned and discarded multiple times.

> >  Not only that but block_t is not designed to cope with non-immediate
> > 
> > writable metadata fields. That causes leaks abd possibly heap
> > corruption.
> 
> Forgive my lack of formal CS education but what do you mean by
> "non-immediate"?

I mean indirect.

More accurately, block_t can only feature allocations done by the allocator as 
things stand.

> If the block releasing function properly cleans those side data, why
> would it leak and/or corrupt the heap?

Obviously because nobody frees the allocation, or because it gets copied by 
value.

> >  And even when all the boilerplate is added to fix those leaks (it's
> > 
> > not so easy anymore). Then there is still the problem that you cannot
> > "generically" pass side data through the block_t users. Things like
> > block chain, timeshift, packetizers will need to handle it explicitly
> > (it's even harder than fixing the leaks).
> 
> I don't mind the out-of-band way, I just don't see how we could
> reliably link those data to the relevant frame considering that
> multimedia is sheer madness, unless we add a generic UID field to the
> block, but then should the module ask the core "give me all the data
> linked to this UID".

Passing it in-band solves that problem either. How do you handle side data in 
a block chain - the common case for packetizers? How does the generic block 
chain helper know which byte each side datum is associated with? Maybe it even 
needs to packetize the side data - not something a generic helper can do.

For the umpteenth time, you _cannot_ pass data through. That concept looks 
easy but it is totally flawed.

> Or could it be achieved with an es_out_send_side_data API to keep it
> attached to their mother ES... But then how to keep them in the fifo,
> when should we release them if they are not consumed by a module.

Again, that difficulty is intrinsic. You cannot just paper over it like this 
patch tries and fails to.

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/