On 1 July 2010 00:47, Martin Storsjö <[email protected]> wrote:
> On Wed, 30 Jun 2010, Josh Allmann wrote:
>
>> +/**
>> + * @file
>> + * @brief RTP support for the SV3V (SVQ3) payload (RE'ed)
>
> I think this could afford a few more words for clarity, e.g. "no RFC,
> reverse engineered".
>
Added a link to the multimediawiki page per Luca's request.
>> +
>> + if (config_packet) {
>> +
>> + if (sv->timestamp)
>> + av_free(st->codec->extradata);
>
> Umm, why the check? If some extradata already exists, it should be freed
> here, if not, av_free() does nothing, so the check can just as well be
> left out. Also, av_freep() would be even better, since if we'd happen to
> be in the len < 2 case, we'd leave the freed pointer dangling.
>
Removed the check and changed to av_freep.
>> +
>> +static PayloadContext *
>> +sv3v_extradata_new(void)
>> +{
>
> Why the weird line breaking here, instead of just keeping it all on one
> line? Also, if writing it on one line, the * belongs next to the function
> name.
>
Fixed.
>> +RTPDynamicProtocolHandler ff_svq3_dynamic_handler = {
>> + "X-SV3V-ES",
>> + CODEC_TYPE_VIDEO,
>> + CODEC_ID_NONE,
>
> A comment explaining why the codec id is none would very much be needed here.
>
>> + NULL, // parse sdp line
>> + sv3v_extradata_new,
>> + sv3v_extradata_free,
>> + sv3v_parse_packet,
>> +};
>
Fixed.
> The file is named _svq3, the dynamic handler struct is named ff_svq3_, but
> the internal functions are named sv3v, this feels a little inconsistent.
> I'd prefer one single naming used throughout the file, unless Ronald has
> a good explanation of why this is better.
>
>
Fixed.
>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>> index 8684903..8225639 100644
>> --- a/libavformat/aviobuf.c
>> +++ b/libavformat/aviobuf.c
>> @@ -893,7 +893,10 @@ int url_open_dyn_packet_buf(ByteIOContext **s, int
>> max_packet_size)
>> int url_close_dyn_buf(ByteIOContext *s, uint8_t **pbuffer)
>> {
>> DynBuffer *d = s->opaque;
>> - int size;
>> + int size, i;
>> +
>> + for (i = 0; i < FF_INPUT_BUFFER_PADDING_SIZE; i++)
>> + put_byte(s, 0);
>>
>
> This is not right. If we implicitly pad the buffer, the size returned for
> the buffer should be of the data that the caller had fed in, not including
> the padding as it does now.
>
D'oh, you mentioned that in an earlier email. Fixed.
Also changed to the style preferred by Ronald.
> Also, for buffers opened with url_open_dyn_packet_buf, this is not ok,
> only for those opened with url_open_dyn_buf. When using
> url_open_dyn_packet_buf, the data buffer has framing for the individual
> packets, which would expose the padding data as part of one of the
> packets, which is not what we want, and which would break many things.
>
Fixed.
> Also, documentation should be added to url_open_dyn_buf saying that
> padding is added and that the caller can rely on it. If not, the
> depacketizer shouldn't rely on it being done either.
>
Done.
> Except for that, Michael, are you ok with adding implicit padding at the
> end for all buffers opened with url_open_dyn_buf?
>
From 24f282ef9b71ea2a6bd224a6d927ddf98b119050 Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Thu, 1 Jul 2010 08:58:56 -0700
Subject: [PATCH 1/2] Add RTP depacketization of SVQ3.
---
Changelog | 1 +
libavformat/Makefile | 1 +
libavformat/rtpdec.c | 2 +
libavformat/rtpdec_svq3.c | 136 +++++++++++++++++++++++++++++++++++++++++++++
libavformat/rtpdec_svq3.h | 33 +++++++++++
5 files changed, 173 insertions(+), 0 deletions(-)
create mode 100644 libavformat/rtpdec_svq3.c
create mode 100644 libavformat/rtpdec_svq3.h
diff --git a/Changelog b/Changelog
index be3b074..49990be 100644
--- a/Changelog
+++ b/Changelog
@@ -16,6 +16,7 @@ version <next>:
- DTS-ES extension (XCh) decoding support
- native VP8 decoder
- RTSP tunneling over HTTP
+- RTP depacketization of SVQ3
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 2977673..5291fe3 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -229,6 +229,7 @@ OBJS-$(CONFIG_SDP_DEMUXER) += rtsp.o \
rtpdec_h263.o \
rtpdec_h264.o \
rtpdec_mpeg4.o \
+ rtpdec_svq3.o \
rtpdec_xiph.o
OBJS-$(CONFIG_SEGAFILM_DEMUXER) += segafilm.o
OBJS-$(CONFIG_SHORTEN_DEMUXER) += raw.o id3v2.o
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 38a4c5b..ca06bff 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -35,6 +35,7 @@
#include "rtpdec_h263.h"
#include "rtpdec_h264.h"
#include "rtpdec_mpeg4.h"
+#include "rtpdec_svq3.h"
#include "rtpdec_xiph.h"
//#define DEBUG
@@ -68,6 +69,7 @@ void av_register_rtp_dynamic_payload_handlers(void)
ff_register_dynamic_payload_handler(&ff_h264_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_vorbis_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_theora_dynamic_handler);
+ ff_register_dynamic_payload_handler(&ff_svq3_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_ms_rtp_asf_pfv_handler);
ff_register_dynamic_payload_handler(&ff_ms_rtp_asf_pfa_handler);
diff --git a/libavformat/rtpdec_svq3.c b/libavformat/rtpdec_svq3.c
new file mode 100644
index 0000000..c15bd5a
--- /dev/null
+++ b/libavformat/rtpdec_svq3.c
@@ -0,0 +1,136 @@
+/*
+ * Sorenson-3 (SVQ3/SV3V) payload for RTP
+ * Copyright (c) 2010 Ronald S. Bultje
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * @brief RTP support for the SV3V (SVQ3) payload (http://wiki.multimedia.cx/index.php?title=Sorenson_Video_3#Packetization)
+ * @author Ronald S. Bultje <[email protected]>
+ */
+
+#include <string.h>
+#include <libavcodec/get_bits.h>
+#include "rtp.h"
+#include "rtpdec.h"
+#include "rtpdec_svq3.h"
+
+struct PayloadContext {
+ ByteIOContext *pktbuf;
+ int64_t timestamp;
+ int is_keyframe;
+};
+
+/** return 0 on packet, <0 on partial packet or error... */
+static int svq3_parse_packet (AVFormatContext *s, PayloadContext *sv,
+ AVStream *st, AVPacket *pkt,
+ uint32_t *timestamp,
+ const uint8_t *buf, int len, int flags)
+{
+ int config_packet, start_packet, end_packet;
+
+ if (len < 2)
+ return AVERROR_INVALIDDATA;
+
+ config_packet = buf[0] & 0x40;
+ start_packet = buf[0] & 0x20;
+ end_packet = buf[0] & 0x10;
+ buf += 2; // ignore buf[1]
+ len -= 2;
+
+ if (config_packet) {
+
+ av_freep(&st->codec->extradata);
+
+ if (len < 2 || !(st->codec->extradata =
+ av_malloc(len + 8 + FF_INPUT_BUFFER_PADDING_SIZE)))
+ return AVERROR_INVALIDDATA;
+
+ st->codec->extradata_size = len + 8;
+ memcpy(st->codec->extradata, "SEQH", 4);
+ AV_WB32(st->codec->extradata + 4, len);
+ memcpy(st->codec->extradata + 8, buf, len);
+
+ /* We set codec_id to CODEC_ID_NONE initially to
+ * delay decoder initialization since extradata is
+ * carried within the RTP stream, not SDP. Here,
+ * by setting codec_id to CODEC_ID_SVQ3, we are signalling
+ * to the decoder that it is OK to initialize. */
+ st->codec->codec_id = CODEC_ID_SVQ3;
+
+ return AVERROR(EAGAIN);
+ }
+
+ if (start_packet) {
+ int res;
+
+ if (sv->pktbuf) {
+ uint8_t *tmp;
+ url_close_dyn_buf(sv->pktbuf, &tmp);
+ av_free(tmp);
+ }
+ if ((res = url_open_dyn_buf(&sv->pktbuf)) < 0)
+ return res;
+ sv->timestamp = *timestamp;
+ sv->is_keyframe = flags & RTP_FLAG_KEY;
+ }
+
+ if (!sv->pktbuf)
+ return AVERROR_INVALIDDATA;
+
+ put_buffer(sv->pktbuf, buf, len);
+
+ if (end_packet) {
+ av_init_packet(pkt);
+ pkt->stream_index = st->index;
+ pkt->pts = sv->timestamp;
+ pkt->flags = sv->is_keyframe ? AV_PKT_FLAG_KEY : 0;
+ pkt->size = url_close_dyn_buf(sv->pktbuf, &pkt->data);
+ pkt->destruct = av_destruct_packet;
+ sv->pktbuf = NULL;
+ return 0;
+ }
+
+ return AVERROR(EAGAIN);
+}
+
+static PayloadContext *svq3_extradata_new(void)
+{
+ return av_mallocz(sizeof(PayloadContext));
+}
+
+static void svq3_extradata_free(PayloadContext *sv)
+{
+ if (sv->pktbuf) {
+ uint8_t *buf;
+ url_close_dyn_buf(sv->pktbuf, &buf);
+ av_free(buf);
+ }
+ av_free(sv);
+}
+
+RTPDynamicProtocolHandler ff_svq3_dynamic_handler = {
+ "X-SV3V-ES",
+ CODEC_TYPE_VIDEO,
+ CODEC_ID_NONE, // see if (config_packet) above
+ NULL, // parse sdp line
+ svq3_extradata_new,
+ svq3_extradata_free,
+ svq3_parse_packet,
+};
diff --git a/libavformat/rtpdec_svq3.h b/libavformat/rtpdec_svq3.h
new file mode 100644
index 0000000..267d4d9
--- /dev/null
+++ b/libavformat/rtpdec_svq3.h
@@ -0,0 +1,33 @@
+/*
+ * Sorenson-3 (SVQ3/SV3V) payload for RTP
+ * Copyright (c) 2010 Ronald S. Bultje
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVFORMAT_RTPDEC_SVQ3_H
+#define AVFORMAT_RTPDEC_SVQ3_H
+
+#include "libavcodec/avcodec.h"
+#include "rtpdec.h"
+
+/**
+ * Sorenson-3 RTP callbacks.
+ */
+extern RTPDynamicProtocolHandler ff_svq3_dynamic_handler;
+
+#endif /* AVFORMAT_RTPDEC_SVQ3_H */
--
1.7.0.4
From 44559d594e78477e2c1c51bd395fc7f92053b716 Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Thu, 1 Jul 2010 09:01:05 -0700
Subject: [PATCH 2/2] Pad the buffer in url_close_dyn_buf.
---
libavformat/avio.h | 5 ++++-
libavformat/aviobuf.c | 7 ++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/libavformat/avio.h b/libavformat/avio.h
index dc8fdd6..a955048 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -530,7 +530,10 @@ int url_open_dyn_packet_buf(ByteIOContext **s, int max_packet_size);
/**
* Return the written size and a pointer to the buffer. The buffer
- * must be freed with av_free().
+ * must be freed with av_free(). If the buffer is opened with
+ * url_open_dyn_buf, then padding of FF_INPUT_BUFFER_PADDING_SIZE is
+ * added; if opened with url_open_dyn_packet_buf, no padding is added.
+ *
* @param s IO context
* @param pbuffer pointer to a byte buffer
* @return the length of the byte buffer
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 8684903..d9eb422 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -894,6 +894,11 @@ int url_close_dyn_buf(ByteIOContext *s, uint8_t **pbuffer)
{
DynBuffer *d = s->opaque;
int size;
+ static const char padbuf[FF_INPUT_BUFFER_PADDING_SIZE] = {0};
+
+ /* don't attempt to pad fixed-size packet buffers */
+ if (!s->max_packet_size)
+ put_buffer(s, padbuf, sizeof(padbuf));
put_flush_packet(s);
@@ -901,6 +906,6 @@ int url_close_dyn_buf(ByteIOContext *s, uint8_t **pbuffer)
size = d->size;
av_free(d);
av_free(s);
- return size;
+ return size - (s->max_packet_size ? 0 : FF_INPUT_BUFFER_PADDING_SIZE);
}
#endif /* CONFIG_MUXERS || CONFIG_NETWORK */
--
1.7.0.4
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
