[FFmpeg-devel] [RFC] Request for pixdesc API review

[Stefano Sabatini]

On date Sunday 2009-11-08 18:35:43 +0100, Stefano Sabatini encoded:
> Hi,
> 
> we recently got completed all the definitions for the pixel format
> descriptors in libavutil/pixdesc.{h,c}.
> 
> pixdescs API is the designated replacement for the hackish PixFmtInfo
> system defined in libavcodec/imgconvert.c, and it may be used, once
> completed, at least by lavfi and lsws code.
> 
> Since now mainly just Micheal and me worked on it, before pixdescs API
> can be published, we need, as suggested by Michael, a review of the
> new API from some other developer, especially to get sure that it is
> clear enough and we are not missing pieces.
> 
> See the thread:
> http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/99224
> 
> In particular I plan to move it back to lavc and start to replace the
> functions using PixFmtInfo with corresponding functions using
> pixdescs, then once this will be done I plan to move it again to lavu
> and enable it.
> 
> Comments are very welcome.

Some considerations.

* In attachment a patch to make more clear the use of the log2_chroma_w
  and log2_chroma_h fields.

  The doxy addition reflects a limitation we have in the API. This is
  not a problem with all the currently supported pixel formats, but
  could be a problem when supporting other possibly weird formats.

  I don't think this will be a problem, at least I have not experience
  of a pixel format which would break with our constraints, but I
  would like to hear the opinion of someone with more expertise with
  pixel formats than me.

* I also wonder if:
  int av_get_bits_per_pixel(enum PixelFormat pix_fmt);

  would be preferable to the current:
  bits = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);

  The first signature is easier to use in the most common scenario,
  when the user has the pix_fmt and she wants to extracts the bits:
  bits = av_get_bits_per_pixel(pix_fmt);

  which is far simpler than:
  bits = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);

* In order to prove that the API is expressive enough, I rewrote
  ff_fill_linesize() and ff_fill_pointer() in imgconvert.c to make use
  of pixdescs, the experiment resulted in code more generic and
  functionally equivalent to the old one (it requires some patches I
  posted today), sample patch attached.

That said I consider the pixdesc API good enough to be exported.

Regards.
-- 
FFmpeg = Foolish & Frightening Magnificient Power Eccentric Gem
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clarify-log2-chroma.patch
Type: text/x-diff
Size: 906 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091116/2d01698a/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: use-pixdesc-for-fill-linesize.patch
Type: text/x-diff
Size: 4570 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091116/2d01698a/attachment-0001.patch>