{"id":527,"date":"2015-10-15T19:40:08","date_gmt":"2015-10-15T19:40:08","guid":{"rendered":"http:\/\/blogs.gentoo.org\/lu_zero\/?p=527"},"modified":"2015-10-15T19:41:58","modified_gmt":"2015-10-15T19:41:58","slug":"deprecating-avpicture","status":"publish","type":"post","link":"https:\/\/blogs.gentoo.org\/lu_zero\/2015\/10\/15\/deprecating-avpicture\/","title":{"rendered":"Deprecating AVPicture"},"content":{"rendered":"

In Libav we try to clean up the API and make it more regular, this is one of the possibly many articles I write about APIs, this time about deprecating some relic from the past and why we are doing it.<\/p>\n

AVPicture<\/h2>\n

This struct used to store image data using data<\/code> pointers and linesizes<\/code>. It comes from the far past and it looks like this:<\/p>\n

\n
typedef<\/span> struct<\/span> AVPicture<\/span> {<\/span>\n    uint8_t<\/span> *<\/span>data<\/span>[<\/span>AV_NUM_DATA_POINTERS<\/span>];<\/span>\n    int<\/span> linesize<\/span>[<\/span>AV_NUM_DATA_POINTERS<\/span>];<\/span>\n}<\/span> AVPicture<\/span>;<\/span>\n<\/pre>\n<\/div>\n
Once the AVFrame<\/strong> was introduced it was made so it would alias<\/strong> to it and for some time the two structures were actually defined sharing the commond initial fields through a macro.<\/p>\n
The AVFrame then evolved to store both audio<\/strong> and image<\/strong> data, to use AVBuffer to not have to do needless copies and to provide more useful information (e.g. the actual data format<\/strong>), now it looks like:<\/p>\n
\n
typedef<\/span> struct<\/span> AVFrame<\/span> {<\/span>\n    uint8_t<\/span> *<\/span>data<\/span>[<\/span>AV_NUM_DATA_POINTERS<\/span>];<\/span>\n    int<\/span> linesize<\/span>[<\/span>AV_NUM_DATA_POINTERS<\/span>];<\/span>\n\n    uint8_t<\/span> **<\/span>extended_data<\/span>;<\/span>\n\n    int<\/span> width<\/span>,<\/span> height<\/span>;<\/span>\n\n    int<\/span> nb_samples<\/span>;<\/span>\n\n    int<\/span> format<\/span>;<\/span>\n\n    int<\/span> key_frame<\/span>;<\/span>\n\n    enum<\/span> AVPictureType<\/span> pict_type<\/span>;<\/span>\n\n    AVRational<\/span> sample_aspect_ratio<\/span>;<\/span>\n\n    int64_t<\/span> pts<\/span>;<\/span>\n\n    ...<\/span>\n}<\/span> AVFrame<\/span>;<\/span>\n<\/pre>\n<\/div>\n
The image-data manipulation functions moved to the av_image<\/code> namespace and use directly data<\/code> and linesize<\/code> pointers, while the equivalent avpicture<\/code> became a wrapper over them.<\/p>\n
\n
int<\/span> avpicture_fill<\/span>(<\/span>AVPicture<\/span> *<\/span>picture<\/span>,<\/span> uint8_t<\/span> *<\/span>ptr<\/span>,<\/span>\n                   enum<\/span> AVPixelFormat<\/span> pix_fmt<\/span>,<\/span> int<\/span> width<\/span>,<\/span> int<\/span> height<\/span>)<\/span>\n{<\/span>\n    return<\/span> av_image_fill_arrays<\/span>(<\/span>picture<\/span>-><\/span>data<\/span>,<\/span> picture<\/span>-><\/span>linesize<\/span>,<\/span>\n                                ptr<\/span>,<\/span> pix_fmt<\/span>,<\/span> width<\/span>,<\/span> height<\/span>,<\/span> 1<\/span>);<\/span>\n}<\/span>\n\nint<\/span> avpicture_layout<\/span>(<\/span>const<\/span> AVPicture<\/span>*<\/span> src<\/span>,<\/span> enum<\/span> AVPixelFormat<\/span> pix_fmt<\/span>,<\/span>\n                     int<\/span> width<\/span>,<\/span> int<\/span> height<\/span>,<\/span>\n                     unsigned<\/span> char<\/span> *<\/span>dest<\/span>,<\/span> int<\/span> dest_size<\/span>)<\/span>\n{<\/span>\n    return<\/span> av_image_copy_to_buffer<\/span>(<\/span>dest<\/span>,<\/span> dest_size<\/span>,<\/span>\n                                   src<\/span>-><\/span>data<\/span>,<\/span> src<\/span>-><\/span>linesize<\/span>,<\/span>\n                                   pix_fmt<\/span>,<\/span> width<\/span>,<\/span> height<\/span>,<\/span> 1<\/span>);<\/span>\n}<\/span>\n\n...<\/span>\n<\/pre>\n<\/div>\n
It is also used in the subtitle abstraction:<\/p>\n
\n
typedef<\/span> struct<\/span> AVSubtitleRect<\/span> {<\/span>\n    int<\/span> x<\/span>,<\/span> y<\/span>,<\/span> w<\/span>,<\/span> h<\/span>;<\/span>\n    int<\/span> nb_colors<\/span>;<\/span>\n\n    AVPicture<\/span> pict<\/span>;<\/span>\n    enum<\/span> AVSubtitleType<\/span> type<\/span>;<\/span>\n\n    char<\/span> *<\/span>text<\/span>;<\/span>\n    char<\/span> *<\/span>ass<\/span>;<\/span>\n    int<\/span> flags<\/span>;<\/span>\n}<\/span> AVSubtitleRect<\/span>;<\/span>\n<\/pre>\n<\/div>\n
And to crudely pass AVFrame from the decoder level to the muxer level, for certain rawvideo muxers by doing something such as:<\/p>\n
\n
    pkt<\/span>.<\/span>data<\/span>   =<\/span> (<\/span>uint8_t<\/span> *<\/span>)<\/span>frame<\/span>;<\/span>\n    pkt<\/span>.<\/span>size<\/span>   =<\/span>  sizeof<\/span>(<\/span>AVPicture<\/span>);<\/span>\n<\/pre>\n<\/div>\n
AVPicture problems<\/h2>\n
In the codebase its remaining usage is dubious at best:<\/p>\n
AVFrame as AVPicture<\/h3>\n
In some codecs the AVFrame<\/code> produced or consumed are casted as AVPicture<\/code> and passed to avpicture<\/code> functions instead
\nof directly use the av_image<\/code> functions.<\/p>\n
AVSubtitleRect<\/h3>\n
For the subtitle codecs, accessing the Rect data requires a pointless indirection, usually something like:<\/p>\n
\n
    wrap3<\/span> =<\/span> rect<\/span>-><\/span>pict<\/span>.<\/span>linesize<\/span>[<\/span>0<\/span>];<\/span>\n    p<\/span> =<\/span> rect<\/span>-><\/span>pict<\/span>.<\/span>data<\/span>[<\/span>0<\/span>];<\/span>\n    pal<\/span> =<\/span> (<\/span>const<\/span> uint32_t<\/span> *<\/span>)<\/span>rect<\/span>-><\/span>pict<\/span>.<\/span>data<\/span>[<\/span>1<\/span>];<\/span>  \/* Now in YCrCb! *\/<\/span>\n<\/pre>\n<\/div>\n
AVFMT_RAWPICTURE<\/h3>\n
Copying memory from a buffer to another when can be avoided is consider a major sin (“memcpy<\/code> is murder”) since it is a costly operation in itself and usually it invalidates the cache if we are talking about large buffers.<\/p>\n
Certain muxers for rawvideo, try to spare a memcpy<\/code> and thus avoid a “murder” by not copying the AVFrame data to the AVPacket.<\/p>\n
The idea in itself is simple enough, store the AVFrame pointer as if it would point a flat array, consider the data size as the AVPicture size and hope that the data pointed by the AVFrame remains valid while the AVPacket is consumed.<\/p>\n
Simple and faulty<\/strong>: with the AVFrame<\/code> ref-counted API codecs may use a Pool of AVFrame<\/code>s and reuse<\/strong> them.
\nIt can lead to surprising results because the buffer gets updated before the AVPacket is actually written.
\nIf the frame referenced changes dimensions or gets deallocated it could even lead to crashes.<\/p>\n
Definitely not a great idea.<\/p>\n
Solutions<\/h2>\n
Vittorio, wm4 and I worked together to fix the problems. Radically.<\/p>\n
AVFrame as AVPicture<\/h4>\n
The av_image<\/code> functions are now used when needed.
\nSome pointless copies got replaced by av_frame_ref<\/code>, leading to less memory usage and simpler code.<\/p>\n
No AVPictures are left in the video codecs.<\/p>\n
AVSubtitle<\/h4>\n
The AVSubtitleRect is updated to have simple data<\/code> and linesize<\/code> fields and each codec is updated to keep the AVPicture<\/code> and the new fields in sync during the deprecation window.<\/p>\n
The code is already a little easier to follow now.<\/p>\n
AVFMT_RAWPICTURE<\/h4>\n
Just dropping the “feature” would be a problem since those muxers are widely used in FATE<\/a> and the time the additional copy takes adds up to quite a lot. Your regression test must be as quick as possible.<\/p>\n
I wrote a safer wrapper pseudo-codec that leverages the fact that both AVPacket<\/code> and AVFrame<\/code> use a ref-counted system:<\/p>\n