On 03/03/2015 10:13 AM, Martin Storsjö wrote:
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?
Right, fixed.
+
+#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).
Yes, I removed the AVSTREAM_PARSE_FULL flag from the patch. We can check
this again for the next/final version of the parser.
According to Diego's comment I have also removed the debug message in
the beginning of the init function.
+
+ 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:
Okay, I see, we can improve this.
But I wouldn't expect a change for the definition of RTP_FLAG_MARKER (or
the RTP rfc).
if (!!last_fragment != !!(flags & RTP_FLAG_MARKER))
I have added a combination of your and Luca's comment.
+ 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.
Thanks, I use this also in other parsers. I believe this makes the code
more readable since such defines give a hint from where a "1" or "2" comes.
(e.g., in the beginning of vp9_handle_packet() where there is a check
for "len < RTP_VP9_DESC_REQUIRED_SIZE + 1")
Of course, one could argue that a comment would do the same.
Best regards,
Thomas.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
