On 19/10/15 14:48, wm4 wrote:
> On Sun, 18 Oct 2015 17:59:47 +0200
> Luca Barbato <[email protected]> wrote:
>
>> As documented, `av_dup_packet` is broken by design, `av_packet_ref`
>> matches the AVFrame ref-counted API and can be safely used instead.
>>
>> Deprecate `av_dup_packet`.
>> ---
>> avplay.c | 17 +++++++++++------
>> libavcodec/avcodec.h | 5 ++++-
>> libavcodec/avpacket.c | 4 ++++
>> libavformat/matroskaenc.c | 7 +------
>> libavformat/mux.c | 10 ++++------
>> 5 files changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/avplay.c b/avplay.c
>> index 7f1bbb7..5fe5f2f 100644
>> --- a/avplay.c
>> +++ b/avplay.c
>> @@ -319,15 +319,20 @@ static void packet_queue_end(PacketQueue *q)
>> static int packet_queue_put(PacketQueue *q, AVPacket *pkt)
>> {
>> AVPacketList *pkt1;
>> -
>> - /* duplicate the packet */
>> - if (pkt != &flush_pkt && av_dup_packet(pkt) < 0)
>> - return -1;
>> + int ret;
>>
>> pkt1 = av_malloc(sizeof(AVPacketList));
>> if (!pkt1)
>> - return -1;
>> - pkt1->pkt = *pkt;
>> + return AVERROR(ENOMEM);
>> +
>> + /* duplicate the packet */
>> + if (pkt != &flush_pkt) {
>> + if (ret = av_packet_ref(&pkt1->pkt, pkt)) < 0)
>
> Wait, you're calling av_packet_ref() on an uninitialized packet? But
> the destination packet must be initialized before calling
> av_packet_ref(). (I think I insisted that this kind of stuff should be
> added to the doxygen when av_packet_ref() was added...)
>
> So use av_mallocz().
You are right and that call can be just dropped now.
Moving it to patch 2.
> Also add that av_dup_packet() after a read_frame() is not needed
> anymore, and packets are always refcounted. Put yourself into the
> situation of someone who tries to find out what they should do with an
> av_dup_packet() call in code they're not entirely familiar with
> (maintainers).
Not sure if doing that in patch 2 now, but sure, makes sense.
>> if (codec_type == AVMEDIA_TYPE_AUDIO) {
>> - mkv->cur_audio_pkt = *pkt;
>> - if (pkt->buf) {
>> - mkv->cur_audio_pkt.buf = av_buffer_ref(pkt->buf);
>> - ret = mkv->cur_audio_pkt.buf ? 0 : AVERROR(ENOMEM);
>> - } else
>> - ret = av_dup_packet(&mkv->cur_audio_pkt);
>> + ret = av_packet_ref(&mkv->cur_audio_pkt, pkt);
>
> Is mkv->cur_audio_pkt initialized?
>
yes.
>> } else
>> ret = mkv_write_packet_internal(s, pkt);
>> return ret;
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index e86d202..4a81e36 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -416,12 +416,8 @@ int ff_interleave_add_packet(AVFormatContext *s,
>> AVPacket *pkt,
>> this_pktl = av_mallocz(sizeof(AVPacketList));
>> if (!this_pktl)
>> return AVERROR(ENOMEM);
>> - this_pktl->pkt = *pkt;
>> - pkt->buf = NULL;
>> - pkt->side_data = NULL;
>> - pkt->side_data_elems = 0;
>> - // Duplicate the packet if it uses non-allocated memory
>> - if ((ret = av_dup_packet(&this_pktl->pkt)) < 0) {
>> +
>> + if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
>
> Same here.
And yes.
>
Thanks a lot!
lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
