Re: [libav-devel] [PATCH] rtsp: add pkt_size option
On Thu, Apr 11, 2019 at 1:41 AM Martin Storsjö wrote:
>
> On Thu, 11 Apr 2019, Tristan Matthews wrote:
>
> > This allows users to specify an upper limit on the size of outgoing packets
> > when publishing via RTSP.
> >
> > ---
> > libavformat/rtsp.c | 5 -
> > libavformat/rtsp.h | 1 +
> > 2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index 8bf9d9e3c..12c4998c6 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -74,7 +74,8 @@
> >
> > #define COMMON_OPTS() \
> > { "reorder_queue_size", "Number of packets to buffer for handling of
> > reordered packets", OFFSET(reordering_queue_size), AV_OPT_TYPE_INT, { .i64
> > = -1 }, -1, INT_MAX, DEC }, \
> > -{ "buffer_size","Underlying protocol send/receive buffer
> > size", OFFSET(buffer_size), AV_OPT_TYPE_INT, {
> > .i64 = -1 }, -1, INT_MAX, DEC|ENC } \
> > +{ "buffer_size","Underlying protocol send/receive buffer
> > size", OFFSET(buffer_size), AV_OPT_TYPE_INT, {
> > .i64 = -1 }, -1, INT_MAX, DEC|ENC }, \
> > +{ "pkt_size", "Underlying protocol send packet size",
> > OFFSET(pkt_size), AV_OPT_TYPE_INT, { .i64 =
> > -1 }, -1, INT_MAX, ENC } \
> >
> >
> > const AVOption ff_rtsp_options[] = {
> > @@ -118,6 +119,8 @@ static AVDictionary *map_to_opts(RTSPState *rt)
> >
> > snprintf(buf, sizeof(buf), "%d", rt->buffer_size);
> > av_dict_set(&opts, "buffer_size", buf, 0);
> > +snprintf(buf, sizeof(buf), "%d", rt->pkt_size);
> > +av_dict_set(&opts, "pkt_size", buf, 0);
> >
> > return opts;
> > }
> > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
> > index 9dfbc5367..c38b90432 100644
> > --- a/libavformat/rtsp.h
> > +++ b/libavformat/rtsp.h
> > @@ -399,6 +399,7 @@ typedef struct RTSPState {
> >
> > char default_lang[4];
> > int buffer_size;
> > +int pkt_size;
> >
> > const URLProtocol **protocols;
> > } RTSPState;
> > --
> > 2.17.1
>
> LGTM
>
> // Martin
This OK to merge?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] rtsp: add pkt_size option
This allows users to specify an upper limit on the size of outgoing packets
when publishing via RTSP.
---
libavformat/rtsp.c | 5 -
libavformat/rtsp.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 8bf9d9e3c..12c4998c6 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -74,7 +74,8 @@
#define COMMON_OPTS() \
{ "reorder_queue_size", "Number of packets to buffer for handling of
reordered packets", OFFSET(reordering_queue_size), AV_OPT_TYPE_INT, { .i64 = -1
}, -1, INT_MAX, DEC }, \
-{ "buffer_size","Underlying protocol send/receive buffer size",
OFFSET(buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },
-1, INT_MAX, DEC|ENC } \
+{ "buffer_size","Underlying protocol send/receive buffer size",
OFFSET(buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },
-1, INT_MAX, DEC|ENC }, \
+{ "pkt_size", "Underlying protocol send packet size",
OFFSET(pkt_size), AV_OPT_TYPE_INT, { .i64 = -1 },
-1, INT_MAX, ENC } \
const AVOption ff_rtsp_options[] = {
@@ -118,6 +119,8 @@ static AVDictionary *map_to_opts(RTSPState *rt)
snprintf(buf, sizeof(buf), "%d", rt->buffer_size);
av_dict_set(&opts, "buffer_size", buf, 0);
+snprintf(buf, sizeof(buf), "%d", rt->pkt_size);
+av_dict_set(&opts, "pkt_size", buf, 0);
return opts;
}
diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
index 9dfbc5367..c38b90432 100644
--- a/libavformat/rtsp.h
+++ b/libavformat/rtsp.h
@@ -399,6 +399,7 @@ typedef struct RTSPState {
char default_lang[4];
int buffer_size;
+int pkt_size;
const URLProtocol **protocols;
} RTSPState;
--
2.17.1
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] rtsp: only break on parse_rtsp_message on error
On Wed, Nov 29, 2017 at 3:14 PM, Martin Storsjö wrote:
> On Wed, 29 Nov 2017, Tristan Matthews wrote:
>
>> Fix suggested by Luca Barbato.
>>
>> This was causing spurious EOFs when using -rtsp_transport udp, as
>> reported in https://bugzilla.libav.org/show_bug.cgi?id=1103
>> ---
>> libavformat/rtsp.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>> index 17a25a310..8bf9d9e3c 100644
>> --- a/libavformat/rtsp.c
>> +++ b/libavformat/rtsp.c
>> @@ -1998,7 +1998,9 @@ static int udp_read_packet(AVFormatContext *s,
>> RTSPStream **prtsp_st,
>> }
>> #if CONFIG_RTSP_DEMUXER
>> if (rt->rtsp_hd && p[0].revents & POLLIN) {
>> -return parse_rtsp_message(s);
>> +if ((ret = parse_rtsp_message(s)) < 0) {
>> +return ret;
>> +}
>> }
>> #endif
>> } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
>> --
>> 2.15.0
>
>
> Looks good, I think. I wonder why nobody has run into this until now though.
>
I found a few similar looking issues when searching for "RTSP EOF".
Most people just reported that they'd "fixed" the issue by forcing
-rtsp_transport tcp.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] rtsp: only break on parse_rtsp_message on error
Fix suggested by Luca Barbato.
This was causing spurious EOFs when using -rtsp_transport udp, as
reported in https://bugzilla.libav.org/show_bug.cgi?id=1103
---
libavformat/rtsp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 17a25a310..8bf9d9e3c 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1998,7 +1998,9 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream
**prtsp_st,
}
#if CONFIG_RTSP_DEMUXER
if (rt->rtsp_hd && p[0].revents & POLLIN) {
-return parse_rtsp_message(s);
+if ((ret = parse_rtsp_message(s)) < 0) {
+return ret;
+}
}
#endif
} else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
--
2.15.0
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] Canopus HQ/HQA decoder
On Tue, Mar 24, 2015 at 4:09 PM, Tristan Matthews wrote: > On Tue, Mar 24, 2015 at 3:03 PM, Derek Buitenhuis > wrote: >> On 3/24/2015 6:53 PM, Luca Barbato wrote: >>> No, it is exactly to keep the structure packed a little as I explained >>> to Vittorio privately. >> >> So, micro-optimizations / ricing? >> >>> Diego (Flameeyes) explained in a number of blogpost why it is useful, >>> sadly pahole is not available for mac. >> >> I can't much think how it is useful in any practical sense... link? >> Google appears to have no knowledge of it. > > https://blog.flameeyes.eu/2008/03/some-words-about-global-variable sorry, https://blog.flameeyes.eu/2008/03/some-words-about-global-variables ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] Canopus HQ/HQA decoder
On Tue, Mar 24, 2015 at 3:03 PM, Derek Buitenhuis wrote: > On 3/24/2015 6:53 PM, Luca Barbato wrote: >> No, it is exactly to keep the structure packed a little as I explained >> to Vittorio privately. > > So, micro-optimizations / ricing? > >> Diego (Flameeyes) explained in a number of blogpost why it is useful, >> sadly pahole is not available for mac. > > I can't much think how it is useful in any practical sense... link? > Google appears to have no knowledge of it. https://blog.flameeyes.eu/2008/03/some-words-about-global-variable https://blog.flameeyes.eu/2009/11/the-importance-of-opaque-types -t ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] Canopus HQ/HQA decoder
On Mon, Mar 23, 2015 at 8:40 AM, Vittorio Giovara
wrote:
> On Sun, Mar 22, 2015 at 7:36 PM, Luca Barbato wrote:
>>> +typedef struct HQContext {
>>> +AVCodecContext *avctx;
>>> +
>>> +VLC hq_ac_vlc;
>>> +VLC hqa_cbp_vlc;
>>> +DECLARE_ALIGNED(16, int16_t, block)[12][64];
>>> +} HQContext;
>>> +
>>> +typedef struct HQProfile {
>>> +int width, height;
>>> +int num_slices;
>>> +const uint8_t *perm_tab;
>>> +int tab_w, tab_h;
>>> +} HQProfile;
>>
>>
>> Pointers first, int after please.
>>
>
> Can you explain this in more details please?
I think he means for packing, see: http://www.catb.org/esr/structure-packing/
Best,
t
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 1/1] srtpproto: fix option flag type
---
libavformat/srtpproto.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavformat/srtpproto.c b/libavformat/srtpproto.c
index f9b94d7..1f818d7 100644
--- a/libavformat/srtpproto.c
+++ b/libavformat/srtpproto.c
@@ -42,8 +42,8 @@ typedef struct SRTPProtoContext {
static const AVOption options[] = {
{ "srtp_out_suite", "", offsetof(SRTPProtoContext, out_suite),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
{ "srtp_out_params", "", offsetof(SRTPProtoContext, out_params),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
-{ "srtp_in_suite", "", offsetof(SRTPProtoContext, in_suite),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
-{ "srtp_in_params", "", offsetof(SRTPProtoContext, in_params),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
+{ "srtp_in_suite", "", offsetof(SRTPProtoContext, in_suite),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
+{ "srtp_in_params", "", offsetof(SRTPProtoContext, in_params),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D },
{ NULL }
};
--
1.9.3
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH 1/1] v4l2: Use av_strerror
---
libavdevice/v4l2.c | 49 ++---
1 file changed, 34 insertions(+), 15 deletions(-)
diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
index e210dc4..3439325 100644
--- a/libavdevice/v4l2.c
+++ b/libavdevice/v4l2.c
@@ -119,6 +119,7 @@ static int device_open(AVFormatContext *ctx)
int fd;
int res, err;
int flags = O_RDWR;
+char errbuf[128];
if (ctx->flags & AVFMT_FLAG_NONBLOCK) {
flags |= O_NONBLOCK;
@@ -126,19 +127,21 @@ static int device_open(AVFormatContext *ctx)
fd = avpriv_open(ctx->filename, flags);
if (fd < 0) {
-err = errno;
+err = AVERROR(errno);
+av_strerror(err, errbuf, sizeof(errbuf));
av_log(ctx, AV_LOG_ERROR, "Cannot open video device %s : %s\n",
- ctx->filename, strerror(err));
+ ctx->filename, errbuf);
-return AVERROR(err);
+return err;
}
res = ioctl(fd, VIDIOC_QUERYCAP, &cap);
if (res < 0) {
err = errno;
+av_strerror(AVERROR(err), errbuf, sizeof(errbuf));
av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QUERYCAP): %s\n",
- strerror(err));
+ errbuf);
goto fail;
}
@@ -401,7 +404,9 @@ static int mmap_init(AVFormatContext *ctx)
s->fd, buf.m.offset);
if (s->buf_start[i] == MAP_FAILED) {
-av_log(ctx, AV_LOG_ERROR, "mmap: %s\n", strerror(errno));
+char errbuf[128];
+av_strerror(AVERROR(errno), errbuf, sizeof(errbuf));
+av_log(ctx, AV_LOG_ERROR, "mmap: %s\n", errbuf);
return AVERROR(errno);
}
@@ -423,6 +428,7 @@ static void mmap_release_buffer(void *opaque, uint8_t *data)
int res, fd;
struct buff_data *buf_descriptor = opaque;
struct video_data *s = buf_descriptor->s;
+char errbuf[128];
buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
buf.memory = V4L2_MEMORY_MMAP;
@@ -431,9 +437,11 @@ static void mmap_release_buffer(void *opaque, uint8_t
*data)
av_free(buf_descriptor);
res = ioctl(fd, VIDIOC_QBUF, &buf);
-if (res < 0)
+if (res < 0) {
+av_strerror(AVERROR(errno), errbuf, sizeof(errbuf));
av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n",
- strerror(errno));
+ errbuf);
+}
avpriv_atomic_int_add_and_fetch(&s->buffers_queued, 1);
}
@@ -457,13 +465,15 @@ static int mmap_read_frame(AVFormatContext *ctx, AVPacket
*pkt)
/* FIXME: Some special treatment might be needed in case of loss of
signal... */
while ((res = ioctl(s->fd, VIDIOC_DQBUF, &buf)) < 0 && (errno == EINTR));
if (res < 0) {
+char errbuf[128];
if (errno == EAGAIN) {
pkt->size = 0;
return AVERROR(EAGAIN);
}
+av_strerror(AVERROR(errno), errbuf, sizeof(errbuf));
av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_DQBUF): %s\n",
- strerror(errno));
+ errbuf);
return AVERROR(errno);
}
@@ -542,7 +552,8 @@ static int mmap_start(AVFormatContext *ctx)
{
struct video_data *s = ctx->priv_data;
enum v4l2_buf_type type;
-int i, res;
+int i, res, err;
+char errbuf[128];
for (i = 0; i < s->buffers; i++) {
struct v4l2_buffer buf = {
@@ -553,10 +564,12 @@ static int mmap_start(AVFormatContext *ctx)
res = ioctl(s->fd, VIDIOC_QBUF, &buf);
if (res < 0) {
+err = AVERROR(errno);
+av_strerror(err, errbuf, sizeof(errbuf));
av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n",
- strerror(errno));
+ errbuf);
-return AVERROR(errno);
+return err;
}
}
s->buffers_queued = s->buffers;
@@ -564,10 +577,12 @@ static int mmap_start(AVFormatContext *ctx)
type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
res = ioctl(s->fd, VIDIOC_STREAMON, &type);
if (res < 0) {
+err = AVERROR(errno);
+av_strerror(err, errbuf, sizeof(errbuf));
av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_STREAMON): %s\n",
- strerror(errno));
+ errbuf);
-return AVERROR(errno);
+return err;
}
return 0;
@@ -677,8 +692,10 @@ static int v4l2_set_parameters(AVFormatContext *s1)
}
} else {
if (ioctl(s->fd, VIDIOC_G_PARM, &streamparm) != 0) {
+char errbuf[128];
+av_strerror(AVERROR(errno), errbuf, sizeof(errbuf));
av_log(s1, AV_LOG_ERROR, "ioctl(VIDIOC_G_PARM): %s\n",
- strerror(errno));
+ errbuf);
return AVERROR(errno);
}
}
@@ -779,8 +796,10 @@ static int v4l2_read_header(AVFormatContext *s1)
"Querying the device for the current frame size\n");
fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
if (ioctl(s->fd, VIDIOC_G_FMT, &fmt) < 0) {
+
[libav-devel] [PATCH] flacdec: remove unused headers
--- libavcodec/flacdec.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c index 9ca55cc..a475309 100644 --- a/libavcodec/flacdec.c +++ b/libavcodec/flacdec.c @@ -33,8 +33,6 @@ #include -#include "libavutil/channel_layout.h" -#include "libavutil/crc.h" #include "avcodec.h" #include "internal.h" #include "get_bits.h" -- 1.9.3 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] tiffenc: fix packet size calculation
---
libavcodec/tiffenc.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
index ccfb07c..c94c7f4 100644
--- a/libavcodec/tiffenc.c
+++ b/libavcodec/tiffenc.c
@@ -218,6 +218,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket
*pkt,
int is_yuv = 0;
uint8_t *yuv_line = NULL;
int shift_h, shift_v;
+int packet_size;
const AVPixFmtDescriptor *pfd;
s->avctx = avctx;
@@ -288,10 +289,11 @@ static int encode_frame(AVCodecContext *avctx, AVPacket
*pkt,
strips = (s->height - 1) / s->rps + 1;
+packet_size = avctx->height * (avctx->width * s->bpp + 7 >> 3) * 2 +
+ avctx->height * 4 + FF_MIN_BUFFER_SIZE;
+
if (!pkt->data &&
-(ret = av_new_packet(pkt,
- avctx->width * avctx->height * s->bpp * 2 +
- avctx->height * 4 + FF_MIN_BUFFER_SIZE)) < 0) {
+(ret = av_new_packet(pkt, packet_size)) < 0) {
av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");
return ret;
}
--
1.9.1
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] tiffenc: fix packet size calculation
---
libavcodec/tiffenc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
index ccfb07c..c40f340 100644
--- a/libavcodec/tiffenc.c
+++ b/libavcodec/tiffenc.c
@@ -290,7 +290,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket
*pkt,
if (!pkt->data &&
(ret = av_new_packet(pkt,
- avctx->width * avctx->height * s->bpp * 2 +
+ (avctx->width * avctx->height * s->bpp * 2) >> 3 +
avctx->height * 4 + FF_MIN_BUFFER_SIZE)) < 0) {
av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");
return ret;
--
1.9.1
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] configure: fix enable-libopus help string
--- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 1e9a54c..9b73883 100755 --- a/configure +++ b/configure @@ -192,7 +192,7 @@ External library support: --enable-libopencore-amrwb enable AMR-WB decoding via libopencore-amrwb [no] --enable-libopencv enable video filtering via libopencv [no] --enable-libopenjpeg enable JPEG 2000 de/encoding via OpenJPEG [no] - --enable-libopus enable Opus decoding via libopus [no] + --enable-libopus enable Opus de/encoding via libopus [no] --enable-libpulseenable Pulseaudio input via libpulse [no] --enable-librtmp enable RTMP[E] support via librtmp [no] --enable-libschroedinger enable Dirac de/encoding via libschroedinger [no] -- 1.9.0 ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] lavu: use unsigned for sample and channel counts
---
libavutil/samplefmt.c | 9 +
libavutil/samplefmt.h | 3 ++-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/libavutil/samplefmt.c b/libavutil/samplefmt.c
index 389f726..8361bcf 100644
--- a/libavutil/samplefmt.c
+++ b/libavutil/samplefmt.c
@@ -105,7 +105,8 @@ int av_sample_fmt_is_planar(enum AVSampleFormat sample_fmt)
return sample_fmt_info[sample_fmt].planar;
}
-int av_samples_get_buffer_size(int *linesize, int nb_channels, int nb_samples,
+int av_samples_get_buffer_size(int *linesize, unsigned nb_channels,
+ unsigned nb_samples,
enum AVSampleFormat sample_fmt, int align)
{
int line_size;
@@ -113,7 +114,7 @@ int av_samples_get_buffer_size(int *linesize, int
nb_channels, int nb_samples,
int planar = av_sample_fmt_is_planar(sample_fmt);
/* validate parameter ranges */
-if (!sample_size || nb_samples <= 0 || nb_channels <= 0)
+if (!sample_size || nb_samples == 0 || nb_channels == 0)
return AVERROR(EINVAL);
/* auto-select alignment if not specified */
@@ -123,8 +124,8 @@ int av_samples_get_buffer_size(int *linesize, int
nb_channels, int nb_samples,
}
/* check for integer overflow */
-if (nb_channels > INT_MAX / align ||
-(int64_t)nb_channels * nb_samples > (INT_MAX - (align * nb_channels))
/ sample_size)
+if (nb_channels > UINT_MAX / align ||
+(uint64_t)nb_channels * nb_samples > (UINT_MAX - (align *
nb_channels)) / sample_size)
return AVERROR(EINVAL);
line_size = planar ? FFALIGN(nb_samples * sample_size,
align) :
diff --git a/libavutil/samplefmt.h b/libavutil/samplefmt.h
index 33cbded..10ae1df 100644
--- a/libavutil/samplefmt.h
+++ b/libavutil/samplefmt.h
@@ -138,7 +138,8 @@ int av_sample_fmt_is_planar(enum AVSampleFormat sample_fmt);
* @param align buffer size alignment (0 = default, 1 = no alignment)
* @return required buffer size, or negative error code on failure
*/
-int av_samples_get_buffer_size(int *linesize, int nb_channels, int nb_samples,
+int av_samples_get_buffer_size(int *linesize, unsigned nb_channels,
+ unsigned nb_samples,
enum AVSampleFormat sample_fmt, int align);
/**
--
1.8.5.3
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] samplefmt: avoid integer overflow in av_samples_get_buffer_size()
On Thu, Jan 30, 2014 at 2:20 PM, Luca Barbato wrote:
>
> On 30/01/14 20:11, Justin Ruggles wrote:
> > CC:libav-sta...@libav.org
> > ---
> > libavutil/samplefmt.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/libavutil/samplefmt.c b/libavutil/samplefmt.c
> > index 389f726..bff6004 100644
> > --- a/libavutil/samplefmt.c
> > +++ b/libavutil/samplefmt.c
> > @@ -118,6 +118,8 @@ int av_samples_get_buffer_size(int *linesize, int
> > nb_channels, int nb_samples,
> >
> > /* auto-select alignment if not specified */
> > if (!align) {
> > +if (nb_samples > INT_MAX - 31)
> > +return AVERROR(EINVAL);
> > align = 1;
> > nb_samples = FFALIGN(nb_samples, 32);
> > }
> >
>
> Sounds safe.
In the long term, would it make sense for the nb_channels and
nb_samples arguments to be changed to unsigned (i.e. in new API)?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH v2] jack: use condition variable instead of semaphore
On Fri, Dec 6, 2013 at 9:59 AM, Rémi Denis-Courmont wrote: > Le vendredi 6 décembre 2013, 09:28:00 Tristan Matthews a écrit : >> > A condition variable MUST have some kind of state protected by its mutex. >> > Without such state, there is NO meaning to the condition variable. If you >> > do lock then wait then unlock with nothing in between them, then the use >> > of a condition variable is wrong. Period. >> >> Then should it be (in the reader thread): >> 1) Lock the mutex >> 2) Check the read space >> 3) If empty, wait on the condition variable >> 4) Unlock the mutex >> >> Currently 1) and 2) are inverted. > > If the read space is protected by the mutex, yes. Otherwise, you should not be > using a condition variable. It's protected by atomic operations, so I'm guessing the answer will again be "you should not be using a condition variable." To recap for others, we need a wakeup mechanism to notify the reader thread that packets are ready if it's waiting on an empty queue, semaphores are the best solution, but they're broken on OS X. Bug-Id: 312 called for condition variables, but if they're not appropriate I'm open to other suggestions. Best, Tristan -- Tristan Matthews web: http://tristanswork.blogspot.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH v2] jack: use condition variable instead of semaphore
On Fri, Dec 6, 2013 at 8:16 AM, Rémi Denis-Courmont wrote: > Le lundi 2 décembre 2013, 16:38:55 Tristan Matthews a écrit : >> On Sun, Dec 1, 2013 at 4:21 PM, Rémi Denis-Courmont wrote: >> > Le dimanche 1 décembre 2013, 16:15:22 Tristan Matthews a écrit : >> >> jack_ringbuffers are used instead of AVFifoBuffers as they are safe for >> >> lock-free use (in single-writer, single-reader scenarios). >> > >> > The use of condition variable without locked predicate/state makes as >> > little sense (i.e. no) as in previous iterations IMO. >> >> In this version, the process callback is locking the mutex before >> signaling (with try_lock), is that not adequate? > > This is irrelevant. > > A condition variable MUST have some kind of state protected by its mutex. > Without such state, there is NO meaning to the condition variable. If you do > lock then wait then unlock with nothing in between them, then the use of a > condition variable is wrong. Period. Then should it be (in the reader thread): 1) Lock the mutex 2) Check the read space 3) If empty, wait on the condition variable 4) Unlock the mutex Currently 1) and 2) are inverted. Best, Tristan -- Tristan Matthews web: http://tristanswork.blogspot.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH v2] jack: use condition variable instead of semaphore
On Sun, Dec 1, 2013 at 4:21 PM, Rémi Denis-Courmont wrote: > Le dimanche 1 décembre 2013, 16:15:22 Tristan Matthews a écrit : >> jack_ringbuffers are used instead of AVFifoBuffers as they are safe for >> lock-free use (in single-writer, single-reader scenarios). > > The use of condition variable without locked predicate/state makes as little > sense (i.e. no) as in previous iterations IMO. In this version, the process callback is locking the mutex before signaling (with try_lock), is that not adequate? In any case, even if the reader misses a signal because it is not yet waiting on the condition variable, it will catch the next one. Moreover, it will only start waiting if it finds the ringbuffer to be empty. I agree that using a semaphore is better (since you avoid the above missed signal problem), but that's the whole issue with https://bugzilla.libav.org/show_bug.cgi?id=312 Could someone clarify what exactly is wrong with Mac OS X semaphores? I've only found this so far: http://dev.alopix.net/2012/10/os-x-dont-trust-posix-certificate.html in which case, could this be more simply resolved by using a named semaphore? Best, Tristan -- Tristan Matthews web: http://tristanswork.blogspot.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH v2] jack: use condition variable instead of semaphore
jack_ringbuffers are used instead of AVFifoBuffers as they are safe for
lock-free use (in single-writer, single-reader scenarios).
Bug-Id: 312
---
libavdevice/jack_audio.c | 118 +--
1 file changed, 74 insertions(+), 44 deletions(-)
diff --git a/libavdevice/jack_audio.c b/libavdevice/jack_audio.c
index c261514..53f75a1 100644
--- a/libavdevice/jack_audio.c
+++ b/libavdevice/jack_audio.c
@@ -21,11 +21,11 @@
*/
#include "config.h"
-#include
+#include
#include
+#include
#include "libavutil/log.h"
-#include "libavutil/fifo.h"
#include "libavutil/opt.h"
#include "libavutil/time.h"
#include "libavcodec/avcodec.h"
@@ -34,22 +34,23 @@
#include "timefilter.h"
/**
- * Size of the internal FIFO buffers as a number of audio packets
+ * Size of the internal ringbuffers as a number of audio packets
*/
-#define FIFO_PACKETS_NUM 16
+#define RING_PACKETS_NUM 16
typedef struct {
AVClass*class;
jack_client_t * client;
int activated;
-sem_t packet_count;
+pthread_mutex_t read_lock;
+pthread_cond_t packet_ready;
jack_nframes_t sample_rate;
jack_nframes_t buffer_size;
jack_port_t ** ports;
int nports;
TimeFilter *timefilter;
-AVFifoBuffer * new_pkts;
-AVFifoBuffer * filled_pkts;
+jack_ringbuffer_t *new_pkts;
+jack_ringbuffer_t *filled_pkts;
int pkt_xrun;
int jack_xrun;
} JackData;
@@ -79,13 +80,14 @@ static int process_callback(jack_nframes_t nframes, void
*arg)
self->buffer_size);
/* Check if an empty packet is available, and if there's enough space to
send it back once filled */
-if ((av_fifo_size(self->new_pkts) < sizeof(pkt)) ||
(av_fifo_space(self->filled_pkts) < sizeof(pkt))) {
+if ((jack_ringbuffer_read_space(self->new_pkts) < sizeof(pkt)) ||
+(jack_ringbuffer_write_space(self->filled_pkts) < sizeof(pkt))) {
self->pkt_xrun = 1;
return 0;
}
/* Retrieve empty (but allocated) packet */
-av_fifo_generic_read(self->new_pkts, &pkt, sizeof(pkt), NULL);
+jack_ringbuffer_read(self->new_pkts, (char *) &pkt, sizeof(pkt));
pkt_data = (float *) pkt.data;
latency = 0;
@@ -107,9 +109,13 @@ static int process_callback(jack_nframes_t nframes, void
*arg)
/* Timestamp the packet with the cycle start time minus the average
latency */
pkt.pts = (cycle_time - (double) latency / (self->nports *
self->sample_rate)) * 100.0;
-/* Send the now filled packet back, and increase packet counter */
-av_fifo_generic_write(self->filled_pkts, &pkt, sizeof(pkt), NULL);
-sem_post(&self->packet_count);
+/* Send the now filled packet back, and signal that it's ready */
+jack_ringbuffer_write(self->filled_pkts, (char *) &pkt, sizeof(pkt));
+
+if (pthread_mutex_trylock(&self->read_lock) == 0) {
+pthread_cond_signal(&self->packet_ready);
+pthread_mutex_unlock(&self->read_lock);
+}
return 0;
}
@@ -136,12 +142,12 @@ static int supply_new_packets(JackData *self,
AVFormatContext *context)
/* Supply the process callback with new empty packets, by filling the new
* packets FIFO buffer with as many packets as possible. process_callback()
* can't do this by itself, because it can't allocate memory in realtime.
*/
-while (av_fifo_space(self->new_pkts) >= sizeof(pkt)) {
+while (jack_ringbuffer_write_space(self->new_pkts) >= sizeof(pkt)) {
if ((test = av_new_packet(&pkt, pkt_size)) < 0) {
av_log(context, AV_LOG_ERROR, "Could not create packet of size
%d\n", pkt_size);
return test;
}
-av_fifo_generic_write(self->new_pkts, &pkt, sizeof(pkt), NULL);
+jack_ringbuffer_write(self->new_pkts, (char *) &pkt, sizeof(pkt));
}
return 0;
}
@@ -160,7 +166,8 @@ static int start_jack(AVFormatContext *context)
return AVERROR(EIO);
}
-sem_init(&self->packet_count, 0, 0);
+pthread_mutex_init(&self->read_lock, NULL);
+pthread_cond_init(&self->packet_ready, NULL);
self->sample_rate = jack_get_sample_rate(self->client);
self->ports = av_malloc(self->nports * sizeof(*self->ports));
@@ -181,11 +188,6 @@ static int start_jack(AVFormatContext *context)
}
}
-/* Register JACK callbacks */
-jack_set_process_callback(self->client, process_callback, self);
-jack_on_shutdown(self->client, shutdown_callback, self);
-jack_set_xrun_callback(self->client, xrun_callback, self);
-
/* Create time filter */
period= (double) self->buffer_size / self->sample_rate;
o = 2 * M_PI * 1.5 * period; /// bandwidth: 1.5Hz
@@ -195,27 +197,45 @@ static int start_jack(AVFormatContext *context)
return AVERROR(ENOMEM);
}
-/* Create FIFO buffers */
-self->filled_pkts = av_fifo_allo
Re: [libav-devel] [PATCH] jack: use pthread_cond_t instead of semaphore
On Mon, Nov 25, 2013 at 5:22 AM, Rémi Denis-Courmont wrote:
> On Sun, 24 Nov 2013 19:15:09 -0500, Tristan Matthews
> wrote:
>> Fixes https://bugzilla.libav.org/show_bug.cgi?id=312
>> ---
>> libavdevice/jack_audio.c | 55
>> +++-
>> 1 file changed, 40 insertions(+), 15 deletions(-)
>>
>> diff --git a/libavdevice/jack_audio.c b/libavdevice/jack_audio.c
>> index c261514..ef6e0fb 100644
>> --- a/libavdevice/jack_audio.c
>> +++ b/libavdevice/jack_audio.c
>> @@ -21,7 +21,7 @@
>> */
>>
>> #include "config.h"
>> -#include
>> +#include
>> #include
>>
>> #include "libavutil/log.h"
>> @@ -42,7 +42,8 @@ typedef struct {
>> AVClass*class;
>> jack_client_t * client;
>> int activated;
>> -sem_t packet_count;
>> +pthread_mutex_t pkt_mutex;
>> +pthread_cond_t pkt_cond;
>> jack_nframes_t sample_rate;
>> jack_nframes_t buffer_size;
>> jack_port_t ** ports;
>> @@ -107,9 +108,9 @@ static int process_callback(jack_nframes_t nframes,
>> void *arg)
>> /* Timestamp the packet with the cycle start time minus the average
>> latency */
>> pkt.pts = (cycle_time - (double) latency / (self->nports *
>> self->sample_rate)) * 100.0;
>>
>> -/* Send the now filled packet back, and increase packet counter */
>> +/* Send the now filled packet back, and signal that it's ready */
>> av_fifo_generic_write(self->filled_pkts, &pkt, sizeof(pkt), NULL);
>> -sem_post(&self->packet_count);
>> +pthread_cond_signal(&self->pkt_cond);
>
> Signalling a condition variable without any mutex operation in the
> vicinity raises both of my eyebrows.
Yes, from the documentation you _can_ signal without locking but you
won't have "predictable
scheduling behaviour." I'm not sure how serious that is in a
single-writer, single-reader situation
like this one.
I can't lock the mutex in the process callback because it's not
real-time safe, in theory I could
try_lock but then how should failure to lock be handled?
>
>>
>> return 0;
>> }
>> @@ -160,7 +161,8 @@ static int start_jack(AVFormatContext *context)
>> return AVERROR(EIO);
>> }
>>
>> -sem_init(&self->packet_count, 0, 0);
>> +pthread_mutex_init(&self->pkt_mutex, NULL);
>> +pthread_cond_init(&self->pkt_cond, NULL);
>>
>> self->sample_rate = jack_get_sample_rate(self->client);
>> self->ports = av_malloc(self->nports * sizeof(*self->ports));
>> @@ -225,7 +227,8 @@ static void stop_jack(JackData *self)
>> jack_deactivate(self->client);
>> jack_client_close(self->client);
>> }
>> -sem_destroy(&self->packet_count);
>> +pthread_cond_destroy(&self->pkt_cond);
>> +pthread_mutex_destroy(&self->pkt_mutex);
>> free_pkt_fifo(self->new_pkts);
>> free_pkt_fifo(self->filled_pkts);
>> av_freep(&self->ports);
>> @@ -284,18 +287,40 @@ static int audio_read_packet(AVFormatContext
>> *context, AVPacket *pkt)
>>
>> /* Wait for a packet coming back from process_callback(), if one
>> isn't available yet */
>> timeout.tv_sec = av_gettime() / 100 + 2;
>> -if (sem_timedwait(&self->packet_count, &timeout)) {
>> -if (errno == ETIMEDOUT) {
>> -av_log(context, AV_LOG_ERROR,
>> - "Input error: timed out when waiting for JACK
> process
>> callback output\n");
>> -} else {
>> -av_log(context, AV_LOG_ERROR, "Error while waiting for
> audio
>> packet: %s\n",
>> +
>> +/* Only wait if there's nothing to read yet */
>> +if (!av_fifo_size(self->filled_pkts)) {
>> +
>> +if (pthread_mutex_lock(&self->pkt_mutex)) {
>
> Cleanly handling pthread_mutex_lock error is _useless_. If that function
> does fail, the code is _already_ in undefined land(*). So, IMHO, aborting
> the process is the only reasonable thing to do.
>
> (*) There is one exception: if the mutex is error-checking *and* already
> locked by the calling thread, then EDEADLK will be returned.
>
>> +av_log(context, AV_LOG_ERROR, "Error while trying to lock
>> mutex: %s\n",
>> strerror(errno));
>> +return AVERRO
Re: [libav-devel] [PATCH] jack: use pthread_cond_t instead of semaphore
On Mon, Nov 25, 2013 at 4:59 AM, Luca Barbato wrote: > On 25/11/13 01:15, Tristan Matthews wrote: >> Fixes https://bugzilla.libav.org/show_bug.cgi?id=312 > > use > > Bug-Id: 312 > > Instead =) >> --- >> libavdevice/jack_audio.c | 55 >> +++- >> 1 file changed, 40 insertions(+), 15 deletions(-) >> > > Did you test it? I don't have jack around but: yes (linux-only though). > > - You are accessing the fifo outside the mutex. I was wondering about this. The original libavdevice/jack_audio.c implementation uses libav fifos which do not have memory barriers (though there are comments mentioning them!) It might be better to use the jackringbuffer structure (that jack provides), which is a lock-free ring buffer that does use atomic ops, but I didn't want to change too many things at once. What do you think? > > - Always save errno and forward it. > > The rest looks promising. Thanks for the fast review. > > lu > > > > ___ > libav-devel mailing list > libav-devel@libav.org > https://lists.libav.org/mailman/listinfo/libav-devel -- Tristan Matthews web: http://tristanswork.blogspot.com ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [PATCH] jack: use pthread_cond_t instead of semaphore
Fixes https://bugzilla.libav.org/show_bug.cgi?id=312
---
libavdevice/jack_audio.c | 55 +++-
1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/libavdevice/jack_audio.c b/libavdevice/jack_audio.c
index c261514..ef6e0fb 100644
--- a/libavdevice/jack_audio.c
+++ b/libavdevice/jack_audio.c
@@ -21,7 +21,7 @@
*/
#include "config.h"
-#include
+#include
#include
#include "libavutil/log.h"
@@ -42,7 +42,8 @@ typedef struct {
AVClass*class;
jack_client_t * client;
int activated;
-sem_t packet_count;
+pthread_mutex_t pkt_mutex;
+pthread_cond_t pkt_cond;
jack_nframes_t sample_rate;
jack_nframes_t buffer_size;
jack_port_t ** ports;
@@ -107,9 +108,9 @@ static int process_callback(jack_nframes_t nframes, void
*arg)
/* Timestamp the packet with the cycle start time minus the average
latency */
pkt.pts = (cycle_time - (double) latency / (self->nports *
self->sample_rate)) * 100.0;
-/* Send the now filled packet back, and increase packet counter */
+/* Send the now filled packet back, and signal that it's ready */
av_fifo_generic_write(self->filled_pkts, &pkt, sizeof(pkt), NULL);
-sem_post(&self->packet_count);
+pthread_cond_signal(&self->pkt_cond);
return 0;
}
@@ -160,7 +161,8 @@ static int start_jack(AVFormatContext *context)
return AVERROR(EIO);
}
-sem_init(&self->packet_count, 0, 0);
+pthread_mutex_init(&self->pkt_mutex, NULL);
+pthread_cond_init(&self->pkt_cond, NULL);
self->sample_rate = jack_get_sample_rate(self->client);
self->ports = av_malloc(self->nports * sizeof(*self->ports));
@@ -225,7 +227,8 @@ static void stop_jack(JackData *self)
jack_deactivate(self->client);
jack_client_close(self->client);
}
-sem_destroy(&self->packet_count);
+pthread_cond_destroy(&self->pkt_cond);
+pthread_mutex_destroy(&self->pkt_mutex);
free_pkt_fifo(self->new_pkts);
free_pkt_fifo(self->filled_pkts);
av_freep(&self->ports);
@@ -284,18 +287,40 @@ static int audio_read_packet(AVFormatContext *context,
AVPacket *pkt)
/* Wait for a packet coming back from process_callback(), if one isn't
available yet */
timeout.tv_sec = av_gettime() / 100 + 2;
-if (sem_timedwait(&self->packet_count, &timeout)) {
-if (errno == ETIMEDOUT) {
-av_log(context, AV_LOG_ERROR,
- "Input error: timed out when waiting for JACK process
callback output\n");
-} else {
-av_log(context, AV_LOG_ERROR, "Error while waiting for audio
packet: %s\n",
+
+/* Only wait if there's nothing to read yet */
+if (!av_fifo_size(self->filled_pkts)) {
+
+if (pthread_mutex_lock(&self->pkt_mutex)) {
+av_log(context, AV_LOG_ERROR, "Error while trying to lock mutex:
%s\n",
strerror(errno));
+return AVERROR(EIO);
}
-if (!self->client)
-av_log(context, AV_LOG_ERROR, "Input error: JACK server is
gone\n");
-return AVERROR(EIO);
+if (pthread_cond_timedwait(&self->pkt_cond, &self->pkt_mutex,
&timeout)) {
+if (errno == ETIMEDOUT) {
+av_log(context, AV_LOG_ERROR,
+ "Input error: timed out when waiting for JACK process
callback output\n");
+} else {
+av_log(context, AV_LOG_ERROR, "Error while waiting for audio
packet: %s\n",
+ strerror(errno));
+}
+if (!self->client)
+av_log(context, AV_LOG_ERROR, "Input error: JACK server is
gone\n");
+
+if (pthread_mutex_unlock(&self->pkt_mutex)) {
+av_log(context, AV_LOG_ERROR, "Error while trying to unlock
mutex: %s\n",
+ strerror(errno));
+}
+
+return AVERROR(EIO);
+}
+
+if (pthread_mutex_unlock(&self->pkt_mutex)) {
+av_log(context, AV_LOG_ERROR, "Error while trying to unlock mutex:
%s\n",
+ strerror(errno));
+return AVERROR(EIO);
+}
}
if (self->pkt_xrun) {
--
1.8.1.2
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
