On Tue, 3 Mar 2015, Thomas Volkert wrote:
From: Thomas Volkert <[email protected]>
---
Changelog | 1 +
libavformat/Makefile | 1 +
libavformat/rtpdec.c | 1 +
libavformat/rtpdec_formats.h | 1 +
libavformat/rtpdec_vp9.c | 298 +++++++++++++++++++++++++++++++++++++++++++
libavformat/version.h | 2 +-
6 files changed, 303 insertions(+), 1 deletion(-)
create mode 100644 libavformat/rtpdec_vp9.c
diff --git a/Changelog b/Changelog
index e4df595..31a550d 100644
--- a/Changelog
+++ b/Changelog
@@ -21,6 +21,7 @@ version <next>:
- RTP depacketizer for DV (RFC 6469)
- Canopus HQX decoder
- RTP depacketization of T.140 text (RFC 4103)
+- VP9 RTP payload format (draft 0) experimental depacketizer
version 11:
diff --git a/libavformat/Makefile b/libavformat/Makefile
index a89d723..d60404b 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -50,6 +50,7 @@ OBJS-$(CONFIG_RTPDEC) += rdt.o
\
rtpdec_qt.o \
rtpdec_svq3.o \
rtpdec_vp8.o \
+ rtpdec_vp9.o \
rtpdec_xiph.o \
srtp.o
OBJS-$(CONFIG_RTPENC_CHAIN) += rtpenc_chain.o rtp.o
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index a80463b..dd2e58f 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -101,6 +101,7 @@ void ff_register_rtp_dynamic_payload_handlers(void)
ff_register_dynamic_payload_handler(&ff_theora_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_vorbis_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_vp8_dynamic_handler);
+ ff_register_dynamic_payload_handler(&ff_vp9_dynamic_handler);
ff_register_dynamic_payload_handler(&opus_dynamic_handler);
ff_register_dynamic_payload_handler(&realmedia_mp3_dynamic_handler);
ff_register_dynamic_payload_handler(&speex_dynamic_handler);
diff --git a/libavformat/rtpdec_formats.h b/libavformat/rtpdec_formats.h
index 43b4b5b..b71b1ae 100644
--- a/libavformat/rtpdec_formats.h
+++ b/libavformat/rtpdec_formats.h
@@ -82,5 +82,6 @@ extern RTPDynamicProtocolHandler ff_svq3_dynamic_handler;
extern RTPDynamicProtocolHandler ff_theora_dynamic_handler;
extern RTPDynamicProtocolHandler ff_vorbis_dynamic_handler;
extern RTPDynamicProtocolHandler ff_vp8_dynamic_handler;
+extern RTPDynamicProtocolHandler ff_vp9_dynamic_handler;
#endif /* AVFORMAT_RTPDEC_FORMATS_H */
diff --git a/libavformat/rtpdec_vp9.c b/libavformat/rtpdec_vp9.c
new file mode 100644
index 0000000..7c0c3d5
--- /dev/null
+++ b/libavformat/rtpdec_vp9.c
@@ -0,0 +1,298 @@
+/*
+ * RTP parser for VP9 payload format (draft version 0) - experimental
+ * Copyright (c) 2015 Thomas Volkert <[email protected]>
+ *
+ * This file is part of Libav.
+ *
+ * Libav 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.
+ *
+ * Libav 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 Libav; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavcodec/bytestream.h"
Use libavutil/intreadwrite.h - I don't see you using the actual bytestream
functions here, right?
+
+#include "avio_internal.h"
+#include "rtpdec_formats.h"
+
+#define RTP_VP9_DESC_REQUIRED_SIZE 1
+
+struct PayloadContext {
+ AVIOContext *buf;
+ uint32_t timestamp;
+};
+
+static av_cold int vp9_init(AVFormatContext *ctx, int st_index,
+ PayloadContext *data)
Indentation is slightly off
+{
+ av_dlog(ctx, "vp9_init() for stream %d\n", st_index);
+ av_log(ctx, AV_LOG_WARNING,
+ "RTP/VP9 support is still experimental\n");
+
+ if (st_index < 0)
+ return 0;
+
+ ctx->streams[st_index]->need_parsing = AVSTREAM_PARSE_FULL;
Please set this via the .need_parsing field. Also, does this even work in
libav when we don't have any VP9 parser? Since we actually reassemble the
frames here within the depacketizer (contrary to the H264/HEVC case,
where we return partial frames and have the parser to reassemble them), it
shouldn't really be necessary (except for splitting superframes in the
ffmpeg vp9 decoder perhaps).
In principle, the init function wouldn't be needed, but printing the
warning is useful. But setting need_parsing shouldn't be done here any
longer (this reduced lots and lots of redundant lines in all the
depacketizers).
+
+ return 0;
+}
+
+static int vp9_handle_packet(AVFormatContext *ctx, PayloadContext *rtp_vp9_ctx,
+ AVStream *st, AVPacket *pkt, uint32_t *timestamp,
+ const uint8_t *buf, int len, uint16_t seq,
+ int flags)
+{
+ int has_pic_id, has_layer_idc, has_ref_idc, has_ss_data, has_su_data;
+ av_unused int pic_id = 0, non_key_frame = 0;
+ av_unused int layer_temporal = -1, layer_spatial = -1, layer_quality = -1;
+ int ref_fields = 0, has_ref_field_ext_pic_id = 0;
+ int first_fragment, last_fragment;
+ int res = 0;
+
+ /* drop data of previous packets in case of non-continuous (lossy) packet
stream */
+ if (rtp_vp9_ctx->buf && rtp_vp9_ctx->timestamp != *timestamp) {
+ ffio_free_dyn_buf(&rtp_vp9_ctx->buf);
+ }
+
+ /* sanity check for size of input packet: 1 byte payload at least */
+ if (len < RTP_VP9_DESC_REQUIRED_SIZE + 1) {
+ av_log(ctx, AV_LOG_ERROR, "Too short RTP/VP9 packet, got %d bytes\n",
len);
+ return AVERROR_INVALIDDATA;
+ }
+
+ /*
+ * decode the required VP9 payload descriptor according to section 4.2
of the spec.:
+ *
+ * 0 1 2 3 4 5 6 7
+ * +-+-+-+-+-+-+-+-+
+ * |I|L|F|B|E|V|U|-| (REQUIRED)
+ * +-+-+-+-+-+-+-+-+
+ *
+ * I: PictureID present
+ * L: Layer indices present
+ * F: Reference indices present
+ * B: Start of VP9 frame
+ * E: End of picture
+ * V: Scalability Structure (SS) present
+ * U: Scalability Structure Update (SU) present
+ */
+ has_pic_id = buf[0] & 0x80;
+ has_layer_idc = buf[0] & 0x40;
+ has_ref_idc = buf[0] & 0x20;
+ first_fragment = buf[0] & 0x10;
+ last_fragment = buf[0] & 0x08;
+ has_ss_data = buf[0] & 0x04;
+ has_su_data = buf[0] & 0x02;
+
+ /* sanity check for markers: B should always be equal to the RTP M marker
*/
+ if (last_fragment >> 2 != flags & RTP_FLAG_MARKER) {
This kinda hardcodes the value of RTP_FLAG_MARKER in a quite fragile way.
(If someone every were to change the value of RTP_FLAG_MARKER, nobody
would notice that this check needs to be updated, and this would break.)
What about this instead:
if (!!last_fragment != !!(flags & RTP_FLAG_MARKER))
+ av_log(ctx, AV_LOG_ERROR, "Invalid combination of B and M marker\n");
+ return AVERROR_INVALIDDATA;
+ }
+
+ /* pass the extensions field */
+ buf += RTP_VP9_DESC_REQUIRED_SIZE;
+ len -= RTP_VP9_DESC_REQUIRED_SIZE;
I'm actually not a huge fan of these extra defines (sometimes they are
useful, but sometimes they mostly obscure the code). Further below, you
have simple buf += 2 and such - I think that's a bit clearer; when the
code reads every bit out of buf[0], it's clear what buf++ does - while
reading it now the reader keeps thinking "are there other bits included in
RTP_VP9_DESC_REQUIRED_SIZE that we skipped over, or is it just equal to 1"
when it is not exactly clear what the constant is.
But if you feel strongly about it I won't object.
The rest of the code seems straightforward though.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
