Re: [FFmpeg-devel] [PATCH 00/10 v4][RFC] AVCodecContext and AVCodecParameters side data

[Anton Khirnov <anton@khirnov.net>]

Quoting James Almer (2023-09-06 19:44:20)
> Changes since the previous version:
> - Removed the AVPacketSideDataSet from AVCodecContext, using instead the
>   existing coded_side_data field, at Anton's request.
> 
> I still prefer using a new field of type AVPacketSideDataSet, given that the
> user can currently only fill coded_side_data manually (See the implementation
> in the codecpar copy functions, which non lavf users would need to replicate),
> and more so considering coded_side_data is a field pretty much nothing but lavf
> uses.
> 
> Opinions are very welcome and needed.

To provide more context on the issue:

AVPackets can have side data attached to them, in the form of
AVPacket.{side_data,side_data_elems} array+count.

Some of this side data can occasionally be stored in global headers,
e.g. in containers or extradata. We have some limited support for this:
* AVStream.{side_data,nb_side_data} array+count allows demuxers to
  export this to user, and muxers to accept it from the user
* AVCodecContext.{coded_side_data,nb_coded_side_data} allows decoders to
  accept it from the user (in principle, in practice it is not used),
  and encoders to export it to the user (this is used, but not very
  much)

The broad idea for this set is adding stream-global "coded/packet" side
data to AVCodecParameters, so that it can be naturally communicated from
demuxers to bitstream filters to decoders, and from encoders to
bitstream filters to muxers. Since AVStream already contains an
AVCodecParameters instance, the abovementioned AVStream.side_data
becomes redundant and is deprecated, the muxer/demuxer stream-global
side data would be passed through AVStream.codecpar.

The original version proposed by James adds a new struct, that bundles
the side data array+count, and a set of helpers operating on this
struct. Then this new struct is added to AVCodecParameters and
AVCodecContext, which replaces the abovementioned AVStream and
AVCodecContext side data array+count. Notably AVPacket is left as-is,
because its side data is widely used and replacing array+count by this
new struct would be a very intrusive break for little gain.

My objections to this approach are
* coded side data is now stored differently in AVPacket and in
  everything else
* the case for replacing AVCodecContext.coded_side_data does not seem
  sufficiently strong to me

My alternative suggestion was:
* avoid adding a new struct (which merely bundles array+count)
* add a set of helpers that accept array+count as parameters and operate
  on them
* use these helpers for all operations on side data across AVPacket,
  AVCodecContext, and AVCodecParameters

I have a moderately strong preference for this approach, but James
disagrees. More opinions are very much welcome.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".