[FFmpeg-devel] [PATCH 1/5] avformat/utils: make AVPacketList helper functions shared
Michael Niedermayer
michael at niedermayer.cc
Wed Mar 28 00:38:26 EEST 2018
On Tue, Mar 27, 2018 at 06:17:09PM -0300, James Almer wrote:
> On 3/27/2018 5:22 PM, Michael Niedermayer wrote:
> > On Mon, Mar 26, 2018 at 03:02:35PM -0300, James Almer wrote:
> >> Based on a patch by Luca Barbato.
> >>
> >> Signed-off-by: James Almer <jamrial at gmail.com>
> >> ---
> >> libavformat/internal.h | 35 ++++++++++++++++++++++++++++++++++
> >> libavformat/utils.c | 51 +++++++++++++++++++++++++++-----------------------
> >> 2 files changed, 63 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/libavformat/internal.h b/libavformat/internal.h
> >> index a020b1b417..7e1b24ebe6 100644
> >> --- a/libavformat/internal.h
> >> +++ b/libavformat/internal.h
> >> @@ -731,6 +731,41 @@ int ff_unlock_avformat(void);
> >> */
> >> void ff_format_set_url(AVFormatContext *s, char *url);
> >>
> >> +/**
> >> + * Append an AVPacket to the list.
> >> + *
> >
> >> + * @param head List head
> >> + * @param tail List tail
> >
> > about the terminology
> > Shouldnt this be a single List element or 2 ListEntry elements ?
>
> What do you suggest to write instead? I don't think the current
> terminology is confusing. It's two AVPacketList arguments, one pointing
> to the head/first item of the list and the other to the tail/last item.
hmm i guess the simplest would be
@param head List head element.
It feels a bit inelegant to have to keep track of 2 elements but changing
this is probably too much work
[...]
> >
> >
> >> + */
> >> +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
> >> + AVPacket *pkt, int ref);
> >
> > ref should not be a int but a enum
> > ff_packet_list_put(,,,1)
> > ff_packet_list_put(,,,0)
> >
> > is alot more confusing to a new developer than with english words
> > the code should be self explanatory not requiring looking up the
> > documentation
>
> Adding an enum like this seems extreme for an internal API, but ok.
yes, i was thinking a bit ahead.
For a internal API it is not that important but this API is quite usefull
i think theres a good chance it might become public in the future.
But even for a internal API named constants seem better IMHO
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180327/fb36db2e/attachment.sig>
More information about the ffmpeg-devel
mailing list