Re: [libav-devel] [PATCH 5/6] lavc: Add private API to manipulate AVPacketList

2014-09-01 Thread Diego Biurrun
On Sun, Aug 31, 2014 at 09:24:28PM +0200, Luca Barbato wrote:
 --- a/libavcodec/avpacket.c
 +++ b/libavcodec/avpacket.c
 @@ -393,3 +394,58 @@ void av_packet_rescale_ts(AVPacket *pkt, AVRational 
 src_tb, AVRational dst_tb)
 +
 +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt)
 +{
 +AVPacketList *pl = av_mallocz(sizeof(AVPacketList));
 +int ret;

Doesn't this warn because of mixed declarations and statements?
I suggest swapping the lines.

 --- a/libavcodec/internal.h
 +++ b/libavcodec/internal.h
 @@ -200,4 +200,40 @@ int ff_get_format(AVCodecContext *avctx, const enum 
 AVPixelFormat *fmt);
  
 +/*
 + * Append an AVPacket to the list creating a new reference
 + * to it.
 + *
 + * @param head List head
 + * @param tail List tail
 + * @param pkt  The packet being appended
 + * @return  0 on failure and 0 on success.
 + */
 +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt);
 +
 +/**
 + * Remove the oldest AVPacket in the list and return it.
 + *
 + * @note The pkt will be overwritten completely. The caller
 + *   owns the packet and must unref it by itself.
 + *
 + * @see av_packet_unref av_packet_ref
 + *
 + * @param head List head.
 + * @param tail List tail.
 + * @param pkt  Pointer to an initialized AVPacket struct
 + * @return  0 on failure and 0 on success.
 + */
 +int ff_packet_list_get(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt);
 +
 +/**
 + * Wipe the list and unref all the packets in it.
 + *
 + * @param head List head.
 + * @param tail List tail.
 + */
 +void ff_packet_list_free(AVPacketList **head, AVPacketList **tail);

Speaking of the list in all these descriptions is confusing, the reader
does not yet know the signature of the function.  A list of AVPackets
or similar would be less confusing.

Drop the periods from parameter descriptions that are not sentences, you
only have periods on some of them anyway.

 --- a/libavformat/avformat.h
 +++ b/libavformat/avformat.h
 @@ -1254,12 +1254,6 @@ typedef struct AVFormatContext {
  
 -typedef struct AVPacketList {
 -AVPacket pkt;
 -struct AVPacketList *next;
 -} AVPacketList;

API change - where is the version and deprecation dance?

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 5/6] lavc: Add private API to manipulate AVPacketList

2014-09-01 Thread Luca Barbato
On 01/09/14 12:47, Diego Biurrun wrote:
 On Sun, Aug 31, 2014 at 09:24:28PM +0200, Luca Barbato wrote:
 --- a/libavcodec/avpacket.c
 +++ b/libavcodec/avpacket.c
 @@ -393,3 +394,58 @@ void av_packet_rescale_ts(AVPacket *pkt, AVRational 
 src_tb, AVRational dst_tb)
 +
 +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt)
 +{
 +AVPacketList *pl = av_mallocz(sizeof(AVPacketList));
 +int ret;
 
 Doesn't this warn because of mixed declarations and statements?
 I suggest swapping the lines.
 
 --- a/libavcodec/internal.h
 +++ b/libavcodec/internal.h
 @@ -200,4 +200,40 @@ int ff_get_format(AVCodecContext *avctx, const enum 
 AVPixelFormat *fmt);
  
 +/*
 + * Append an AVPacket to the list creating a new reference
 + * to it.
 + *
 + * @param head List head
 + * @param tail List tail
 + * @param pkt  The packet being appended
 + * @return  0 on failure and 0 on success.
 + */
 +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt);
 +
 +/**
 + * Remove the oldest AVPacket in the list and return it.
 + *
 + * @note The pkt will be overwritten completely. The caller
 + *   owns the packet and must unref it by itself.
 + *
 + * @see av_packet_unref av_packet_ref
 + *
 + * @param head List head.
 + * @param tail List tail.
 + * @param pkt  Pointer to an initialized AVPacket struct
 + * @return  0 on failure and 0 on success.
 + */
 +int ff_packet_list_get(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt);
 +
 +/**
 + * Wipe the list and unref all the packets in it.
 + *
 + * @param head List head.
 + * @param tail List tail.
 + */
 +void ff_packet_list_free(AVPacketList **head, AVPacketList **tail);
 
 Speaking of the list in all these descriptions is confusing, the reader
 does not yet know the signature of the function.  A list of AVPackets
 or similar would be less confusing.
 
 Drop the periods from parameter descriptions that are not sentences, you
 only have periods on some of them anyway.
 
 --- a/libavformat/avformat.h
 +++ b/libavformat/avformat.h
 @@ -1254,12 +1254,6 @@ typedef struct AVFormatContext {
  
 -typedef struct AVPacketList {
 -AVPacket pkt;
 -struct AVPacketList *next;
 -} AVPacketList;
 
 API change - where is the version and deprecation dance?

In the previous set I guess, those patches are extracted to have some
working code so the hwaccel1.3 stuff could be tried.

Once there is agreement on this step towards hwaccel 1.3 either I'll
prepare the set with all the docs or just go a step further and provide
the implementation of the generic api for hwaccel2 and avoid adding too
many init/close functions.

(help in extending the previous generation hwaccel to use them would be
welcome).

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 5/6] lavc: Add private API to manipulate AVPacketList

2014-09-01 Thread Luca Barbato
On 01/09/14 12:47, Diego Biurrun wrote:
 Doesn't this warn because of mixed declarations and statements?

Those are all declarations.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 5/6] lavc: Add private API to manipulate AVPacketList

2014-08-31 Thread James Almer
On 31/08/14 4:24 PM, Luca Barbato wrote:
 ---
  libavcodec/avcodec.h   |  5 +
  libavcodec/avpacket.c  | 56 
 ++
  libavcodec/internal.h  | 36 
  libavformat/avformat.h |  6 --
  4 files changed, 97 insertions(+), 6 deletions(-)
 
 diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
 index 270c6c8..116496f 100644
 --- a/libavcodec/avcodec.h
 +++ b/libavcodec/avcodec.h
 @@ -1019,6 +1019,11 @@ typedef struct AVPacket {
  #define AV_PKT_FLAG_KEY 0x0001 /// The packet contains a keyframe
  #define AV_PKT_FLAG_CORRUPT 0x0002 /// The packet content is corrupted
  
 +typedef struct AVPacketList {
 +AVPacket pkt;
 +struct AVPacketList *next;
 +} AVPacketList;
 +
  enum AVSideDataParamChangeFlags {
  AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT  = 0x0001,
  AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_LAYOUT = 0x0002,
 diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
 index 25eabdb..bea12df 100644
 --- a/libavcodec/avpacket.c
 +++ b/libavcodec/avpacket.c
 @@ -26,6 +26,7 @@
  #include libavutil/internal.h
  #include libavutil/mathematics.h
  #include libavutil/mem.h
 +#include internal.h
  #include avcodec.h
  #if FF_API_DESTRUCT_PACKET
  
 @@ -393,3 +394,58 @@ void av_packet_rescale_ts(AVPacket *pkt, AVRational 
 src_tb, AVRational dst_tb)
  if (pkt-convergence_duration  0)
  pkt-convergence_duration = av_rescale_q(pkt-convergence_duration, 
 src_tb, dst_tb);
  }
 +
 +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt)

avpriv_? (in all three functions).
lavf can make good use of all this. And i think your original patchset did as 
much.

Thanks for resurrecting this for that matter.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 5/6] lavc: Add private API to manipulate AVPacketList

2014-08-31 Thread Luca Barbato
On 31/08/14 22:53, James Almer wrote:
 On 31/08/14 4:24 PM, Luca Barbato wrote:
 ---
  libavcodec/avcodec.h   |  5 +
  libavcodec/avpacket.c  | 56 
 ++
  libavcodec/internal.h  | 36 
  libavformat/avformat.h |  6 --
  4 files changed, 97 insertions(+), 6 deletions(-)

 diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
 index 270c6c8..116496f 100644
 --- a/libavcodec/avcodec.h
 +++ b/libavcodec/avcodec.h
 @@ -1019,6 +1019,11 @@ typedef struct AVPacket {
  #define AV_PKT_FLAG_KEY 0x0001 /// The packet contains a keyframe
  #define AV_PKT_FLAG_CORRUPT 0x0002 /// The packet content is corrupted
  
 +typedef struct AVPacketList {
 +AVPacket pkt;
 +struct AVPacketList *next;
 +} AVPacketList;
 +
  enum AVSideDataParamChangeFlags {
  AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT  = 0x0001,
  AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_LAYOUT = 0x0002,
 diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
 index 25eabdb..bea12df 100644
 --- a/libavcodec/avpacket.c
 +++ b/libavcodec/avpacket.c
 @@ -26,6 +26,7 @@
  #include libavutil/internal.h
  #include libavutil/mathematics.h
  #include libavutil/mem.h
 +#include internal.h
  #include avcodec.h
  #if FF_API_DESTRUCT_PACKET
  
 @@ -393,3 +394,58 @@ void av_packet_rescale_ts(AVPacket *pkt, AVRational 
 src_tb, AVRational dst_tb)
  if (pkt-convergence_duration  0)
  pkt-convergence_duration = av_rescale_q(pkt-convergence_duration, 
 src_tb, dst_tb);
  }
 +
 +int ff_packet_list_put(AVPacketList **head, AVPacketList **tail,
 +   AVPacket *pkt)
 
 avpriv_? (in all three functions).

later av_packet once we decide a better signature =)

 lavf can make good use of all this. And i think your original patchset did as 
 much.

Yes, but the signature is horrible and I couldn't find time to make it
nicer ^^

 Thanks for resurrecting this for that matter.

Sorry for keeping it pending for that long...

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel