[FFmpeg-devel] [PATCH] avpacket: ABI bump additions

[Michael Niedermayer]

On Thu, Jun 03, 2021 at 06:58:56AM +0200, Lynne wrote:
> Apr 26, 2021, 03:27 by andreas.rheinhardt at outlook.com:
> 
> > Lynne:
> >
> >> From 097aed2ac33dda0bb2052d8b0402711ce95079ba Mon Sep 17 00:00:00 2001
> >> From: Lynne <dev at lynne.ee>
> >> Date: Sat, 23 Jan 2021 19:56:18 +0100
> >> Subject: [PATCH] avpacket: ABI bump additions
> >>
> >> ---
> >>  libavcodec/avpacket.c |  5 +++++
> >>  libavcodec/packet.h   | 21 +++++++++++++++++++++
> >>  2 files changed, 26 insertions(+)
> >>
> >> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> >> index e32c467586..03b73b3b53 100644
> >> --- a/libavcodec/avpacket.c
> >> +++ b/libavcodec/avpacket.c
> >> @@ -382,6 +382,10 @@ int av_packet_copy_props(AVPacket *dst, const AVPacket *src)
> >>  dst->flags                = src->flags;
> >>  dst->stream_index         = src->stream_index;
> >>  
> >> +    i = av_buffer_replace(&dst->opaque_ref, src->opaque_ref);
> >> +    if (i < 0)
> >> +        return i;
> >>
> >
> > 1. Don't use i here; add a new variable.
> > 2. Up until now, av_packet_ref() and av_packet_copy_props() treat the
> > destination packet as uninitialized and make no attempt at unreferencing
> > its content; yet you try to reuse opaque_ref. Even worse, you might
> > return potentially dangerous packets: If the properties were
> > uninitialized and av_packet_copy_props() failed, then the caller were
> > not allowed to unreference the packet even when the non-properties were
> > set to sane values. The easiest way to fix this is to move setting
> > opaque ref to the place after initializing side_data below and either
> > set dst->opaque_ref to NULL before av_buffer_replace() or to not use
> > av_buffer_replace(). It may also be best to unref it again if copying
> > side data fails.
> >
> >> +
> >>  dst->side_data            = NULL;
> >>  dst->side_data_elems      = 0;
> >>  for (i = 0; i < src->side_data_elems; i++) {
> >> @@ -403,6 +407,7 @@ int av_packet_copy_props(AVPacket *dst, const AVPacket *src)
> >>  void av_packet_unref(AVPacket *pkt)
> >>  {
> >>  av_packet_free_side_data(pkt);
> >> +    av_buffer_unref(&pkt->opaque_ref);
> >>  av_buffer_unref(&pkt->buf);
> >>  get_packet_defaults(pkt);
> >>  }
> >> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> >> index fad8341c12..c29ad18a2b 100644
> >> --- a/libavcodec/packet.h
> >> +++ b/libavcodec/packet.h
> >> @@ -383,6 +383,27 @@ typedef struct AVPacket {
> >>  int64_t duration;
> >>  
> >>  int64_t pos;                            ///< byte position in stream, -1 if unknown
> >> +
> >> +    /**
> >> +     * for some private data of the user
> >> +     */
> >> +    void *opaque;
> >>
> >
> > The corresponding AVFrame field is copied when copying props.
> >
> 
> Fixed both, thanks. Also copied the time_base field and made av_init_packet()
> initialize all fields.
> Patch attached. Sorry it's taking me so long to work on this and the Vulkan ABI changes.
> 

>  avpacket.c |   14 +++++++++++++-
>  packet.h   |   21 +++++++++++++++++++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> eb01b7d5d36d3bad80b01af192e0d2cb1060ab48  0001-avpacket-ABI-bump-additions.patch
> From 9ba82d84a6686069f20a6c700c3605af8d894976 Mon Sep 17 00:00:00 2001
> From: Lynne <dev at lynne.ee>
> Date: Sat, 23 Jan 2021 19:56:18 +0100

> Subject: [PATCH] avpacket: ABI bump additions
> 
> ---

Can you make the commit message more verbose ?
if i saw this i would have no clue what it is about


[...]

> +    /**
> +     * Time base of the packet's timestamps.
> +     */
> +    AVRational time_base;

IIUC the usecase for this is so users do not need to keep track of AVStreams
to interpret timestamps ?

Assuming iam correct, is this actually enough ?
2 streams could have different points in time for their 0 point too.

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210707/694e2daf/attachment.sig>