Message ID | 20210201224421.1395-1-jamrial@gmail.com |
---|---|
Headers | show |
Series | deprecate av_init_packet() and sizeof(AVPacket) as part of the ABI | expand |
On Mon, 01. Feb 19:44, James Almer wrote: > As the subject says, this puts AVPacket in line with AVFrame, allowing > easier extensibility of the struct, and preventing potential undefined behavior > in some cases when packets are used uninitialized on stack. > > This set adapts only a few modules, examples, and the tools, as those were the > most complex and the ones that better reflect how AVPackets must be used after > the deprecation period. > I have a branch ready removing every other instance of av_init_packet() in the > tree, and all uses of AVPacket on stack i could find. They are pretty trivial, > so i wont bother spamming the ML with 40+ patches until the core changes are > upstreamed. It's also not exhaustive, as i'm sure i missed a few cases of stack > usage, but they can be removed any time between the deprecation of public > sizeof(AVPacket) and it's effective removal 2+ years from now. > Then there's the few cases where an AVPacket is part of another public struct, > as is the case of AVPacketList, or attached_pic in AVSteam. The latter i could > adapt right now, but it would mean a lot of preprocessor checks littering the > codebase until removed, so i refrained from doing so. > > James Almer (10): > avformat/mux: use av_packet_alloc() to allocate packets > avformat/movenc: use av_packet_alloc() to allocate packets > avformat/utils: use av_packet_alloc() to allocate packets > doc/examples/demuxing_decoding: use av_packet_alloc() to allocate > packets > doc/examples/transcode_aac: use av_packet_alloc() to allocate packets > doc/examples/transcoding: use av_packet_alloc() to allocate packets > fftools/ffprobe: use av_packet_alloc() to allocate packets > fftools/ffmpeg: use av_packet_alloc() to allocate packets > fftools/ffplay: use av_packet_alloc() to allocate packets > avcodec/packet: deprecate av_init_packet() > > doc/examples/demuxing_decoding.c | 25 +-- > doc/examples/transcode_aac.c | 46 +++-- > doc/examples/transcoding.c | 48 +++-- > fftools/ffmpeg.c | 318 ++++++++++++++++--------------- > fftools/ffmpeg.h | 4 + > fftools/ffmpeg_opt.c | 5 +- > fftools/ffplay.c | 222 ++++++++++++--------- > fftools/ffprobe.c | 34 ++-- > libavcodec/avpacket.c | 23 ++- > libavcodec/packet.h | 23 ++- > libavcodec/version.h | 3 + > libavformat/avformat.h | 4 + > libavformat/internal.h | 6 + > libavformat/movenc.c | 101 ++++++---- > libavformat/movenc.h | 2 + > libavformat/movenchint.c | 19 +- > libavformat/mux.c | 44 +++-- > libavformat/options.c | 9 + > libavformat/utils.c | 106 ++++++----- > 19 files changed, 613 insertions(+), 429 deletions(-) > Hi James, In case you didn't see, there is some weirdness with fate over the commits: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=3189
On 2/2/2021 4:33 PM, Andriy Gelman wrote: > On Mon, 01. Feb 19:44, James Almer wrote: >> As the subject says, this puts AVPacket in line with AVFrame, allowing >> easier extensibility of the struct, and preventing potential undefined behavior >> in some cases when packets are used uninitialized on stack. >> >> This set adapts only a few modules, examples, and the tools, as those were the >> most complex and the ones that better reflect how AVPackets must be used after >> the deprecation period. >> I have a branch ready removing every other instance of av_init_packet() in the >> tree, and all uses of AVPacket on stack i could find. They are pretty trivial, >> so i wont bother spamming the ML with 40+ patches until the core changes are >> upstreamed. It's also not exhaustive, as i'm sure i missed a few cases of stack >> usage, but they can be removed any time between the deprecation of public >> sizeof(AVPacket) and it's effective removal 2+ years from now. >> Then there's the few cases where an AVPacket is part of another public struct, >> as is the case of AVPacketList, or attached_pic in AVSteam. The latter i could >> adapt right now, but it would mean a lot of preprocessor checks littering the >> codebase until removed, so i refrained from doing so. >> >> James Almer (10): >> avformat/mux: use av_packet_alloc() to allocate packets >> avformat/movenc: use av_packet_alloc() to allocate packets >> avformat/utils: use av_packet_alloc() to allocate packets >> doc/examples/demuxing_decoding: use av_packet_alloc() to allocate >> packets >> doc/examples/transcode_aac: use av_packet_alloc() to allocate packets >> doc/examples/transcoding: use av_packet_alloc() to allocate packets >> fftools/ffprobe: use av_packet_alloc() to allocate packets >> fftools/ffmpeg: use av_packet_alloc() to allocate packets >> fftools/ffplay: use av_packet_alloc() to allocate packets >> avcodec/packet: deprecate av_init_packet() >> >> doc/examples/demuxing_decoding.c | 25 +-- >> doc/examples/transcode_aac.c | 46 +++-- >> doc/examples/transcoding.c | 48 +++-- >> fftools/ffmpeg.c | 318 ++++++++++++++++--------------- >> fftools/ffmpeg.h | 4 + >> fftools/ffmpeg_opt.c | 5 +- >> fftools/ffplay.c | 222 ++++++++++++--------- >> fftools/ffprobe.c | 34 ++-- >> libavcodec/avpacket.c | 23 ++- >> libavcodec/packet.h | 23 ++- >> libavcodec/version.h | 3 + >> libavformat/avformat.h | 4 + >> libavformat/internal.h | 6 + >> libavformat/movenc.c | 101 ++++++---- >> libavformat/movenc.h | 2 + >> libavformat/movenchint.c | 19 +- >> libavformat/mux.c | 44 +++-- >> libavformat/options.c | 9 + >> libavformat/utils.c | 106 ++++++----- >> 19 files changed, 613 insertions(+), 429 deletions(-) >> > > Hi James, > > In case you didn't see, there is some weirdness with fate over the commits: > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=3189 Yeah, i messed up and placed a patch using a FF_API_ define before the patch that introduced it :p With all 10 patches applied it works as intended, but patch 10/10 needs to go in first to ensure the above doesn't happen.