[FFmpeg-devel] [PATCH] Add av_copy_packet()

[Andrey Utkin]

2012/9/20 Michael Niedermayer <michaelni at gmx.at>:
> the av_* prefix means public API, yet its static thus not public,
> i think the av_ prefix should be removed

Valid point, will be fixed.

>>  {
>> -    AVPacket tmp_pkt;
>> -
>> -    if (pkt->destruct == NULL && pkt->data) {
>> -        tmp_pkt = *pkt;
>> +    dst->data      = NULL;
>> +    dst->side_data = NULL;
>> +    DUP_DATA(dst->data, src->data, dst->size, 1);
>> +    dst->destruct = av_destruct_packet;
>
> I think either the copy should be implemented with the current
> av_dup_packet() making the diff much simpler or

av_dup_packet() copies stuff at the particular condition. I see no
sense to reuse av_dup_packet(), because we would have to "simulate"
this condition, which is not something fundamental and can change in
future (it has already changed once since my question about
av_dup_packet() purpose). I mean in way you propose, the code will be
fragile.

> the redundant operations like first copying and then reseting data
> side data, destruct should be avoided.

DUP_DATA+reset can be replaced by straight alloc with failure check, i
can do it in separate commit. It's just such change will not look
elegant, because big part of DUP_DATA would be copied into code. By
this reason i did not include such change here.

> also maybe you could split the diff so that variable renamings
> (pkt->dst, "tmp_pkt."->"src->") would be seperate from functional
> changes. This would it much clearer what is changed in each patch.

Ok.

-- 
Andrey Utkin