[FFmpeg-devel] [PATCH] avformat: add apic to AVStream

James Almer jamrial at gmail.com
Sun Mar 28 23:42:07 EEST 2021


On 3/28/2021 5:32 PM, Marton Balint wrote:
> 
> 
> On Sun, 28 Mar 2021, James Almer wrote:
> 
>> As a replacement for attached_pic, which is in turn deprecated and 
>> scheduled
>> for removal.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> TODO: APIChanges entry and version bump.
>>
>> Decided to use the name apic for the field, since it's how id3v2 and 
>> other
>> formats call it.
> 
> Fine by me.
> 
>> diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
>> index f15cfa877a..f771df3fc2 100644
>> --- a/libavformat/flac_picture.c
>> +++ b/libavformat/flac_picture.c
>> @@ -165,12 +165,20 @@ int ff_flac_parse_picture(AVFormatContext *s, 
>> uint8_t *buf, int buf_size, int tr
>>         RETURN_ERROR(AVERROR(ENOMEM));
>>     }
>>
>> -    av_packet_unref(&st->attached_pic);
>> -    st->attached_pic.buf          = data;
>> -    st->attached_pic.data         = data->data;
>> -    st->attached_pic.size         = len;
>> -    st->attached_pic.stream_index = st->index;
>> -    st->attached_pic.flags       |= AV_PKT_FLAG_KEY;
>> +    av_packet_unref(st->apic);
>> +    st->apic->buf          = data;
>> +    st->apic->data         = data->data;
>> +    st->apic->size         = len;
>> +    st->apic->stream_index = st->index;
>> +    st->apic->flags       |= AV_PKT_FLAG_KEY;
>> +    data = NULL;
>> +#if FF_API_ATTACHED_PIC
>> +FF_DISABLE_DEPRECATION_WARNINGS
> 
> If st->apic is unreffed above, shouldn't you also unref st->attached_pic 
> here?

No, i can in fact just remove it, since the stream was allocated right 
before it.

I think this unref is there because it was previously an 
av_init_packet(), which i replaced as part of my deprecation work to 
achieve the same effect of initializing st->attached_pic (a zeroed 
packet), for the sake of setting fields like pts to no_pts (This is not 
needed for st->apic, which is properly initialized when it's allocated).
The av_packet_ref() below will set all st->attached_pic fields.

> 
> Thanks,
> Marton
> 
>> +    ret = av_packet_ref(&st->attached_pic, st->apic);
>> +    if (ret < 0)
>> +        goto fail;
>> +FF_ENABLE_DEPRECATION_WARNINGS
>> +#endif
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".



More information about the ffmpeg-devel mailing list