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

[Rémi Denis-Courmont]

Le mardi 26 juin 2018, 12:28:15 EEST Jean-Baptiste Kempf a écrit :
> > Flags is actually the most generic of all meta-data field in there, as it
> > carries the generic corrupt flag and the mostly generic discontinuity
> > flag.
> > The remaining space is left for ad-hoc use for convenience.
> 
> Absolutely not. 14 flags out of 14 are about video/audio/demux data.

"I don't like your tone."

And this plain patently false. The corrupt flag used in input stream and 
discontinuity flag used in audio output. Just like I already wrote.

> > If anything, the least generic of the grandfathered fields is DTS.
> 
> i_dts, i_pts and i_length are vlc_tick_t

And?

i_dts is the only field, as far as I am aware, that is only used for video 
coded blocks. All other fields are used in at least 2 of the block_t use 
cases.

And while I don't like the fact that i_nb_samples is used outside aout, the 
matter of the fact is that it is used also in demux/packetizer.

> > > If we want to have a block as a generic input data structure, we can
> > > have a
> > > different structure,
> > 
> > Yeah, if you want to extend the existing generic block data  structure,
> > you
> > can make a sub-class.
> 
> You removed the audio sample structure, IIRC.

And?

> You cannot then claim that block is generic.

That's ad hominem and that's also fallacious.

The reuse of block_t in aout changes nothing here. It was already used by both 
input stream as plain raw data and by ES output as timed ES coded data. That 
probably even predates me joining the project.

Futhermore, support for audio pass-through means that coded audio and decoded 
audio are effectively the same thing. aout_buffer_t was only a failed attempt 
to use the stack for buffers to make the audio output thread soft real-time.

> > > but that is not what we have currently.
> > 
> > It is.
> 
> It is not. At least 5 fields out of 11 are video/audio/demux specific.
> And it has always been like that, since 10 years+

It is. block_t has been used for input stream for at least 15 years. It's even 
(ab)used for key events nowadays (also not me).

> > Just check all the places that block_t is used for. It's far from only
> > ES coded data.
> 
> How is that a justification?

How is that not a justification? If the reality of the source code is not a 
justification, then I wonder what is a justification.

Certainly not your fallacies.

> Misusing a structure and then claiming that we cannot extend it for its
> original purpose is not good.

Indeed. block_t  is a blob of binary data, with a few pieces of immediate 
meta-data. This patch misuses it as an associative array of blobs.

> Also, noone said it was ES coded data only, but also input_stream data.
> And that's good because we need ancillary in both ES and input_stream data.

Even leaving aside design/taste considerations, input streams do not use 
block_t internally and do not treat block boundaries as meaningful, so there 
are no ways that this will work.

-- 
Rémi Denis-Courmont